[patch -mainline] af_key: more info leaks in pfkey messages

[patch -mainline] af_key: more info leaks in pfkey messages

[Dan Carpenter]

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;
 

Re: [patch -mainline] af_key: more info leaks in pfkey messages

[Mathias Krause]

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>

Re: [patch -mainline] af_key: more info leaks in pfkey messages

[Steffen Klassert]

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.

Re: [patch -mainline] af_key: more info leaks in pfkey messages

[Dan Carpenter]

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

[patch -mainline v2] af_key: more info leaks in pfkey messages

[Dan Carpenter]

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;
 

Re: [patch -mainline v2] af_key: more info leaks in pfkey messages

[David Miller]

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.

Re: [patch -mainline v2] af_key: more info leaks in pfkey messages

[Steffen Klassert]

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>

Re: [patch -mainline v2] af_key: more info leaks in pfkey messages

[David Miller]

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.