netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch -mainline] af_key: more info leaks in pfkey messages
@ 2013-07-24 10:39 Dan Carpenter
  2013-07-25  7:15 ` Mathias Krause
  2013-07-25  7:32 ` Steffen Klassert
  0 siblings, 2 replies; 8+ messages in thread
From: Dan Carpenter @ 2013-07-24 10:39 UTC (permalink / raw)
  To: Steffen Klassert, Mathias Krause
  Cc: Herbert Xu, David S. Miller, netdev, kernel-janitors

This is inspired by a5cc68f3d6 "af_key: fix info leaks in notify
messages".  There are some struct members which don't get initialized
and could disclose small amounts of private information.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
I'm at the "monkey see, monkey do" level of understanding this code so
let me know if I've missed something.

diff --git a/net/key/af_key.c b/net/key/af_key.c
index 456b262..e939d32 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -2081,6 +2081,7 @@ static int pfkey_xfrm_policy2msg(struct sk_buff *skb, const struct xfrm_policy *
 			pol->sadb_x_policy_type = IPSEC_POLICY_NONE;
 	}
 	pol->sadb_x_policy_dir = dir+1;
+	pol->sadb_x_policy_reserved = 0;
 	pol->sadb_x_policy_id = xp->index;
 	pol->sadb_x_policy_priority = xp->priority;
 
@@ -3137,7 +3138,9 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
 	pol->sadb_x_policy_exttype = SADB_X_EXT_POLICY;
 	pol->sadb_x_policy_type = IPSEC_POLICY_IPSEC;
 	pol->sadb_x_policy_dir = XFRM_POLICY_OUT + 1;
+	pol->sadb_x_policy_reserved = 0;
 	pol->sadb_x_policy_id = xp->index;
+	pol->sadb_x_policy_priority = 0;
 
 	/* Set sadb_comb's. */
 	if (x->id.proto == IPPROTO_AH)
@@ -3525,6 +3528,7 @@ static int pfkey_send_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 	pol->sadb_x_policy_exttype = SADB_X_EXT_POLICY;
 	pol->sadb_x_policy_type = IPSEC_POLICY_IPSEC;
 	pol->sadb_x_policy_dir = dir + 1;
+	pol->sadb_x_policy_reserved = 0;
 	pol->sadb_x_policy_id = 0;
 	pol->sadb_x_policy_priority = 0;
 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch -mainline] af_key: more info leaks in pfkey messages
  2013-07-24 10:39 [patch -mainline] af_key: more info leaks in pfkey messages Dan Carpenter
@ 2013-07-25  7:15 ` Mathias Krause
  2013-07-25  7:32 ` Steffen Klassert
  1 sibling, 0 replies; 8+ messages in thread
From: Mathias Krause @ 2013-07-25  7:15 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Steffen Klassert, Herbert Xu, David S. Miller, netdev, kernel-janitors

On 24 July 2013 12:39, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> This is inspired by a5cc68f3d6 "af_key: fix info leaks in notify
> messages".  There are some struct members which don't get initialized
> and could disclose small amounts of private information.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> I'm at the "monkey see, monkey do" level of understanding this code so
> let me know if I've missed something.
>
> diff --git a/net/key/af_key.c b/net/key/af_key.c
> index 456b262..e939d32 100644
> --- a/net/key/af_key.c
> +++ b/net/key/af_key.c
> @@ -2081,6 +2081,7 @@ static int pfkey_xfrm_policy2msg(struct sk_buff *skb, const struct xfrm_policy *
>                         pol->sadb_x_policy_type = IPSEC_POLICY_NONE;
>         }
>         pol->sadb_x_policy_dir = dir+1;
> +       pol->sadb_x_policy_reserved = 0;
>         pol->sadb_x_policy_id = xp->index;
>         pol->sadb_x_policy_priority = xp->priority;
>
> @@ -3137,7 +3138,9 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
>         pol->sadb_x_policy_exttype = SADB_X_EXT_POLICY;
>         pol->sadb_x_policy_type = IPSEC_POLICY_IPSEC;
>         pol->sadb_x_policy_dir = XFRM_POLICY_OUT + 1;
> +       pol->sadb_x_policy_reserved = 0;
>         pol->sadb_x_policy_id = xp->index;
> +       pol->sadb_x_policy_priority = 0;
>
>         /* Set sadb_comb's. */
>         if (x->id.proto == IPPROTO_AH)
> @@ -3525,6 +3528,7 @@ static int pfkey_send_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
>         pol->sadb_x_policy_exttype = SADB_X_EXT_POLICY;
>         pol->sadb_x_policy_type = IPSEC_POLICY_IPSEC;
>         pol->sadb_x_policy_dir = dir + 1;
> +       pol->sadb_x_policy_reserved = 0;
>         pol->sadb_x_policy_id = 0;
>         pol->sadb_x_policy_priority = 0;
>

