All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v0 1/2] mac802154: fix header and payload encrypted
@ 2017-08-07 17:22 Diogenes Pereira
  2017-08-07 17:22 ` [PATCH v0 2/2] mac802154: replace hardcoded value with macro Diogenes Pereira
  2017-08-09 10:52 ` [PATCH v0 1/2] mac802154: fix header and payload encrypted Stefan Schmidt
  0 siblings, 2 replies; 14+ messages in thread
From: Diogenes Pereira @ 2017-08-07 17:22 UTC (permalink / raw)
  To: linux-wpan; +Cc: aar, stefan, Diogenes Pereira

According to 802.15.4-2015 specification (section 9.2.1 Outgoing frame
security procedure) just the outgoing payload is encrypted. The header
carries security parameters to destination address, so is not encrypted.

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..3c8ae3f 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);
+
+	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;
-- 
2.7.4


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

* [PATCH v0 2/2] mac802154: replace hardcoded value with macro
  2017-08-07 17:22 [PATCH v0 1/2] mac802154: fix header and payload encrypted Diogenes Pereira
@ 2017-08-07 17:22 ` Diogenes Pereira
  2017-08-09 10:47   ` Stefan Schmidt
  2017-08-09 16:19   ` [PATCH v1] " Diogenes Pereira
  2017-08-09 10:52 ` [PATCH v0 1/2] mac802154: fix header and payload encrypted Stefan Schmidt
  1 sibling, 2 replies; 14+ messages in thread
From: Diogenes Pereira @ 2017-08-07 17:22 UTC (permalink / raw)
  To: linux-wpan; +Cc: aar, stefan, Diogenes Pereira

Use IEEE802154_SCF_SECLEVEL_NONE macro defined at ieee802154.h file.

Signed-off-by: Diogenes Pereira <dvnp@cesar.org.br>
---
 net/mac802154/llsec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mac802154/llsec.c b/net/mac802154/llsec.c
index 3c8ae3f..f51093e 100644
--- a/net/mac802154/llsec.c
+++ b/net/mac802154/llsec.c
@@ -718,7 +718,8 @@ int mac802154_llsec_encrypt(struct mac802154_llsec *sec, struct sk_buff *skb)
 	if (hlen < 0 || hdr.fc.type != IEEE802154_FC_TYPE_DATA)
 		return -EINVAL;
 
