All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/2] two small fixes for sctp key usage
@ 2013-02-08 13:04 ` Daniel Borkmann
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Borkmann @ 2013-02-08 13:04 UTC (permalink / raw)
  To: davem; +Cc: linux-sctp, netdev

Cryptographically used keys should be zeroed out when our session
ends resp. memory is freed, thus do not leave them somewhere in the
memory.

Daniel Borkmann (2):
  net: sctp: sctp_setsockopt_auth_key: use kzfree instead of kfree
  net: sctp: sctp_endpoint_free: zero out secret key data

 net/sctp/endpointola.c | 5 +++++
 net/sctp/socket.c      | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH net 0/2] two small fixes for sctp key usage
@ 2013-02-08 13:04 ` Daniel Borkmann
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Borkmann @ 2013-02-08 13:04 UTC (permalink / raw)
  To: davem; +Cc: linux-sctp, netdev

Cryptographically used keys should be zeroed out when our session
ends resp. memory is freed, thus do not leave them somewhere in the
memory.

Daniel Borkmann (2):
  net: sctp: sctp_setsockopt_auth_key: use kzfree instead of kfree
  net: sctp: sctp_endpoint_free: zero out secret key data

 net/sctp/endpointola.c | 5 +++++
 net/sctp/socket.c      | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

-- 
1.7.11.7


^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH net 1/2] net: sctp: sctp_setsockopt_auth_key: use kzfree instead of kfree
  2013-02-08 13:04 ` Daniel Borkmann
@ 2013-02-08 13:04   ` Daniel Borkmann
  -1 siblings, 0 replies; 12+ messages in thread
From: Daniel Borkmann @ 2013-02-08 13:04 UTC (permalink / raw)
  To: davem; +Cc: linux-sctp, netdev

In sctp_setsockopt_auth_key, we create a temporary copy of the user
passed shared auth key for the endpoint or association and after
internal setup, we free it right away. Since it's sensitive data, we
should zero out the key before returning the memory back to the
allocator. Thus, use kzfree instead of kfree, just as we do in
sctp_auth_key_put().

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 net/sctp/socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 9e65758..cedd9bf 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3390,7 +3390,7 @@ static int sctp_setsockopt_auth_key(struct sock *sk,
 
 	ret = sctp_auth_set_key(sctp_sk(sk)->ep, asoc, authkey);
 out:
-	kfree(authkey);
+	kzfree(authkey);
 	return ret;
 }
 
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH net 1/2] net: sctp: sctp_setsockopt_auth_key: use kzfree instead of kfree
@ 2013-02-08 13:04   ` Daniel Borkmann
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Borkmann @ 2013-02-08 13:04 UTC (permalink / raw)
  To: davem; +Cc: linux-sctp, netdev

In sctp_setsockopt_auth_key, we create a temporary copy of the user
passed shared auth key for the endpoint or association and after
internal setup, we free it right away. Since it's sensitive data, we
should zero out the key before returning the memory back to the
allocator. Thus, use kzfree instead of kfree, just as we do in
sctp_auth_key_put().

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 net/sctp/socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 9e65758..cedd9bf 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3390,7 +3390,7 @@ static int sctp_setsockopt_auth_key(struct sock *sk,
 
 	ret = sctp_auth_set_key(sctp_sk(sk)->ep, asoc, authkey);
 out:
-	kfree(authkey);
+	kzfree(authkey);
 	return ret;
 }
 
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH net 2/2] net: sctp: sctp_endpoint_free: zero out secret key data
  2013-02-08 13:04 ` Daniel Borkmann