Looks good to me. All end up being send to userland over the PF_KEY interface.

Acked-by: Mathias Krause <minipli@googlemail.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch -mainline] af_key: more info leaks in pfkey messages
  2013-07-24 10:39 [patch -mainline] af_key: more info leaks in pfkey messages Dan Carpenter
  2013-07-25  7:15 ` Mathias Krause
@ 2013-07-25  7:32 ` Steffen Klassert
  2013-07-25  8:24   ` Dan Carpenter
  2013-07-28 20:04   ` [patch -mainline v2] " Dan Carpenter
  1 sibling, 2 replies; 8+ messages in thread
From: Steffen Klassert @ 2013-07-25  7:32 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Mathias Krause, Herbert Xu, David S. Miller, netdev, kernel-janitors

On Wed, Jul 24, 2013 at 01:39:57PM +0300, Dan Carpenter wrote:
> This is inspired by a5cc68f3d6 "af_key: fix info leaks in notify
> messages".  There are some struct members which don't get initialized
> and could disclose small amounts of private information.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> I'm at the "monkey see, monkey do" level of understanding this code so
> let me know if I've missed something.
> 
> diff --git a/net/key/af_key.c b/net/key/af_key.c
> index 456b262..e939d32 100644
> --- a/net/key/af_key.c
> +++ b/net/key/af_key.c
> @@ -2081,6 +2081,7 @@ static int pfkey_xfrm_policy2msg(struct sk_buff *skb, const struct xfrm_policy *
>  			pol->sadb_x_policy_type = IPSEC_POLICY_NONE;
>  	}
>  	pol->sadb_x_policy_dir = dir+1;
> +	pol->sadb_x_policy_reserved = 0;
>  	pol->sadb_x_policy_id = xp->index;
>  	pol->sadb_x_policy_priority = xp->priority;
>  
> @@ -3137,7 +3138,9 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
>  	pol->sadb_x_policy_exttype = SADB_X_EXT_POLICY;
>  	pol->sadb_x_policy_type = IPSEC_POLICY_IPSEC;
>  	pol->sadb_x_policy_dir = XFRM_POLICY_OUT + 1;
> +	pol->sadb_x_policy_reserved = 0;
>  	pol->sadb_x_policy_id = xp->index;
> +	pol->sadb_x_policy_priority = 0;

Userspace seems not to care, but the correct setting would be

pol->sadb_x_policy_priority = xp->priority;

So maybe we should send the right priority to userspace, just for
the case that anyone is interested in it.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch -mainline] af_key: more info leaks in pfkey messages
  2013-07-25  7:32 ` Steffen Klassert
@ 2013-07-25  8:24   ` Dan Carpenter
  2013-07-28 20:04   ` [patch -mainline v2] " Dan Carpenter
  1 sibling, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2013-07-25  8:24 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: Mathias Krause, Herbert Xu, David S. Miller, netdev, kernel-janitors

On Thu, Jul 25, 2013 at 09:32:06AM +0200, Steffen Klassert wrote:
> Userspace seems not to care, but the correct setting would be
> 
> pol->sadb_x_policy_priority = xp->priority;
> 
> So maybe we should send the right priority to userspace, just for
> the case that anyone is interested in it.
> 

Yep.  Of course.  Thanks.  I will redo and resend.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [patch -mainline v2] af_key: more info leaks in pfkey messages
  2013-07-25  7:32 ` Steffen Klassert
  2013-07-25  8:24   ` Dan Carpenter
@ 2013-07-28 20:04   ` Dan Carpenter
  2013-07-28 20:21     ` David Miller
  1 sibling, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2013-07-28 20:04 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: Herbert Xu, David S. Miller, netdev, kernel-janitors, Mathias Krause

This is inspired by a5cc68f3d6 "af_key: fix info leaks in notify
messages".  There are some struct members which don't get initialized
and could disclose small amounts of private information.

Acked-by: Mathias Krause <minipli@googlemail.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: set ->sadb_x_policy_priority correctly instead of just clearing it

diff --git a/net/key/af_key.c b/net/key/af_key.c
index 456b262..d49f676 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -2081,6 +2081,7 @@ static int pfkey_xfrm_policy2msg(struct sk_buff *skb, const struct xfrm_policy *
 			pol->sadb_x_policy_type = IPSEC_POLICY_NONE;
 	}
 	pol->sadb_x_policy_dir = dir+1;
+	pol->sadb_x_policy_reserved = 0;
 	pol->sadb_x_policy_id = xp->index;
 	pol->sadb_x_policy_priority = xp->priority;
 
@@ -3137,7 +3138,9 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
 	pol->sadb_x_policy_exttype = SADB_X_EXT_POLICY;
 	pol->sadb_x_policy_type = IPSEC_POLICY_IPSEC;
 	pol->sadb_x_policy_dir = XFRM_POLICY_OUT + 1;
+	pol->sadb_x_policy_reserved = 0;
 	pol->sadb_x_policy_id = xp->index;
+	pol->sadb_x_policy_priority = xp->priority;
 
 	/* Set sadb_comb's. */
 	if (x->id.proto == IPPROTO_AH)
@@ -3525,6 +3528,7 @@ static int pfkey_send_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 	pol->sadb_x_policy_exttype = SADB_X_EXT_POLICY;
 	pol->sadb_x_policy_type = IPSEC_POLICY_IPSEC;
 	pol->sadb_x_policy_dir = dir + 1;
+	pol->sadb_x_policy_reserved = 0;
 	pol->sadb_x_policy_id = 0;
 	pol->sadb_x_policy_priority = 0;
 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch -mainline v2] af_key: more info leaks in pfkey messages
  2013-07-28 20:04   ` [patch -mainline v2] " Dan Carpenter
