diff mbox

[v1,1/2] mac802154: Fix MAC header and payload encrypted

Message ID 1504613884-20870-1-git-send-email-dvnp@cesar.org.br (mailing list archive)
State Accepted
Headers show

Commit Message

Diogenes Pereira Sept. 5, 2017, 12:18 p.m. UTC
According to  802.15.4-2003/2006/2015 specifications the MAC frame is
composed of MHR, MAC payload and MFR and just the outgoing MAC payload
must be encrypted.

If communication is secure,sender build Auxiliary Security Header(ASH),
insert it next to the standard MHR header with security enabled bit ON,
and secure frames before transmitting them. According to the information
carried within the ASH, recipient retrieves the right cryptographic key
and correctly un-secure MAC frames.

The error scenario occurs on Linux using IEEE802154_SCF_SECLEVEL_ENC(4)
security level when llsec_do_encrypt_unauth() function builds theses MAC
frames incorrectly. On recipients these MAC frames are discarded,logging
"got invalid frame" messages.

Acked-by: Stefan Schmidt <stefan@osg.samsung.com>
Signed-off-by: Diogenes Pereira <dvnp@cesar.org.br>
---
 net/mac802154/llsec.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Stefan Schmidt Sept. 11, 2017, 11:21 a.m. UTC | #1
Hello.

On 09/05/2017 02:18 PM, Diogenes Pereira wrote:
> According to  802.15.4-2003/2006/2015 specifications the MAC frame is
> composed of MHR, MAC payload and MFR and just the outgoing MAC payload
> must be encrypted.
> 
> If communication is secure,sender build Auxiliary Security Header(ASH),
> insert it next to the standard MHR header with security enabled bit ON,
> and secure frames before transmitting them. According to the information
> carried within the ASH, recipient retrieves the right cryptographic key
> and correctly un-secure MAC frames.
> 
> The error scenario occurs on Linux using IEEE802154_SCF_SECLEVEL_ENC(4)
> security level when llsec_do_encrypt_unauth() function builds theses MAC
> frames incorrectly. On recipients these MAC frames are discarded,logging
> "got invalid frame" messages.
> 
> Acked-by: Stefan Schmidt <stefan@osg.samsung.com>

I did not ack this patch so far. Maybe you mixed this up with the second 
patch I acked.

I can see from the updated commit message that you also checked for this 
behavior in the older specs. Thanks! Did you also run tests against 
another LLC implementation (e.g. contiki) to see if it does not break 
anything fro them?

regards
Stefan Schmidt