@ 2013-02-08 13:04   ` Daniel Borkmann
  -1 siblings, 0 replies; 12+ messages in thread
From: Daniel Borkmann @ 2013-02-08 13:04 UTC (permalink / raw)
  To: davem; +Cc: linux-sctp, netdev

On sctp_endpoint_destroy, previously used sensitive keying material
should be zeroed out before the memory is returned, as we already do
with e.g. auth keys when released.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 net/sctp/endpointola.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index 17a001b..1a9c5fb 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -249,6 +249,8 @@ void sctp_endpoint_free(struct sctp_endpoint *ep)
 /* Final destructor for endpoint.  */
 static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
 {
+	int i;
+
 	SCTP_ASSERT(ep->base.dead, "Endpoint is not dead", return);
 
 	/* Free up the HMAC transform. */
@@ -271,6 +273,9 @@ static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
 	sctp_inq_free(&ep->base.inqueue);
 	sctp_bind_addr_free(&ep->base.bind_addr);
 
+	for (i = 0; i < SCTP_HOW_MANY_SECRETS; ++i)
+		memset(&ep->secret_key[i], 0, SCTP_SECRET_SIZE);
+
 	/* Remove and free the port */
 	if (sctp_sk(ep->base.sk)->bind_hash)
 		sctp_put_port(ep->base.sk);
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH net 2/2] net: sctp: sctp_endpoint_free: zero out secret key data
@ 2013-02-08 13:04   ` Daniel Borkmann
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Borkmann @ 2013-02-08 13:04 UTC (permalink / raw)
  To: davem; +Cc: linux-sctp, netdev

On sctp_endpoint_destroy, previously used sensitive keying material
should be zeroed out before the memory is returned, as we already do
with e.g. auth keys when released.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 net/sctp/endpointola.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index 17a001b..1a9c5fb 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -249,6 +249,8 @@ void sctp_endpoint_free(struct sctp_endpoint *ep)
 /* Final destructor for endpoint.  */
 static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
 {
+	int i;
+
 	SCTP_ASSERT(ep->base.dead, "Endpoint is not dead", return);
 
 	/* Free up the HMAC transform. */
@@ -271,6 +273,9 @@ static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
 	sctp_inq_free(&ep->base.inqueue);
 	sctp_bind_addr_free(&ep->base.bind_addr);
 
+	for (i = 0; i < SCTP_HOW_MANY_SECRETS; ++i)
+		memset(&ep->secret_key[i], 0, SCTP_SECRET_SIZE);
+
 	/* Remove and free the port */
 	if (sctp_sk(ep->base.sk)->bind_hash)
 		sctp_put_port(ep->base.sk);
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH net 2/2] net: sctp: sctp_endpoint_free: zero out secret key data
  2013-02-08 13:04   ` Daniel Borkmann
@ 2013-02-08 15:50     ` Vlad Yasevich
  -1 siblings, 0 replies; 12+ messages in thread
From: Vlad Yasevich @ 2013-02-08 15:50 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: davem, linux-sctp, netdev

On 02/08/2013 08:04 AM, Daniel Borkmann wrote:
> On sctp_endpoint_destroy, previously used sensitive keying material
> should be zeroed out before the memory is returned, as we already do
> with e.g. auth keys when released.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>

I'll ack this, but the whole multiple cookie keys code is completely 
unused and has been all this time.  Noone uses anything other then the 
secret_key[0] since there is no changeover support anywhere.  It might 
be nice to clean that up too.

Acked-by: Vlad Yasevich <vyasevic@redhat.com>

-vlad