@ 2013-07-28 20:21     ` David Miller
  2013-07-29  9:16       ` Steffen Klassert
  0 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2013-07-28 20:21 UTC (permalink / raw)
  To: dan.carpenter; +Cc: steffen.klassert, herbert, netdev, kernel-janitors, minipli

From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Sun, 28 Jul 2013 23:04:45 +0300

> This is inspired by a5cc68f3d6 "af_key: fix info leaks in notify
> messages".  There are some struct members which don't get initialized
> and could disclose small amounts of private information.
> 
> Acked-by: Mathias Krause <minipli@googlemail.com>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: set ->sadb_x_policy_priority correctly instead of just clearing it

I'll apply this as soon as Steffen reviews it.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch -mainline v2] af_key: more info leaks in pfkey messages
  2013-07-28 20:21     ` David Miller
@ 2013-07-29  9:16       ` Steffen Klassert
  2013-07-30 23:26         ` David Miller
  0 siblings, 1 reply; 8+ messages in thread
From: Steffen Klassert @ 2013-07-29  9:16 UTC (permalink / raw)
  To: David Miller; +Cc: dan.carpenter, herbert, netdev, kernel-janitors, minipli

On Sun, Jul 28, 2013 at 01:21:28PM -0700, David Miller wrote:
> From: Dan Carpenter <dan.carpenter@oracle.com>
> Date: Sun, 28 Jul 2013 23:04:45 +0300
> 
> > This is inspired by a5cc68f3d6 "af_key: fix info leaks in notify
> > messages".  There are some struct members which don't get initialized
> > and could disclose small amounts of private information.
> > 
> > Acked-by: Mathias Krause <minipli@googlemail.com>
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: set ->sadb_x_policy_priority correctly instead of just clearing it
> 
> I'll apply this as soon as Steffen reviews it.

The patch looks good, for the case you want to apply it directly to
the net tree:

Acked-by: Steffen Klassert <steffen.klassert@secunet.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch -mainline v2] af_key: more info leaks in pfkey messages
  2013-07-29  9:16       ` Steffen Klassert
@ 2013-07-30 23:26         ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2013-07-30 23:26 UTC (permalink / raw)
  To: steffen.klassert; +Cc: dan.carpenter, herbert, netdev, kernel-janitors, minipli

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Mon, 29 Jul 2013 11:16:53 +0200

> On Sun, Jul 28, 2013 at 01:21:28PM -0700, David Miller wrote:
>> From: Dan Carpenter <dan.carpenter@oracle.com>
>> Date: Sun, 28 Jul 2013 23:04:45 +0300
>> 
>> > This is inspired by a5cc68f3d6 "af_key: fix info leaks in notify
>> > messages".  There are some struct members which don't get initialized
>> > and could disclose small amounts of private information.
>> > 
>> > Acked-by: Mathias Krause <minipli@googlemail.com>
>> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> > ---
>> > v2: set ->sadb_x_policy_priority correctly instead of just clearing it
>> 
>> I'll apply this as soon as Steffen reviews it.
> 
> The patch looks good, for the case you want to apply it directly to
> the net tree:
> 
> Acked-by: Steffen Klassert <steffen.klassert@secunet.com>

Applied and queued up for -stable, thanks everyone.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-07-30 23:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-24 10:39 [patch -mainline] af_key: more info leaks in pfkey messages Dan Carpenter
2013-07-25  7:15 ` Mathias Krause
2013-07-25  7:32 ` Steffen Klassert
2013-07-25  8:24   ` Dan Carpenter
2013-07-28 20:04   ` [patch -mainline v2] " Dan Carpenter
2013-07-28 20:21     ` David Miller
2013-07-29  9:16       ` Steffen Klassert
2013-07-30 23:26         ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).