Linux Security Modules development
 help / color / mirror / Atom feed
* [PATCH RFC] Smack: Fix handling of IPv4 traffic received by PF_INET6 sockets
       [not found] <CGME20180709121236eucas1p21b689ba0e2c53326ddd9d36b924530ad@eucas1p2.samsung.com>
@ 2018-07-09 12:12 ` Piotr Sawicki
  0 siblings, 0 replies; 11+ messages in thread
From: Piotr Sawicki @ 2018-07-09 12:12 UTC (permalink / raw)
  To: linux-security-module

A socket which has sk_family set to PF_INET6 is able to recevie not
only IPv6 but also IPv4 traffic (IPv4-mapped IPv6 addresses).

Prior to this patch, the smk_skb_to_addr_ipv6() could have been
called for socket buffers containing IPv4 packets, in result such
traffic was allowed.

Signed-off-by: Piotr Sawicki <p.sawicki2@partner.samsung.com>
---
  security/smack/smack_lsm.c | 7 ++++++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 19de675..a6b4c2e 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3924,15 +3924,20 @@ static int smack_socket_sock_rcv_skb(struct sock 
*sk, struct sk_buff *skb)
      struct smack_known *skp = NULL;
      int rc = 0;
      struct smk_audit_info ad;
+    u16 family = sk->sk_family;
  #ifdef CONFIG_AUDIT
      struct lsm_network_audit net;
  #endif
  #if IS_ENABLED(CONFIG_IPV6)
      struct sockaddr_in6 sadd;
      int proto;
+
+    if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP)) {
+        family = PF_INET;
+    }
  #endif /* CONFIG_IPV6 */

-    switch (sk->sk_family) {
+    switch (family) {
      case PF_INET:
  #ifdef CONFIG_SECURITY_SMACK_NETFILTER
          /*
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH RFC] Smack: Fix handling of IPv4 traffic received by PF_INET6 sockets
       [not found] <CGME20180709123728eucas1p2e80949d219441dced3b0ab58e73315ce@eucas1p2.samsung.com>
@ 2018-07-09 12:37 ` Piotr Sawicki
  2018-07-10  6:53   ` Piotr Sawicki
  0 siblings, 1 reply; 11+ messages in thread
From: Piotr Sawicki @ 2018-07-09 12:37 UTC (permalink / raw)
  To: linux-security-module

A socket which has sk_family set to PF_INET6 is able to recevie not
only IPv6 but also IPv4 traffic (IPv4-mapped IPv6 addresses).

Prior to this patch, the smk_skb_to_addr_ipv6() could have been
called for socket buffers containing IPv4 packets, in result such
traffic was allowed.

Signed-off-by: Piotr Sawicki <p.sawicki2@partner.samsung.com>
---
  security/smack/smack_lsm.c | 7 ++++++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 19de675..a6b4c2e 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3924,15 +3924,20 @@ static int smack_socket_sock_rcv_skb(struct sock 
*sk, struct sk_buff *skb)
      struct smack_known *skp = NULL;
      int rc = 0;
      struct smk_audit_info ad;
+    u16 family = sk->sk_family;
  #ifdef CONFIG_AUDIT
      struct lsm_network_audit net;
  #endif
  #if IS_ENABLED(CONFIG_IPV6)
      struct sockaddr_in6 sadd;
      int proto;
+
+    if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP)) {
+        family = PF_INET;
+    }
  #endif /* CONFIG_IPV6 */

-    switch (sk->sk_family) {
+    switch (family) {
      case PF_INET:
  #ifdef CONFIG_SECURITY_SMACK_NETFILTER
          /*
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH RFC] Smack: Fix handling of IPv4 traffic received by PF_INET6 sockets
  2018-07-09 12:37 ` Piotr Sawicki