> ---
>   net/sctp/endpointola.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
> index 17a001b..1a9c5fb 100644
> --- a/net/sctp/endpointola.c
> +++ b/net/sctp/endpointola.c
> @@ -249,6 +249,8 @@ void sctp_endpoint_free(struct sctp_endpoint *ep)
>   /* Final destructor for endpoint.  */
>   static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
>   {
> +	int i;
> +
>   	SCTP_ASSERT(ep->base.dead, "Endpoint is not dead", return);
>
>   	/* Free up the HMAC transform. */
> @@ -271,6 +273,9 @@ static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
>   	sctp_inq_free(&ep->base.inqueue);
>   	sctp_bind_addr_free(&ep->base.bind_addr);
>
> +	for (i = 0; i < SCTP_HOW_MANY_SECRETS; ++i)
> +		memset(&ep->secret_key[i], 0, SCTP_SECRET_SIZE);
> +
>   	/* Remove and free the port */
>   	if (sctp_sk(ep->base.sk)->bind_hash)
>   		sctp_put_port(ep->base.sk);
>


c
If

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH net 2/2] net: sctp: sctp_endpoint_free: zero out secret key data
@ 2013-02-08 15:50     ` Vlad Yasevich
  0 siblings, 0 replies; 12+ messages in thread
From: Vlad Yasevich @ 2013-02-08 15:50 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: davem, linux-sctp, netdev

On 02/08/2013 08:04 AM, Daniel Borkmann wrote:
> On sctp_endpoint_destroy, previously used sensitive keying material
> should be zeroed out before the memory is returned, as we already do
> with e.g. auth keys when released.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>

I'll ack this, but the whole multiple cookie keys code is completely 
unused and has been all this time.  Noone uses anything other then the 
secret_key[0] since there is no changeover support anywhere.  It might 
be nice to clean that up too.

Acked-by: Vlad Yasevich <vyasevic@redhat.com>

-vlad

> ---
>   net/sctp/endpointola.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
> index 17a001b..1a9c5fb 100644
> --- a/net/sctp/endpointola.c
> +++ b/net/sctp/endpointola.c
> @@ -249,6 +249,8 @@ void sctp_endpoint_free(struct sctp_endpoint *ep)
>   /* Final destructor for endpoint.  */
>   static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
>   {
> +	int i;
> +
>   	SCTP_ASSERT(ep->base.dead, "Endpoint is not dead", return);
>
>   	/* Free up the HMAC transform. */
> @@ -271,6 +273,9 @@ static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
>   	sctp_inq_free(&ep->base.inqueue);
>   	sctp_bind_addr_free(&ep->base.bind_addr);
>
> +	for (i = 0; i < SCTP_HOW_MANY_SECRETS; ++i)
> +		memset(&ep->secret_key[i], 0, SCTP_SECRET_SIZE);
> +
>   	/* Remove and free the port */
>   	if (sctp_sk(ep->base.sk)->bind_hash)
>   		sctp_put_port(ep->base.sk);
>


c
If

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH net 2/2] net: sctp: sctp_endpoint_free: zero out secret key data
  2013-02-08 15:50     ` Vlad Yasevich
@ 2013-02-08 16:02       ` Daniel Borkmann
  -1 siblings, 0 replies; 12+ messages in thread
From: Daniel Borkmann @ 2013-02-08 16:02 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: davem, linux-sctp, netdev

On 02/08/2013 04:50 PM, Vlad Yasevich wrote:
> On 02/08/2013 08:04 AM, Daniel Borkmann wrote:
>> On sctp_endpoint_destroy, previously used sensitive keying material
>> should be zeroed out before the memory is returned, as we already do
>> with e.g. auth keys when released.
>>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>
> I'll ack this, but the whole multiple cookie keys code is completely unused and has been all this time.  Noone uses anything other then the secret_key[0] since there is no changeover support anywhere.  It might be nice to clean that up too.

Put on my todo list for follow-up patches, thanks.

> Acked-by: Vlad Yasevich <vyasevic@redhat.com>
>
> -vlad
>
>> ---
>>   net/sctp/endpointola.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
>> index 17a001b..1a9c5fb 100644
>> --- a/net/sctp/endpointola.c
>> +++ b/net/sctp/endpointola.c
>> @@ -249,6 +249,8 @@ void sctp_endpoint_free(struct sctp_endpoint *ep)
>>   /* Final destructor for endpoint.  */
>>   static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
>>   {
>> +    int i;
>> +
>>       SCTP_ASSERT(ep->base.dead, "Endpoint is not dead", return);
>>
>>       /* Free up the HMAC transform. */
>> @@ -271,6 +273,9 @@ static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
>>       sctp_inq_free(&ep->base.inqueue);
>>       sctp_bind_addr_free(&ep->base.bind_addr);
>>
>> +    for (i = 0; i < SCTP_HOW_MANY_SECRETS; ++i)
>> +        memset(&ep->secret_key[i], 0, SCTP_SECRET_SIZE);
>> +
>>       /* Remove and free the port */
>>       if (sctp_sk(ep->base.sk)->bind_hash)
>>           sctp_put_port(ep->base.sk);
>>
>
>
> c
> If

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH net 2/2] net: sctp: sctp_endpoint_free: zero out secret key data
@ 2013-02-08 16:02       ` Daniel Borkmann
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Borkmann @ 2013-02-08 16:02 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: davem, linux-sctp, netdev