-	if (!hdr.fc.security_enabled || hdr.sec.level == 0) {
+	if (!hdr.fc.security_enabled ||
+		hdr.sec.level == IEEE802154_SCF_SECLEVEL_NONE) {
 		skb_push(skb, hlen);
 		return 0;
 	}
-- 
2.7.4


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

* Re: [PATCH v0 2/2] mac802154: replace hardcoded value with macro
  2017-08-07 17:22 ` [PATCH v0 2/2] mac802154: replace hardcoded value with macro Diogenes Pereira
@ 2017-08-09 10:47   ` Stefan Schmidt
       [not found]     ` <CAC9NHkm=SknMqkY8MuaOuJsnnBWauFAzU_cjYttMxRh6N0MQuA@mail.gmail.com>
  2017-08-09 16:19   ` [PATCH v1] " Diogenes Pereira
  1 sibling, 1 reply; 14+ messages in thread
From: Stefan Schmidt @ 2017-08-09 10:47 UTC (permalink / raw)
  To: Diogenes Pereira, linux-wpan; +Cc: aar

Hello.

On 08/07/2017 07:22 PM, Diogenes Pereira wrote:
> Use IEEE802154_SCF_SECLEVEL_NONE macro defined at ieee802154.h file.
> 
> Signed-off-by: Diogenes Pereira <dvnp@cesar.org.br>
> ---
>   net/mac802154/llsec.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mac802154/llsec.c b/net/mac802154/llsec.c
> index 3c8ae3f..f51093e 100644
> --- a/net/mac802154/llsec.c
> +++ b/net/mac802154/llsec.c
> @@ -718,7 +718,8 @@ int mac802154_llsec_encrypt(struct mac802154_llsec *sec, struct sk_buff *skb)
>   	if (hlen < 0 || hdr.fc.type != IEEE802154_FC_TYPE_DATA)
>   		return -EINVAL;
>   
> -	if (!hdr.fc.security_enabled || hdr.sec.level == 0) {
> +	if (!hdr.fc.security_enabled ||
> +		hdr.sec.level == IEEE802154_SCF_SECLEVEL_NONE) {

Please align the start of the new line with the open parenthesis.

With this change you can add my

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

regards
Stefan Schmidt

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

* Re: [PATCH v0 1/2] mac802154: fix header and payload encrypted
  2017-08-07 17:22 [PATCH v0 1/2] mac802154: fix header and payload encrypted Diogenes Pereira
  2017-08-07 17:22 ` [PATCH v0 2/2] mac802154: replace hardcoded value with macro Diogenes Pereira
@ 2017-08-09 10:52 ` Stefan Schmidt
       [not found]   ` <CAC9NHkkHZJLk7_shZ37jFqp06Gu9Qg1Eu=sGi_259GjhkpSftg@mail.gmail.com>
  1 sibling, 1 reply; 14+ messages in thread
From: Stefan Schmidt @ 2017-08-09 10:52 UTC (permalink / raw)
  To: Diogenes Pereira, linux-wpan; +Cc: aar

Hello.

On 08/07/2017 07:22 PM, Diogenes Pereira wrote:
> According to 802.15.4-2015 specification (section 9.2.1 Outgoing frame
> security procedure) just the outgoing payload is encrypted. The header
> carries security parameters to destination address, so is not encrypted.

Did you check by any chance if that was different in the 2006 or 2003 
versions of the spec? A lot of our code is based on them and we are only 
very slowly catching up on -2015 :)

> 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..3c8ae3f 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);
> +
> +	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;

What systems did you test this against? Do you have a specific error 
scenario which this patch fixes?

What I try to understand here is if we are going to break llsec 
communication with other systems already out there.

regards
Stefan Schmidt

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

* Re: [PATCH v0 2/2] mac802154: replace hardcoded value with macro
       [not found]     ` <CAC9NHkm=SknMqkY8MuaOuJsnnBWauFAzU_cjYttMxRh6N0MQuA@mail.gmail.com>
@ 2017-08-09 11:17       ` Stefan Schmidt
  0 siblings, 0 replies; 14+ messages in thread
From: Stefan Schmidt @ 2017-08-09 11:17 UTC (permalink / raw)
  To: Diógenes Vila Nova Pereira; +Cc: linux-wpan

Hello.

On 08/09/2017 01:07 PM, Diógenes Vila Nova Pereira wrote:
> Hi Stefan,
> 
> I can fix this same patch file and send it again ?

Yes, please do so.

regards
Stefan Schmidt

> Rgds
> 
> *Saúde e Paz !
> []'s
> Diogenes Pereira*
> Senior Embedded Software Engineer
> 
> *E-mail: * dvnp@cesar.org.br <mailto:dvnp@cesar.org.br>
> *Celular:* +55 81 98895 0809
> *Comercial:* +55 81 3419 6706
> 
> <http://www.cesar.org.br/>
> Antes de imprimir, pense em sua responsabilidade e compromisso com o 
> MEIO AMBIENTE.
> 
> NOTICE: This transmittal and/or attachments is confidential. If you are 
> not the intended recipient, you are hereby notified that you have 
> received this transmittal in error and that any review, dissemination, 
> distribution or copying of this message is strictly prohibited. If you 
> have received this message in error, please notify us immediately and 
> immediately delete this message and all its attachments. Thank you.
> 
> On Wed, Aug 9, 2017 at 7:47 AM, Stefan Schmidt <stefan@osg.samsung.com 
> <mailto:stefan@osg.samsung.com>> wrote:
> 
>     Hello.
> 
>     On 08/07/2017 07:22 PM, Diogenes Pereira wrote:
> 
>         Use IEEE802154_SCF_SECLEVEL_NONE macro defined at ieee802154.h file.
> 
>         Signed-off-by: Diogenes Pereira <dvnp@cesar.org.br
>         <mailto:dvnp@cesar.org.br>>
>         ---
>            net/mac802154/llsec.c | 3 ++-
>            1 file changed, 2 insertions(+), 1 deletion(-)
> 
>         diff --git a/net/mac802154/llsec.c b/net/mac802154/llsec.c
>         index 3c8ae3f..f51093e 100644
>         --- a/net/mac802154/llsec.c
>         +++ b/net/mac802154/llsec.c
>         @@ -718,7 +718,8 @@ int mac802154_llsec_encrypt(struct
>         mac802154_llsec *sec, struct sk_buff *skb)
>                  if (hlen < 0 || hdr.fc.type != IEEE802154_FC_TYPE_DATA)
>                          return -EINVAL;
>            -     if (!hdr.fc.security_enabled || hdr.sec.level == 0) {
>         +       if (!hdr.fc.security_enabled ||
>         +               hdr.sec.level == IEEE802154_SCF_SECLEVEL_NONE) {
> 
> 
>     Please align the start of the new line with the open parenthesis.
> 
>     With this change you can add my
> 
>     Acked-by: Stefan Schmidt <stefan@osg.samsung.com
>     <mailto:stefan@osg.samsung.com>>
> 
>     regards
>     Stefan Schmidt
> 
> 

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

* Re: [PATCH v0 1/2] mac802154: fix header and payload encrypted
       [not found]   ` <CAC9NHkkHZJLk7_shZ37jFqp06Gu9Qg1Eu=sGi_259GjhkpSftg@mail.gmail.com>
@ 2017-08-09 13:26     ` Stefan Schmidt
  2017-09-05 12:18     ` [PATCH v1 1/2] mac802154: Fix MAC " Diogenes Pereira
  1 sibling, 0 replies; 14+ messages in thread
From: Stefan Schmidt @ 2017-08-09 13:26 UTC (permalink / raw)
  To: Diógenes Vila Nova Pereira; +Cc: linux-wpan

Hello.

On 08/09/2017 02:49 PM, Diógenes Vila Nova Pereira wrote:
> Hi Stefan,
> 
> So, can you give me time analyze the specifications and to tests at 
> other platforms no linux?

Sure, take all the time you need. Better have something well understood 
and tested.

You can submit the second patch separately so we can get it applied 
while you are looking into this one a bit further.

regards
Stefan Schmidt

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

* [PATCH v1] mac802154: replace hardcoded value with macro
  2017-08-07 17:22 ` [PATCH v0 2/2] mac802154: replace hardcoded value with macro Diogenes Pereira
  2017-08-09 10:47   ` Stefan Schmidt
@ 2017-08-09 16:19   ` Diogenes Pereira
  2017-08-21 14:46     ` Stefan Schmidt
  2017-09-20 12:31     ` Stefan Schmidt
  1 sibling, 2 replies; 14+ messages in thread
From: Diogenes Pereira @ 2017-08-09 16:19 UTC (permalink / raw)
  To: linux-wpan; +Cc: stefan, alex.aring, Diogenes Pereira

Use IEEE802154_SCF_SECLEVEL_NONE macro defined at ieee802154.h file.

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

diff --git a/net/mac802154/llsec.c b/net/mac802154/llsec.c
index 1e1c9b2..edec2f9 100644
--- a/net/mac802154/llsec.c
+++ b/net/mac802154/llsec.c
@@ -713,7 +713,8 @@ int mac802154_llsec_encrypt(struct mac802154_llsec *sec, struct sk_buff *skb)
 	if (hlen < 0 || hdr.fc.type != IEEE802154_FC_TYPE_DATA)
 		return -EINVAL;
 
-	if (!hdr.fc.security_enabled || hdr.sec.level == 0) {
+	if (!hdr.fc.security_enabled ||
+	    (hdr.sec.level == IEEE802154_SCF_SECLEVEL_NONE)) {
 		skb_push(skb, hlen);
 		return 0;
 	}
-- 
2.7.4


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

* Re: [PATCH v1] mac802154: replace hardcoded value with macro
  2017-08-09 16:19   ` [PATCH v1] " Diogenes Pereira
@ 2017-08-21 14:46     ` Stefan Schmidt
  2017-09-20 12:31     ` Stefan Schmidt
  1 sibling, 0 replies; 14+ messages in thread
From: Stefan Schmidt @ 2017-08-21 14:46 UTC (permalink / raw)
  To: Diogenes Pereira, linux-wpan; +Cc: alex.aring, Marcel Holtmann

Hello Marcel.

could you pick up this one through the bluetooth-next tree as well?

regards
Stefan Schmidt

On 08/09/2017 06:19 PM, Diogenes Pereira wrote:
> Use IEEE802154_SCF_SECLEVEL_NONE macro defined at ieee802154.h file.
> 
> Signed-off-by: Diogenes Pereira <dvnp@cesar.org.br>
> Acked-by: Stefan Schmidt <stefan@osg.samsung.com>
> ---
>   net/mac802154/llsec.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mac802154/llsec.c b/net/mac802154/llsec.c
> index 1e1c9b2..edec2f9 100644
> --- a/net/mac802154/llsec.c
> +++ b/net/mac802154/llsec.c
> @@ -713,7 +713,8 @@ int mac802154_llsec_encrypt(struct mac802154_llsec *sec, struct sk_buff *skb)
>   	if (hlen < 0 || hdr.fc.type != IEEE802154_FC_TYPE_DATA)
>   		return -EINVAL;
>   
> -	if (!hdr.fc.security_enabled || hdr.sec.level == 0) {
> +	if (!hdr.fc.security_enabled ||
> +	    (hdr.sec.level == IEEE802154_SCF_SECLEVEL_NONE)) {
>   		skb_push(skb, hlen);
>   		return 0;
>   	}
> 

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

* [PATCH v1 1/2] mac802154: Fix MAC header and payload encrypted
       [not found]   ` <CAC9NHkkHZJLk7_shZ37jFqp06Gu9Qg1Eu=sGi_259GjhkpSftg@mail.gmail.com>
  2017-08-09 13:26     ` Stefan Schmidt
@ 2017-09-05 12:18     ` Diogenes Pereira
  2017-09-11 11:21       ` Stefan Schmidt
                         ` (2 more replies)
  1 sibling, 3 replies; 14+ messages in thread
From: Diogenes Pereira @ 2017-09-05 12:18 UTC (permalink / raw)
  To: linux-wpan; +Cc: stefan, alex.aring, ckt, dvnp

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;
-- 
2.7.4


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

* Re: [PATCH v1 1/2] mac802154: Fix MAC header and payload encrypted
  2017-09-05 12:18     ` [PATCH v1 1/2] mac802154: Fix MAC " Diogenes Pereira
@ 2017-09-11 11:21       ` Stefan Schmidt
       [not found]         ` <CAC9NHknYZ7=FqwkXSHpXKQKBD74b7ddaDA=q=O5j-wkXhpMSJw@mail.gmail.com>
  2017-09-14 13:39       ` Stefan Schmidt
  2017-09-20 12:31       ` Stefan Schmidt
  2 siblings, 1 reply; 14+ messages in thread
From: Stefan Schmidt @ 2017-09-11 11:21 UTC (permalink / raw)
  To: Diogenes Pereira, linux-wpan; +Cc: alex.aring, ckt

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;
> 

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

* Re: [PATCH v1 1/2] mac802154: Fix MAC header and payload encrypted
       [not found]         ` <CAC9NHknYZ7=FqwkXSHpXKQKBD74b7ddaDA=q=O5j-wkXhpMSJw@mail.gmail.com>
@ 2017-09-14 13:35           ` Stefan Schmidt
  0 siblings, 0 replies; 14+ messages in thread
From: Stefan Schmidt @ 2017-09-14 13:35 UTC (permalink / raw)
  To: Diógenes Vila Nova Pereira
  Cc: linux-wpan, Alexander Aring, Claudio Koiti Takahasi

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



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

* Re: [PATCH v1 1/2] mac802154: Fix MAC header and payload encrypted
  2017-09-05 12:18     ` [PATCH v1 1/2] mac802154: Fix MAC " Diogenes Pereira
  2017-09-11 11:21       ` Stefan Schmidt
@ 2017-09-14 13:39       ` Stefan Schmidt
  2017-09-20 12:31       ` Stefan Schmidt
  2 siblings, 0 replies; 14+ messages in thread
From: Stefan Schmidt @ 2017-09-14 13:39 UTC (permalink / raw)
  To: Diogenes Pereira, linux-wpan; +Cc: alex.aring, ckt

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;
> 

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

* Re: [PATCH v1] mac802154: replace hardcoded value with macro
  2017-08-09 16:19   ` [PATCH v1] " Diogenes Pereira
  2017-08-21 14:46     ` Stefan Schmidt
@ 2017-09-20 12:31     ` Stefan Schmidt
  1 sibling, 0 replies; 14+ messages in thread
From: Stefan Schmidt @ 2017-09-20 12:31 UTC (permalink / raw)
  To: Diogenes Pereira, linux-wpan; +Cc: alex.aring

Hello.

On 08/09/2017 06:19 PM, Diogenes Pereira wrote:
> Use IEEE802154_SCF_SECLEVEL_NONE macro defined at ieee802154.h file.
> 
> Signed-off-by: Diogenes Pereira <dvnp@cesar.org.br>
> Acked-by: Stefan Schmidt <stefan@osg.samsung.com>
> ---
>  net/mac802154/llsec.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mac802154/llsec.c b/net/mac802154/llsec.c
> index 1e1c9b2..edec2f9 100644
> --- a/net/mac802154/llsec.c
> +++ b/net/mac802154/llsec.c
> @@ -713,7 +713,8 @@ int mac802154_llsec_encrypt(struct mac802154_llsec *sec, struct sk_buff *skb)
>  	if (hlen < 0 || hdr.fc.type != IEEE802154_FC_TYPE_DATA)
>  		return -EINVAL;
>  
> -	if (!hdr.fc.security_enabled || hdr.sec.level == 0) {
> +	if (!hdr.fc.security_enabled ||
> +	    (hdr.sec.level == IEEE802154_SCF_SECLEVEL_NONE)) {
>  		skb_push(skb, hlen);
>  		return 0;
>  	}
> 

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

regards
Stefan Schmidt

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

* Re: [PATCH v1 1/2] mac802154: Fix MAC header and payload encrypted
  2017-09-05 12:18     ` [PATCH v1 1/2] mac802154: Fix MAC " Diogenes Pereira
  2017-09-11 11:21       ` Stefan Schmidt
  2017-09-14 13:39       ` Stefan Schmidt
@ 2017-09-20 12:31       ` Stefan Schmidt
  2 siblings, 0 replies; 14+ messages in thread
From: Stefan Schmidt @ 2017-09-20 12:31 UTC (permalink / raw)
  To: Diogenes Pereira, linux-wpan; +Cc: alex.aring, ckt

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

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

end of thread, other threads:[~2017-09-20 12:31 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-07 17:22 [PATCH v0 1/2] mac802154: fix header and payload encrypted Diogenes Pereira
2017-08-07 17:22 ` [PATCH v0 2/2] mac802154: replace hardcoded value with macro Diogenes Pereira
2017-08-09 10:47   ` Stefan Schmidt
     [not found]     ` <CAC9NHkm=SknMqkY8MuaOuJsnnBWauFAzU_cjYttMxRh6N0MQuA@mail.gmail.com>
2017-08-09 11:17       ` Stefan Schmidt
2017-08-09 16:19   ` [PATCH v1] " Diogenes Pereira
2017-08-21 14:46     ` Stefan Schmidt
2017-09-20 12:31     ` Stefan Schmidt
2017-08-09 10:52 ` [PATCH v0 1/2] mac802154: fix header and payload encrypted Stefan Schmidt
     [not found]   ` <CAC9NHkkHZJLk7_shZ37jFqp06Gu9Qg1Eu=sGi_259GjhkpSftg@mail.gmail.com>
2017-08-09 13:26     ` Stefan Schmidt
2017-09-05 12:18     ` [PATCH v1 1/2] mac802154: Fix MAC " Diogenes Pereira
2017-09-11 11:21       ` Stefan Schmidt
     [not found]         ` <CAC9NHknYZ7=FqwkXSHpXKQKBD74b7ddaDA=q=O5j-wkXhpMSJw@mail.gmail.com>
2017-09-14 13:35           ` Stefan Schmidt
2017-09-14 13:39       ` Stefan Schmidt
2017-09-20 12:31       ` Stefan Schmidt

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.