@ 2018-07-10  6:53   ` Piotr Sawicki
  0 siblings, 0 replies; 11+ messages in thread
From: Piotr Sawicki @ 2018-07-10  6:53 UTC (permalink / raw)
  To: linux-security-module

A socket which has sk_family set to PF_INET6 is able to receive not
only IPv6 but also IPv4 traffic (IPv4-mapped IPv6 addresses).

Prior to this change, the smk_skb_to_addr_ipv6() could have been
called for socket buffers containing IPv4 packets.

Signed-off-by: Piotr Sawicki <p.sawicki2@partner.samsung.com>
---
 ?security/smack/smack_lsm.c | 12 ++++++++----
 ?1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 19de675..1315de4 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3924,15 +3924,19 @@ static int smack_socket_sock_rcv_skb(struct sock 
*sk, struct sk_buff *skb)
 ???? struct smack_known *skp = NULL;
 ???? int rc = 0;
 ???? struct smk_audit_info ad;
+??? u16 family = sk->sk_family;
 ?#ifdef CONFIG_AUDIT
 ???? struct lsm_network_audit net;
 ?#endif
 ?#if IS_ENABLED(CONFIG_IPV6)
 ???? struct sockaddr_in6 sadd;
 ???? int proto;
+
+??? if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP))
+??? ??? family = PF_INET;
 ?#endif /* CONFIG_IPV6 */

-??? switch (sk->sk_family) {
+??? switch (family) {
 ???? case PF_INET:
 ?#ifdef CONFIG_SECURITY_SMACK_NETFILTER
 ???? ??? /*
@@ -3950,7 +3954,7 @@ static int smack_socket_sock_rcv_skb(struct sock 
*sk, struct sk_buff *skb)
 ???? ??? ?*/
 ???? ??? netlbl_secattr_init(&secattr);

-??? ??? rc = netlbl_skbuff_getattr(skb, sk->sk_family, &secattr);
+??? ??? rc = netlbl_skbuff_getattr(skb, family, &secattr);
 ???? ??? if (rc == 0)
 ???? ??? ??? skp = smack_from_secattr(&secattr, ssp);
 ???? ??? else
@@ -3963,7 +3967,7 @@ static int smack_socket_sock_rcv_skb(struct sock 
*sk, struct sk_buff *skb)
 ?#endif
 ?#ifdef CONFIG_AUDIT
 ???? ??? smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
-??? ??? ad.a.u.net->family = sk->sk_family;
+??? ??? ad.a.u.net->family = family;
 ???? ??? ad.a.u.net->netif = skb->skb_iif;
 ???? ??? ipv4_skb_to_auditdata(skb, &ad.a, NULL);
 ?#endif
@@ -3977,7 +3981,7 @@ static int smack_socket_sock_rcv_skb(struct sock 
*sk, struct sk_buff *skb)
 ???? ??? rc = smk_bu_note("IPv4 delivery", skp, ssp->smk_in,
 ???? ??? ??? ??? ??? MAY_WRITE, rc);
 ???? ??? if (rc != 0)
-??? ??? ??? netlbl_skbuff_err(skb, sk->sk_family, rc, 0);
+??? ??? ??? netlbl_skbuff_err(skb, family, rc, 0);
 ???? ??? break;
 ?#if IS_ENABLED(CONFIG_IPV6)
 ???? case PF_INET6:
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH RFC] Smack: Fix handling of IPv4 traffic received by PF_INET6 sockets
       [not found] <CGME20180710070515eucas1p1dddc12cc44c820971809a242e8cee106@eucas1p1.samsung.com>
@ 2018-07-10  7:05 ` Piotr Sawicki
  2018-07-10 15:21   ` Casey Schaufler
  2018-07-16 23:33   ` Casey Schaufler
  0 siblings, 2 replies; 11+ messages in thread
From: Piotr Sawicki @ 2018-07-10  7:05 UTC (permalink / raw)
  To: linux-security-module

A socket which has sk_family set to PF_INET6 is able to receive not
only IPv6 but also IPv4 traffic (IPv4-mapped IPv6 addresses).

Prior to this patch, the smk_skb_to_addr_ipv6() could have been
called for socket buffers containing IPv4 packets, in result such
traffic was allowed.

Signed-off-by: Piotr Sawicki <p.sawicki2@partner.samsung.com>
---
  security/smack/smack_lsm.c | 12 ++++++++----
  1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 19de675..1315de4 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3924,15 +3924,19 @@ static int smack_socket_sock_rcv_skb(struct sock 
*sk, struct sk_buff *skb)
  	struct smack_known *skp = NULL;
  	int rc = 0;
  	struct smk_audit_info ad;
+	u16 family = sk->sk_family;
  #ifdef CONFIG_AUDIT
  	struct lsm_network_audit net;
  #endif
  #if IS_ENABLED(CONFIG_IPV6)
  	struct sockaddr_in6 sadd;
  	int proto;
+
+	if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP))
+		family = PF_INET;
  #endif /* CONFIG_IPV6 */