On 02/08/2013 04:50 PM, Vlad Yasevich wrote:
> On 02/08/2013 08:04 AM, Daniel Borkmann wrote:
>> On sctp_endpoint_destroy, previously used sensitive keying material
>> should be zeroed out before the memory is returned, as we already do
>> with e.g. auth keys when released.
>>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>
> I'll ack this, but the whole multiple cookie keys code is completely unused and has been all this time.  Noone uses anything other then the secret_key[0] since there is no changeover support anywhere.  It might be nice to clean that up too.

Put on my todo list for follow-up patches, thanks.

> Acked-by: Vlad Yasevich <vyasevic@redhat.com>
>
> -vlad
>
>> ---
>>   net/sctp/endpointola.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
>> index 17a001b..1a9c5fb 100644
>> --- a/net/sctp/endpointola.c
>> +++ b/net/sctp/endpointola.c
>> @@ -249,6 +249,8 @@ void sctp_endpoint_free(struct sctp_endpoint *ep)
>>   /* Final destructor for endpoint.  */
>>   static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
>>   {
>> +    int i;
>> +
>>       SCTP_ASSERT(ep->base.dead, "Endpoint is not dead", return);
>>
>>       /* Free up the HMAC transform. */
>> @@ -271,6 +273,9 @@ static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
>>       sctp_inq_free(&ep->base.inqueue);
>>       sctp_bind_addr_free(&ep->base.bind_addr);
>>
>> +    for (i = 0; i < SCTP_HOW_MANY_SECRETS; ++i)
>> +        memset(&ep->secret_key[i], 0, SCTP_SECRET_SIZE);
>> +
>>       /* Remove and free the port */
>>       if (sctp_sk(ep->base.sk)->bind_hash)
>>           sctp_put_port(ep->base.sk);
>>
>
>
> c
> If

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH net 0/2] two small fixes for sctp key usage
  2013-02-08 13:04 ` Daniel Borkmann
@ 2013-02-08 19:55   ` David Miller
  -1 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2013-02-08 19:55 UTC (permalink / raw)
  To: dborkman; +Cc: linux-sctp, netdev

From: Daniel Borkmann <dborkman@redhat.com>
Date: Fri,  8 Feb 2013 14:04:33 +0100

> Cryptographically used keys should be zeroed out when our session
> ends resp. memory is freed, thus do not leave them somewhere in the
> memory.
> 
> Daniel Borkmann (2):
>   net: sctp: sctp_setsockopt_auth_key: use kzfree instead of kfree
>   net: sctp: sctp_endpoint_free: zero out secret key data

Applied and queued up for -stable, thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH net 0/2] two small fixes for sctp key usage
@ 2013-02-08 19:55   ` David Miller
  0 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2013-02-08 19:55 UTC (permalink / raw)
  To: dborkman; +Cc: linux-sctp, netdev

From: Daniel Borkmann <dborkman@redhat.com>
Date: Fri,  8 Feb 2013 14:04:33 +0100

> Cryptographically used keys should be zeroed out when our session
> ends resp. memory is freed, thus do not leave them somewhere in the
> memory.
> 
> Daniel Borkmann (2):
>   net: sctp: sctp_setsockopt_auth_key: use kzfree instead of kfree
>   net: sctp: sctp_endpoint_free: zero out secret key data

Applied and queued up for -stable, thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-02-08 19:55 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-08 13:04 [PATCH net 0/2] two small fixes for sctp key usage Daniel Borkmann
2013-02-08 13:04 ` Daniel Borkmann
2013-02-08 13:04 ` [PATCH net 1/2] net: sctp: sctp_setsockopt_auth_key: use kzfree instead of kfree Daniel Borkmann
2013-02-08 13:04   ` Daniel Borkmann
2013-02-08 13:04 ` [PATCH net 2/2] net: sctp: sctp_endpoint_free: zero out secret key data Daniel Borkmann
2013-02-08 13:04   ` Daniel Borkmann
2013-02-08 15:50   ` Vlad Yasevich
2013-02-08 15:50     ` Vlad Yasevich
2013-02-08 16:02     ` Daniel Borkmann
2013-02-08 16:02       ` Daniel Borkmann
2013-02-08 19:55 ` [PATCH net 0/2] two small fixes for sctp key usage David Miller
2013-02-08 19:55   ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.