> Signed-off-by: Diogenes Pereira <dvnp@cesar.org.br>
> ---
>   net/mac802154/llsec.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/net/mac802154/llsec.c b/net/mac802154/llsec.c
> index 1e1c9b2..d9e7105 100644
> --- a/net/mac802154/llsec.c
> +++ b/net/mac802154/llsec.c
> @@ -623,13 +623,18 @@ llsec_do_encrypt_unauth(struct sk_buff *skb, const struct mac802154_llsec *sec,
>   	u8 iv[16];
>   	struct scatterlist src;
>   	SKCIPHER_REQUEST_ON_STACK(req, key->tfm0);
> -	int err;
> +	int err, datalen;
> +	unsigned char *data;
>   
>   	llsec_geniv(iv, sec->params.hwaddr, &hdr->sec);
> -	sg_init_one(&src, skb->data, skb->len);
> +	/* Compute data payload offset and data length */
> +	data = skb_mac_header(skb) + skb->mac_len;
> +	datalen = skb_tail_pointer(skb) - data;
> +	sg_init_one(&src, data, datalen);
> +
>   	skcipher_request_set_tfm(req, key->tfm0);
>   	skcipher_request_set_callback(req, 0, NULL, NULL);
> -	skcipher_request_set_crypt(req, &src, &src, skb->len, iv);
> +	skcipher_request_set_crypt(req, &src, &src, datalen, iv);
>   	err = crypto_skcipher_encrypt(req);
>   	skcipher_request_zero(req);
>   	return err;
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stefan Schmidt Sept. 14, 2017, 1:35 p.m. UTC | #2
Hello.

On 09/13/2017 06:28 PM, Diógenes Vila Nova Pereira wrote:
> Hello,
> 
> LLC implementation that I checked some like Contiki, Zephyr and RIOT are 
> in specifications compliance.  All of them build the standard MHR and it 
> inserts the Auxiliary Security Header(ASH) with the Security Enabled bit 
> ON without encrypting them.

Thanks for taking the time to check the other implementations so we can 
feel safe to apply this change. Will ack the patch in a separate mail.

regards
Stefan Schmidt


--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stefan Schmidt Sept. 14, 2017, 1:39 p.m. UTC | #3
Hello.

On 09/05/2017 02:18 PM, Diogenes Pereira wrote:
> According to  802.15.4-2003/2006/2015 specifications the MAC frame is
> composed of MHR, MAC payload and MFR and just the outgoing MAC payload
> must be encrypted.
> 
> If communication is secure,sender build Auxiliary Security Header(ASH),
> insert it next to the standard MHR header with security enabled bit ON,
> and secure frames before transmitting them. According to the information
> carried within the ASH, recipient retrieves the right cryptographic key
> and correctly un-secure MAC frames.
> 
> The error scenario occurs on Linux using IEEE802154_SCF_SECLEVEL_ENC(4)
> security level when llsec_do_encrypt_unauth() function builds theses MAC
> frames incorrectly. On recipients these MAC frames are discarded,logging
> "got invalid frame" messages.
> 
> Acked-by: Stefan Schmidt <stefan@osg.samsung.com>
> Signed-off-by: Diogenes Pereira <dvnp@cesar.org.br>

While it shows me Acked-By it was missing so far.

To make it clear I am ok with this patch now.

Acked-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

> ---
>   net/mac802154/llsec.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/net/mac802154/llsec.c b/net/mac802154/llsec.c
> index 1e1c9b2..d9e7105 100644
> --- a/net/mac802154/llsec.c
> +++ b/net/mac802154/llsec.c
> @@ -623,13 +623,18 @@ llsec_do_encrypt_unauth(struct sk_buff *skb, const struct mac802154_llsec *sec,
>   	u8 iv[16];
>   	struct scatterlist src;
>   	SKCIPHER_REQUEST_ON_STACK(req, key->tfm0);
> -	int err;
> +	int err, datalen;
> +	unsigned char *data;
>   
>   	llsec_geniv(iv, sec->params.hwaddr, &hdr->sec);
> -	sg_init_one(&src, skb->data, skb->len);
> +	/* Compute data payload offset and data length */
> +	data = skb_mac_header(skb) + skb->mac_len;
> +	datalen = skb_tail_pointer(skb) - data;
> +	sg_init_one(&src, data, datalen);
> +
>   	skcipher_request_set_tfm(req, key->tfm0);
>   	skcipher_request_set_callback(req, 0, NULL, NULL);
> -	skcipher_request_set_crypt(req, &src, &src, skb->len, iv);
> +	skcipher_request_set_crypt(req, &src, &src, datalen, iv);
>   	err = crypto_skcipher_encrypt(req);
>   	skcipher_request_zero(req);
>   	return err;
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stefan Schmidt Sept. 20, 2017, 12:31 p.m. UTC | #4
Hello.

On 09/05/2017 02:18 PM, Diogenes Pereira wrote:
> According to  802.15.4-2003/2006/2015 specifications the MAC frame is
> composed of MHR, MAC payload and MFR and just the outgoing MAC payload
> must be encrypted.
> 
> If communication is secure,sender build Auxiliary Security Header(ASH),
> insert it next to the standard MHR header with security enabled bit ON,
> and secure frames before transmitting them. According to the information
> carried within the ASH, recipient retrieves the right cryptographic key
> and correctly un-secure MAC frames.
> 
> The error scenario occurs on Linux using IEEE802154_SCF_SECLEVEL_ENC(4)
> security level when llsec_do_encrypt_unauth() function builds theses MAC
> frames incorrectly. On recipients these MAC frames are discarded,logging
> "got invalid frame" messages.
> 
> Acked-by: Stefan Schmidt <stefan@osg.samsung.com>
> Signed-off-by: Diogenes Pereira <dvnp@cesar.org.br>
> ---
>  net/mac802154/llsec.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/net/mac802154/llsec.c b/net/mac802154/llsec.c
> index 1e1c9b2..d9e7105 100644
> --- a/net/mac802154/llsec.c
> +++ b/net/mac802154/llsec.c
> @@ -623,13 +623,18 @@ llsec_do_encrypt_unauth(struct sk_buff *skb, const struct mac802154_llsec *sec,
>  	u8 iv[16];
>  	struct scatterlist src;
>  	SKCIPHER_REQUEST_ON_STACK(req, key->tfm0);
> -	int err;
> +	int err, datalen;
> +	unsigned char *data;
>  
>  	llsec_geniv(iv, sec->params.hwaddr, &hdr->sec);
> -	sg_init_one(&src, skb->data, skb->len);
> +	/* Compute data payload offset and data length */
> +	data = skb_mac_header(skb) + skb->mac_len;
> +	datalen = skb_tail_pointer(skb) - data;
> +	sg_init_one(&src, data, datalen);
> +
>  	skcipher_request_set_tfm(req, key->tfm0);
>  	skcipher_request_set_callback(req, 0, NULL, NULL);
> -	skcipher_request_set_crypt(req, &src, &src, skb->len, iv);
> +	skcipher_request_set_crypt(req, &src, &src, datalen, iv);
>  	err = crypto_skcipher_encrypt(req);
>  	skcipher_request_zero(req);
>  	return err;
> 

Thanks! This patch has been applied to the wpan-next tree and will be part of the next pull request.

regards
Stefan Schmidt
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/mac802154/llsec.c b/net/mac802154/llsec.c
index 1e1c9b2..d9e7105 100644
--- a/net/mac802154/llsec.c
+++ b/net/mac802154/llsec.c
@@ -623,13 +623,18 @@  llsec_do_encrypt_unauth(struct sk_buff *skb, const struct mac802154_llsec *sec,
 	u8 iv[16];
 	struct scatterlist src;
 	SKCIPHER_REQUEST_ON_STACK(req, key->tfm0);
-	int err;
+	int err, datalen;
+	unsigned char *data;
 
 	llsec_geniv(iv, sec->params.hwaddr, &hdr->sec);
-	sg_init_one(&src, skb->data, skb->len);
+	/* Compute data payload offset and data length */
+	data = skb_mac_header(skb) + skb->mac_len;
+	datalen = skb_tail_pointer(skb) - data;
+	sg_init_one(&src, data, datalen);
+
 	skcipher_request_set_tfm(req, key->tfm0);
 	skcipher_request_set_callback(req, 0, NULL, NULL);
-	skcipher_request_set_crypt(req, &src, &src, skb->len, iv);
+	skcipher_request_set_crypt(req, &src, &src, datalen, iv);
 	err = crypto_skcipher_encrypt(req);
 	skcipher_request_zero(req);
 	return err;