-	switch (sk->sk_family) {
+	switch (family) {
  	case PF_INET:
  #ifdef CONFIG_SECURITY_SMACK_NETFILTER
  		/*
@@ -3950,7 +3954,7 @@ static int smack_socket_sock_rcv_skb(struct sock 
*sk, struct sk_buff *skb)
  		 */
  		netlbl_secattr_init(&secattr);

-		rc = netlbl_skbuff_getattr(skb, sk->sk_family, &secattr);
+		rc = netlbl_skbuff_getattr(skb, family, &secattr);
  		if (rc == 0)
  			skp = smack_from_secattr(&secattr, ssp);
  		else
@@ -3963,7 +3967,7 @@ static int smack_socket_sock_rcv_skb(struct sock 
*sk, struct sk_buff *skb)
  #endif
  #ifdef CONFIG_AUDIT
  		smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
-		ad.a.u.net->family = sk->sk_family;
+		ad.a.u.net->family = family;
  		ad.a.u.net->netif = skb->skb_iif;
  		ipv4_skb_to_auditdata(skb, &ad.a, NULL);
  #endif
@@ -3977,7 +3981,7 @@ static int smack_socket_sock_rcv_skb(struct sock 
*sk, struct sk_buff *skb)
  		rc = smk_bu_note("IPv4 delivery", skp, ssp->smk_in,
  					MAY_WRITE, rc);
  		if (rc != 0)
-			netlbl_skbuff_err(skb, sk->sk_family, rc, 0);
+			netlbl_skbuff_err(skb, family, rc, 0);
  		break;
  #if IS_ENABLED(CONFIG_IPV6)
  	case PF_INET6:
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH RFC] Smack: Fix handling of IPv4 traffic received by PF_INET6 sockets
  2018-07-10  7:05 ` [PATCH RFC] Smack: Fix handling of IPv4 traffic received by PF_INET6 sockets Piotr Sawicki
@ 2018-07-10 15:21   ` Casey Schaufler
  2018-07-10 15:44     ` Piotr Sawicki
  2018-07-16 23:33   ` Casey Schaufler
  1 sibling, 1 reply; 11+ messages in thread
From: Casey Schaufler @ 2018-07-10 15:21 UTC (permalink / raw)
  To: linux-security-module

On 7/10/2018 12:05 AM, Piotr Sawicki wrote:
> A socket which has sk_family set to PF_INET6 is able to receive not
> only IPv6 but also IPv4 traffic (IPv4-mapped IPv6 addresses).
>
> Prior to this patch, the smk_skb_to_addr_ipv6() could have been
> called for socket buffers containing IPv4 packets, in result such
> traffic was allowed.
>
> Signed-off-by: Piotr Sawicki <p.sawicki2@partner.samsung.com>

Looks fine from here. Do you have a simple test case?

> ---
>   security/smack/smack_lsm.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 19de675..1315de4 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -3924,15 +3924,19 @@ static int smack_socket_sock_rcv_skb(struct sock 
> *sk, struct sk_buff *skb)
>   	struct smack_known *skp = NULL;
>   	int rc = 0;
>   	struct smk_audit_info ad;
> +	u16 family = sk->sk_family;
>   #ifdef CONFIG_AUDIT
>   	struct lsm_network_audit net;
>   #endif
>   #if IS_ENABLED(CONFIG_IPV6)
>   	struct sockaddr_in6 sadd;
>   	int proto;
> +
> +	if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP))
> +		family = PF_INET;
>   #endif /* CONFIG_IPV6 */
>
> -	switch (sk->sk_family) {
> +	switch (family) {
>   	case PF_INET:
>   #ifdef CONFIG_SECURITY_SMACK_NETFILTER
>   		/*
> @@ -3950,7 +3954,7 @@ static int smack_socket_sock_rcv_skb(struct sock 
> *sk, struct sk_buff *skb)
>   		 */
>   		netlbl_secattr_init(&secattr);
>
> -		rc = netlbl_skbuff_getattr(skb, sk->sk_family, &secattr);
> +		rc = netlbl_skbuff_getattr(skb, family, &secattr);
>   		if (rc == 0)
>   			skp = smack_from_secattr(&secattr, ssp);
>   		else
> @@ -3963,7 +3967,7 @@ static int smack_socket_sock_rcv_skb(struct sock 
> *sk, struct sk_buff *skb)
>   #endif
>   #ifdef CONFIG_AUDIT
>   		smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
> -		ad.a.u.net->family = sk->sk_family;
> +		ad.a.u.net->family = family;
>   		ad.a.u.net->netif = skb->skb_iif;
>   		ipv4_skb_to_auditdata(skb, &ad.a, NULL);
>   #endif
> @@ -3977,7 +3981,7 @@ static int smack_socket_sock_rcv_skb(struct sock 
> *sk, struct sk_buff *skb)
>   		rc = smk_bu_note("IPv4 delivery", skp, ssp->smk_in,
>   					MAY_WRITE, rc);
>   		if (rc != 0)
> -			netlbl_skbuff_err(skb, sk->sk_family, rc, 0);
> +			netlbl_skbuff_err(skb, family, rc, 0);
>   		break;
>   #if IS_ENABLED(CONFIG_IPV6)
>   	case PF_INET6:

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH RFC] Smack: Fix handling of IPv4 traffic received by PF_INET6 sockets
  2018-07-10 15:21   ` Casey Schaufler
@ 2018-07-10 15:44     ` Piotr Sawicki
  2018-07-10 16:16       ` Casey Schaufler
  0 siblings, 1 reply; 11+ messages in thread
From: Piotr Sawicki @ 2018-07-10 15:44 UTC (permalink / raw)
  To: linux-security-module

On 07/10/2018 05:21 PM, Casey Schaufler wrote:
> On 7/10/2018 12:05 AM, Piotr Sawicki wrote:
>> A socket which has sk_family set to PF_INET6 is able to receive not
>> only IPv6 but also IPv4 traffic (IPv4-mapped IPv6 addresses).
>>
>> Prior to this patch, the smk_skb_to_addr_ipv6() could have been
>> called for socket buffers containing IPv4 packets, in result such
>> traffic was allowed.
>>
>> Signed-off-by: Piotr Sawicki <p.sawicki2@partner.samsung.com>
> 
> Looks fine from here. Do you have a simple test case?


Yes, I've tested it on Tizen emulator.

Tests are available here 
https://review.tizen.org/gerrit/#/admin/projects/platform/core/test/security-tests, 
branch nether.

I'm really sorry for this mess with multiple emails.


>> ---
>>    security/smack/smack_lsm.c | 12 ++++++++----
>>    1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index 19de675..1315de4 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -3924,15 +3924,19 @@ static int smack_socket_sock_rcv_skb(struct sock
>> *sk, struct sk_buff *skb)
>>    	struct smack_known *skp = NULL;
>>    	int rc = 0;
>>    	struct smk_audit_info ad;
>> +	u16 family = sk->sk_family;
>>    #ifdef CONFIG_AUDIT
>>    	struct lsm_network_audit net;
>>    #endif
>>    #if IS_ENABLED(CONFIG_IPV6)
>>    	struct sockaddr_in6 sadd;
>>    	int proto;
>> +
>> +	if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP))
>> +		family = PF_INET;
>>    #endif /* CONFIG_IPV6 */
>>
>> -	switch (sk->sk_family) {
>> +	switch (family) {
>>    	case PF_INET:
>>    #ifdef CONFIG_SECURITY_SMACK_NETFILTER
>>    		/*
>> @@ -3950,7 +3954,7 @@ static int smack_socket_sock_rcv_skb(struct sock
>> *sk, struct sk_buff *skb)
>>    		 */
>>    		netlbl_secattr_init(&secattr);
>>
>> -		rc = netlbl_skbuff_getattr(skb, sk->sk_family, &secattr);
>> +		rc = netlbl_skbuff_getattr(skb, family, &secattr);
>>    		if (rc == 0)
>>    			skp = smack_from_secattr(&secattr, ssp);
>>    		else
>> @@ -3963,7 +3967,7 @@ static int smack_socket_sock_rcv_skb(struct sock
>> *sk, struct sk_buff *skb)
>>    #endif
>>    #ifdef CONFIG_AUDIT
>>    		smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
>> -		ad.a.u.net->family = sk->sk_family;
>> +		ad.a.u.net->family = family;
>>    		ad.a.u.net->netif = skb->skb_iif;
>>    		ipv4_skb_to_auditdata(skb, &ad.a, NULL);
>>    #endif
>> @@ -3977,7 +3981,7 @@ static int smack_socket_sock_rcv_skb(struct sock
>> *sk, struct sk_buff *skb)
>>    		rc = smk_bu_note("IPv4 delivery", skp, ssp->smk_in,
>>    					MAY_WRITE, rc);
>>    		if (rc != 0)
>> -			netlbl_skbuff_err(skb, sk->sk_family, rc, 0);
>> +			netlbl_skbuff_err(skb, family, rc, 0);
>>    		break;
>>    #if IS_ENABLED(CONFIG_IPV6)
>>    	case PF_INET6:
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH RFC] Smack: Fix handling of IPv4 traffic received by PF_INET6 sockets
  2018-07-10 15:44     ` Piotr Sawicki
@ 2018-07-10 16:16       ` Casey Schaufler
  2018-07-11  7:38         ` Piotr Sawicki
  0 siblings, 1 reply; 11+ messages in thread
From: Casey Schaufler @ 2018-07-10 16:16 UTC (permalink / raw)
  To: linux-security-module

On 7/10/2018 8:44 AM, Piotr Sawicki wrote:
> On 07/10/2018 05:21 PM, Casey Schaufler wrote:
>> On 7/10/2018 12:05 AM, Piotr Sawicki wrote:
>>> A socket which has sk_family set to PF_INET6 is able to receive not
>>> only IPv6 but also IPv4 traffic (IPv4-mapped IPv6 addresses).
>>>
>>> Prior to this patch, the smk_skb_to_addr_ipv6() could have been
>>> called for socket buffers containing IPv4 packets, in result such
>>> traffic was allowed.
>>>
>>> Signed-off-by: Piotr Sawicki <p.sawicki2@partner.samsung.com>
>> Looks fine from here. Do you have a simple test case?
>
> Yes, I've tested it on Tizen emulator.
>
> Tests are available here 
> https://review.tizen.org/gerrit/#/admin/projects/platform/core/test/security-tests, 
> branch nether.

Can you identify the specific test? There's quite a bit to look through.
Thank you.

>
> I'm really sorry for this mess with multiple emails.
>
>
>>> ---
>>>    security/smack/smack_lsm.c | 12 ++++++++----
>>>    1 file changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>>> index 19de675..1315de4 100644
>>> --- a/security/smack/smack_lsm.c
>>> +++ b/security/smack/smack_lsm.c
>>> @@ -3924,15 +3924,19 @@ static int smack_socket_sock_rcv_skb(struct sock
>>> *sk, struct sk_buff *skb)
>>>    	struct smack_known *skp = NULL;
>>>    	int rc = 0;
>>>    	struct smk_audit_info ad;
>>> +	u16 family = sk->sk_family;
>>>    #ifdef CONFIG_AUDIT
>>>    	struct lsm_network_audit net;
>>>    #endif
>>>    #if IS_ENABLED(CONFIG_IPV6)
>>>    	struct sockaddr_in6 sadd;
>>>    	int proto;
>>> +
>>> +	if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP))
>>> +		family = PF_INET;
>>>    #endif /* CONFIG_IPV6 */
>>>
>>> -	switch (sk->sk_family) {
>>> +	switch (family) {
>>>    	case PF_INET:
>>>    #ifdef CONFIG_SECURITY_SMACK_NETFILTER
>>>    		/*
>>> @@ -3950,7 +3954,7 @@ static int smack_socket_sock_rcv_skb(struct sock
>>> *sk, struct sk_buff *skb)
>>>    		 */
>>>    		netlbl_secattr_init(&secattr);
>>>
>>> -		rc = netlbl_skbuff_getattr(skb, sk->sk_family, &secattr);
>>> +		rc = netlbl_skbuff_getattr(skb, family, &secattr);
>>>    		if (rc == 0)
>>>    			skp = smack_from_secattr(&secattr, ssp);
>>>    		else
>>> @@ -3963,7 +3967,7 @@ static int smack_socket_sock_rcv_skb(struct sock
>>> *sk, struct sk_buff *skb)
>>>    #endif
>>>    #ifdef CONFIG_AUDIT
>>>    		smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
>>> -		ad.a.u.net->family = sk->sk_family;
>>> +		ad.a.u.net->family = family;
>>>    		ad.a.u.net->netif = skb->skb_iif;
>>>    		ipv4_skb_to_auditdata(skb, &ad.a, NULL);
>>>    #endif
>>> @@ -3977,7 +3981,7 @@ static int smack_socket_sock_rcv_skb(struct sock
>>> *sk, struct sk_buff *skb)
>>>    		rc = smk_bu_note("IPv4 delivery", skp, ssp->smk_in,
>>>    					MAY_WRITE, rc);
>>>    		if (rc != 0)
>>> -			netlbl_skbuff_err(skb, sk->sk_family, rc, 0);
>>> +			netlbl_skbuff_err(skb, family, rc, 0);
>>>    		break;
>>>    #if IS_ENABLED(CONFIG_IPV6)
>>>    	case PF_INET6:
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH RFC] Smack: Fix handling of IPv4 traffic received by PF_INET6 sockets
  2018-07-10 16:16       ` Casey Schaufler
@ 2018-07-11  7:38         ` Piotr Sawicki
  0 siblings, 0 replies; 11+ messages in thread
From: Piotr Sawicki @ 2018-07-11  7:38 UTC (permalink / raw)
  To: linux-security-module



On 07/10/2018 06:16 PM, Casey Schaufler wrote:
> On 7/10/2018 8:44 AM, Piotr Sawicki wrote:
>> On 07/10/2018 05:21 PM, Casey Schaufler wrote:
>>> On 7/10/2018 12:05 AM, Piotr Sawicki wrote:
>>>> A socket which has sk_family set to PF_INET6 is able to receive not
>>>> only IPv6 but also IPv4 traffic (IPv4-mapped IPv6 addresses).
>>>>
>>>> Prior to this patch, the smk_skb_to_addr_ipv6() could have been
>>>> called for socket buffers containing IPv4 packets, in result such
>>>> traffic was allowed.
>>>>
>>>> Signed-off-by: Piotr Sawicki <p.sawicki2@partner.samsung.com>
>>> Looks fine from here. Do you have a simple test case?
>>
>> Yes, I've tested it on Tizen emulator.
>>
>> Tests are available here
>> https://review.tizen.org/gerrit/#/admin/projects/platform/core/test/security-tests,
>> branch nether.
> 
> Can you identify the specific test? There's quite a bit to look through.
> Thank you.

ID:NETHER_IPV6_IPV4_LOCAL_INTER_APP_CONNECTION:nether_check_ipv6_srv_ipv4_udp_local_inter_app_connection_internet_access_granted
ID:NETHER_IPV6_IPV4_LOCAL_INTER_APP_CONNECTION:nether_check_ipv6_srv_ipv4_udp_local_inter_app_connection_internet_access_denied

It's a new thing, so tests are still in review: 
https://review.tizen.org/gerrit/#/c/183458/

To run the tests on the emulator you need to apply also these patches:
(Nether) https://review.tizen.org/gerrit/#/c/183464/ and turn on IPv6 
connection tracking in the kernel. I've used a kernel downloaded from 
https://review.tizen.org/gerrit/#/admin/projects/sdk/emulator/emulator-kernel 
branch tizen.

I know that these tests are Tizen specific, and it might be really time 
consuming to prepare an environment for them. So, I propose to just use 
the netcat tool and run one of its instances as a UDP server listening 
on IPv6 address [::] and the second one as a IPV4 client.

e.g.
server: nc -6 -l -u 20000
client: nc -u 127.0.0.1 20000

In case of TCP connections, access is properly checked in 
smack_inet_conn_request(). But when IPv4-mapped IPv6 addresses are used 
and two processes have write access to each other, 
smack_socket_sock_rcv_skb() will call smk_skb_to_addr_ipv6() function 
for IPv4 packets.





--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH RFC] Smack: Fix handling of IPv4 traffic received by PF_INET6 sockets
  2018-07-10  7:05 ` [PATCH RFC] Smack: Fix handling of IPv4 traffic received by PF_INET6 sockets Piotr Sawicki
  2018-07-10 15:21   ` Casey Schaufler
@ 2018-07-16 23:33   ` Casey Schaufler
  2018-07-17 10:28     ` Piotr Sawicki
  1 sibling, 1 reply; 11+ messages in thread
From: Casey Schaufler @ 2018-07-16 23:33 UTC (permalink / raw)
  To: linux-security-module

On 7/10/2018 12:05 AM, Piotr Sawicki wrote:
> A socket which has sk_family set to PF_INET6 is able to receive not
> only IPv6 but also IPv4 traffic (IPv4-mapped IPv6 addresses).
>
> Prior to this patch, the smk_skb_to_addr_ipv6() could have been
> called for socket buffers containing IPv4 packets, in result such
> traffic was allowed.
>
> Signed-off-by: Piotr Sawicki <p.sawicki2@partner.samsung.com>

Acked-by: Casey Schaufler <casey@schaufler-ca.com

I have added this to git://github.com/cschaufler/next-smack#smack-for-4.19
after doing a little whitespace clean-up and removal of unnecessary braces.

> ---
>   security/smack/smack_lsm.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 19de675..1315de4 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -3924,15 +3924,19 @@ static int smack_socket_sock_rcv_skb(struct sock 
> *sk, struct sk_buff *skb)
>   	struct smack_known *skp = NULL;
>   	int rc = 0;
>   	struct smk_audit_info ad;
> +	u16 family = sk->sk_family;
>   #ifdef CONFIG_AUDIT
>   	struct lsm_network_audit net;
>   #endif
>   #if IS_ENABLED(CONFIG_IPV6)
>   	struct sockaddr_in6 sadd;
>   	int proto;
> +
> +	if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP))
> +		family = PF_INET;
>   #endif /* CONFIG_IPV6 */
>
> -	switch (sk->sk_family) {
> +	switch (family) {
>   	case PF_INET:
>   #ifdef CONFIG_SECURITY_SMACK_NETFILTER
>   		/*
> @@ -3950,7 +3954,7 @@ static int smack_socket_sock_rcv_skb(struct sock 
> *sk, struct sk_buff *skb)
>   		 */
>   		netlbl_secattr_init(&secattr);
>
> -		rc = netlbl_skbuff_getattr(skb, sk->sk_family, &secattr);
> +		rc = netlbl_skbuff_getattr(skb, family, &secattr);
>   		if (rc == 0)
>   			skp = smack_from_secattr(&secattr, ssp);
>   		else
> @@ -3963,7 +3967,7 @@ static int smack_socket_sock_rcv_skb(struct sock 
> *sk, struct sk_buff *skb)
>   #endif
>   #ifdef CONFIG_AUDIT
>   		smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
> -		ad.a.u.net->family = sk->sk_family;
> +		ad.a.u.net->family = family;
>   		ad.a.u.net->netif = skb->skb_iif;
>   		ipv4_skb_to_auditdata(skb, &ad.a, NULL);
>   #endif
> @@ -3977,7 +3981,7 @@ static int smack_socket_sock_rcv_skb(struct sock 
> *sk, struct sk_buff *skb)
>   		rc = smk_bu_note("IPv4 delivery", skp, ssp->smk_in,
>   					MAY_WRITE, rc);
>   		if (rc != 0)
> -			netlbl_skbuff_err(skb, sk->sk_family, rc, 0);
> +			netlbl_skbuff_err(skb, family, rc, 0);
>   		break;
>   #if IS_ENABLED(CONFIG_IPV6)
>   	case PF_INET6:

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH RFC] Smack: Fix handling of IPv4 traffic received by PF_INET6 sockets
  2018-07-16 23:33   ` Casey Schaufler
@ 2018-07-17 10:28     ` Piotr Sawicki
  2018-07-17 15:01       ` Casey Schaufler
  0 siblings, 1 reply; 11+ messages in thread
From: Piotr Sawicki @ 2018-07-17 10:28 UTC (permalink / raw)
  To: linux-security-module



On 07/17/2018 01:33 AM, Casey Schaufler wrote:
> On 7/10/2018 12:05 AM, Piotr Sawicki wrote:
>> A socket which has sk_family set to PF_INET6 is able to receive not
>> only IPv6 but also IPv4 traffic (IPv4-mapped IPv6 addresses).
>>
>> Prior to this patch, the smk_skb_to_addr_ipv6() could have been
>> called for socket buffers containing IPv4 packets, in result such
>> traffic was allowed.
>>
>> Signed-off-by: Piotr Sawicki <p.sawicki2@partner.samsung.com>
> 
> Acked-by: Casey Schaufler <casey@schaufler-ca.com
> 
> I have added this to git://github.com/cschaufler/next-smack#smack-for-4.19
> after doing a little whitespace clean-up and removal of unnecessary braces.

I see that the wrong version of the patch was applied. It should be a 
version taken from the email titled "[PATCH RFC] Smack: Fix handling of 
IPv4 traffic received by PF_INET6 sockets" (10th of July 2018). The 
family variable should be used also in netlbl_skbuff_getattr(), 
netlbl_skbuff_err and in audit structure.


>> ---
>>    security/smack/smack_lsm.c | 12 ++++++++----
>>    1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index 19de675..1315de4 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -3924,15 +3924,19 @@ static int smack_socket_sock_rcv_skb(struct sock
>> *sk, struct sk_buff *skb)
>>    	struct smack_known *skp = NULL;
>>    	int rc = 0;
>>    	struct smk_audit_info ad;
>> +	u16 family = sk->sk_family;
>>    #ifdef CONFIG_AUDIT
>>    	struct lsm_network_audit net;
>>    #endif
>>    #if IS_ENABLED(CONFIG_IPV6)
>>    	struct sockaddr_in6 sadd;
>>    	int proto;
>> +
>> +	if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP))
>> +		family = PF_INET;
>>    #endif /* CONFIG_IPV6 */
>>
>> -	switch (sk->sk_family) {
>> +	switch (family) {
>>    	case PF_INET:
>>    #ifdef CONFIG_SECURITY_SMACK_NETFILTER
>>    		/*
>> @@ -3950,7 +3954,7 @@ static int smack_socket_sock_rcv_skb(struct sock
>> *sk, struct sk_buff *skb)
>>    		 */
>>    		netlbl_secattr_init(&secattr);
>>
>> -		rc = netlbl_skbuff_getattr(skb, sk->sk_family, &secattr);
>> +		rc = netlbl_skbuff_getattr(skb, family, &secattr);
>>    		if (rc == 0)
>>    			skp = smack_from_secattr(&secattr, ssp);
>>    		else
>> @@ -3963,7 +3967,7 @@ static int smack_socket_sock_rcv_skb(struct sock
>> *sk, struct sk_buff *skb)
>>    #endif
>>    #ifdef CONFIG_AUDIT
>>    		smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
>> -		ad.a.u.net->family = sk->sk_family;
>> +		ad.a.u.net->family = family;
>>    		ad.a.u.net->netif = skb->skb_iif;
>>    		ipv4_skb_to_auditdata(skb, &ad.a, NULL);
>>    #endif
>> @@ -3977,7 +3981,7 @@ static int smack_socket_sock_rcv_skb(struct sock
>> *sk, struct sk_buff *skb)
>>    		rc = smk_bu_note("IPv4 delivery", skp, ssp->smk_in,
>>    					MAY_WRITE, rc);
>>    		if (rc != 0)
>> -			netlbl_skbuff_err(skb, sk->sk_family, rc, 0);
>> +			netlbl_skbuff_err(skb, family, rc, 0);
>>    		break;
>>    #if IS_ENABLED(CONFIG_IPV6)
>>    	case PF_INET6:
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH RFC] Smack: Fix handling of IPv4 traffic received by PF_INET6 sockets
  2018-07-17 10:28     ` Piotr Sawicki
@ 2018-07-17 15:01       ` Casey Schaufler
  0 siblings, 0 replies; 11+ messages in thread
From: Casey Schaufler @ 2018-07-17 15:01 UTC (permalink / raw)
  To: linux-security-module

On 7/17/2018 3:28 AM, Piotr Sawicki wrote:
>
> On 07/17/2018 01:33 AM, Casey Schaufler wrote:
>> On 7/10/2018 12:05 AM, Piotr Sawicki wrote:
>>> A socket which has sk_family set to PF_INET6 is able to receive not
>>> only IPv6 but also IPv4 traffic (IPv4-mapped IPv6 addresses).
>>>
>>> Prior to this patch, the smk_skb_to_addr_ipv6() could have been
>>> called for socket buffers containing IPv4 packets, in result such
>>> traffic was allowed.
>>>
>>> Signed-off-by: Piotr Sawicki <p.sawicki2@partner.samsung.com>
>> Acked-by: Casey Schaufler <casey@schaufler-ca.com
>>
>> I have added this to git://github.com/cschaufler/next-smack#smack-for-4.19
>> after doing a little whitespace clean-up and removal of unnecessary braces.
> I see that the wrong version of the patch was applied. It should be a 
> version taken from the email titled "[PATCH RFC] Smack: Fix handling of 
> IPv4 traffic received by PF_INET6 sockets" (10th of July 2018). The 
> family variable should be used also in netlbl_skbuff_getattr(), 
> netlbl_skbuff_err and in audit structure.

Please resend the correct version with a "v2". Thank you.

>
>
>>> ---
>>>    security/smack/smack_lsm.c | 12 ++++++++----
>>>    1 file changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>>> index 19de675..1315de4 100644
>>> --- a/security/smack/smack_lsm.c
>>> +++ b/security/smack/smack_lsm.c
>>> @@ -3924,15 +3924,19 @@ static int smack_socket_sock_rcv_skb(struct sock
>>> *sk, struct sk_buff *skb)
>>>    	struct smack_known *skp = NULL;
>>>    	int rc = 0;
>>>    	struct smk_audit_info ad;
>>> +	u16 family = sk->sk_family;
>>>    #ifdef CONFIG_AUDIT
>>>    	struct lsm_network_audit net;
>>>    #endif
>>>    #if IS_ENABLED(CONFIG_IPV6)
>>>    	struct sockaddr_in6 sadd;
>>>    	int proto;
>>> +
>>> +	if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP))
>>> +		family = PF_INET;
>>>    #endif /* CONFIG_IPV6 */
>>>
>>> -	switch (sk->sk_family) {
>>> +	switch (family) {
>>>    	case PF_INET:
>>>    #ifdef CONFIG_SECURITY_SMACK_NETFILTER
>>>    		/*
>>> @@ -3950,7 +3954,7 @@ static int smack_socket_sock_rcv_skb(struct sock
>>> *sk, struct sk_buff *skb)
>>>    		 */
>>>    		netlbl_secattr_init(&secattr);
>>>
>>> -		rc = netlbl_skbuff_getattr(skb, sk->sk_family, &secattr);
>>> +		rc = netlbl_skbuff_getattr(skb, family, &secattr);
>>>    		if (rc == 0)
>>>    			skp = smack_from_secattr(&secattr, ssp);
>>>    		else
>>> @@ -3963,7 +3967,7 @@ static int smack_socket_sock_rcv_skb(struct sock
>>> *sk, struct sk_buff *skb)
>>>    #endif
>>>    #ifdef CONFIG_AUDIT
>>>    		smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net);
>>> -		ad.a.u.net->family = sk->sk_family;
>>> +		ad.a.u.net->family = family;
>>>    		ad.a.u.net->netif = skb->skb_iif;
>>>    		ipv4_skb_to_auditdata(skb, &ad.a, NULL);
>>>    #endif
>>> @@ -3977,7 +3981,7 @@ static int smack_socket_sock_rcv_skb(struct sock
>>> *sk, struct sk_buff *skb)
>>>    		rc = smk_bu_note("IPv4 delivery", skp, ssp->smk_in,
>>>    					MAY_WRITE, rc);
>>>    		if (rc != 0)
>>> -			netlbl_skbuff_err(skb, sk->sk_family, rc, 0);
>>> +			netlbl_skbuff_err(skb, family, rc, 0);
>>>    		break;
>>>    #if IS_ENABLED(CONFIG_IPV6)
>>>    	case PF_INET6:
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2018-07-17 15:01 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20180710070515eucas1p1dddc12cc44c820971809a242e8cee106@eucas1p1.samsung.com>
2018-07-10  7:05 ` [PATCH RFC] Smack: Fix handling of IPv4 traffic received by PF_INET6 sockets Piotr Sawicki
2018-07-10 15:21   ` Casey Schaufler
2018-07-10 15:44     ` Piotr Sawicki
2018-07-10 16:16       ` Casey Schaufler
2018-07-11  7:38         ` Piotr Sawicki
2018-07-16 23:33   ` Casey Schaufler
2018-07-17 10:28     ` Piotr Sawicki
2018-07-17 15:01       ` Casey Schaufler
     [not found] <CGME20180709123728eucas1p2e80949d219441dced3b0ab58e73315ce@eucas1p2.samsung.com>
2018-07-09 12:37 ` Piotr Sawicki
2018-07-10  6:53   ` Piotr Sawicki
     [not found] <CGME20180709121236eucas1p21b689ba0e2c53326ddd9d36b924530ad@eucas1p2.samsung.com>
2018-07-09 12:12 ` Piotr Sawicki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox