* [RFC PATCH 4/5] netlabel: Add SCTP support
@ 2017-10-17 13:58 ` Richard Haines
0 siblings, 0 replies; 12+ messages in thread
From: Richard Haines @ 2017-10-17 13:58 UTC (permalink / raw)
To: selinux, netdev, linux-sctp, linux-security-module
Cc: paul, vyasevich, nhorman, sds, eparis, marcelo.leitner, Richard Haines
Add support to label SCTP associations and cater for a situation where
family = PF_INET6 with an ip_hdr(skb)->version = 4.
Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
---
include/net/netlabel.h | 3 ++
net/netlabel/netlabel_kapi.c | 80 +++++++++++++++++++++++++++++++++++++++
net/netlabel/netlabel_unlabeled.c | 10 +++++
3 files changed, 93 insertions(+)
diff --git a/include/net/netlabel.h b/include/net/netlabel.h
index 72d6435..7348966 100644
--- a/include/net/netlabel.h
+++ b/include/net/netlabel.h
@@ -494,6 +494,9 @@ int netlbl_conn_setattr(struct sock *sk,
const struct netlbl_lsm_secattr *secattr);
int netlbl_req_setattr(struct request_sock *req,
const struct netlbl_lsm_secattr *secattr);
+int netlbl_sctp_setattr(struct sock *sk,
+ struct sk_buff *skb,
+ const struct netlbl_lsm_secattr *secattr);
void netlbl_req_delattr(struct request_sock *req);
int netlbl_skbuff_setattr(struct sk_buff *skb,
u16 family,
diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index ea7c670..1c82bbe 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -1121,6 +1121,7 @@ int netlbl_conn_setattr(struct sock *sk,
switch (addr->sa_family) {
case AF_INET:
addr4 = (struct sockaddr_in *)addr;
+
entry = netlbl_domhsh_getentry_af4(secattr->domain,
addr4->sin_addr.s_addr);
if (entry == NULL) {
@@ -1177,6 +1178,85 @@ int netlbl_conn_setattr(struct sock *sk,
}
/**
+ * netlbl_sctp_setattr - Label an incoming sctp association socket using
+ * the correct protocol
+ * @sk: the socket to label
+ * @skb: the packet
+ * @secattr: the security attributes
+ *
+ * Description:
+ * Attach the correct label to the given socket using the security attributes
+ * specified in @secattr. Returns zero on success, negative values on failure.
+ *
+ */
+int netlbl_sctp_setattr(struct sock *sk,
+ struct sk_buff *skb,
+ const struct netlbl_lsm_secattr *secattr)
+{
+ int ret_val = -EINVAL;
+ struct netlbl_dommap_def *entry;
+ struct iphdr *hdr4;
+#if IS_ENABLED(CONFIG_IPV6)
+ struct ipv6hdr *hdr6;
+#endif
+
+ rcu_read_lock();
+ switch (sk->sk_family) {
+ case AF_INET:
+ hdr4 = ip_hdr(skb);
+
+ entry = netlbl_domhsh_getentry_af4(secattr->domain,
+ hdr4->saddr);
+ if (entry == NULL) {
+ ret_val = -ENOENT;
+ goto sctp_setattr_return;
+ }
+ switch (entry->type) {
+ case NETLBL_NLTYPE_CIPSOV4:
+ ret_val = cipso_v4_sock_setattr(sk, entry->cipso,
+ secattr);
+ break;
+ case NETLBL_NLTYPE_UNLABELED:
+ netlbl_sock_delattr(sk);
+ ret_val = 0;
+ break;
+ default:
+ ret_val = -ENOENT;
+ }
+ break;
+#if IS_ENABLED(CONFIG_IPV6)
+ case AF_INET6:
+ hdr6 = ipv6_hdr(skb);
+ entry = netlbl_domhsh_getentry_af6(secattr->domain,
+ &hdr6->saddr);
+ if (entry == NULL) {
+ ret_val = -ENOENT;
+ goto sctp_setattr_return;
+ }
+ switch (entry->type) {
+ case NETLBL_NLTYPE_CALIPSO:
+ ret_val = calipso_sock_setattr(sk, entry->calipso,
+ secattr);
+ break;
+ case NETLBL_NLTYPE_UNLABELED:
+ netlbl_sock_delattr(sk);
+ ret_val = 0;
+ break;
+ default:
+ ret_val = -ENOENT;
+ }
+ break;
+#endif /* IPv6 */
+ default:
+ ret_val = -EPROTONOSUPPORT;
+ }
+
+sctp_setattr_return:
+ rcu_read_unlock();
+ return ret_val;
+}
+
+/**
* netlbl_req_setattr - Label a request socket using the correct protocol
* @req: the request socket to label
* @secattr: the security attributes
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index 22dc1b9..c070dfc 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -1472,6 +1472,16 @@ int netlbl_unlabel_getattr(const struct sk_buff *skb,
iface = rcu_dereference(netlbl_unlhsh_def);
if (iface == NULL || !iface->valid)
goto unlabel_getattr_nolabel;
+
+#if IS_ENABLED(CONFIG_IPV6)
+ /* When resolving a fallback label, check the sk_buff version as
+ * it is possible (e.g. SCTP) to have family = PF_INET6 while
+ * receiving ip_hdr(skb)->version = 4.
+ */
+ if (family == PF_INET6 && ip_hdr(skb)->version == 4)
+ family = PF_INET;
+#endif /* IPv6 */
+
switch (family) {
case PF_INET: {
struct iphdr *hdr4;
--
2.13.6
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC PATCH 4/5] netlabel: Add SCTP support
@ 2017-10-17 13:58 ` Richard Haines
0 siblings, 0 replies; 12+ messages in thread
From: Richard Haines @ 2017-10-17 13:58 UTC (permalink / raw)
To: linux-security-module
Add support to label SCTP associations and cater for a situation where
family = PF_INET6 with an ip_hdr(skb)->version = 4.
Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
---
include/net/netlabel.h | 3 ++
net/netlabel/netlabel_kapi.c | 80 +++++++++++++++++++++++++++++++++++++++
net/netlabel/netlabel_unlabeled.c | 10 +++++
3 files changed, 93 insertions(+)
diff --git a/include/net/netlabel.h b/include/net/netlabel.h
index 72d6435..7348966 100644
--- a/include/net/netlabel.h
+++ b/include/net/netlabel.h
@@ -494,6 +494,9 @@ int netlbl_conn_setattr(struct sock *sk,
const struct netlbl_lsm_secattr *secattr);
int netlbl_req_setattr(struct request_sock *req,
const struct netlbl_lsm_secattr *secattr);
+int netlbl_sctp_setattr(struct sock *sk,
+ struct sk_buff *skb,
+ const struct netlbl_lsm_secattr *secattr);
void netlbl_req_delattr(struct request_sock *req);
int netlbl_skbuff_setattr(struct sk_buff *skb,
u16 family,
diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index ea7c670..1c82bbe 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -1121,6 +1121,7 @@ int netlbl_conn_setattr(struct sock *sk,
switch (addr->sa_family) {
case AF_INET:
addr4 = (struct sockaddr_in *)addr;
+
entry = netlbl_domhsh_getentry_af4(secattr->domain,
addr4->sin_addr.s_addr);
if (entry = NULL) {
@@ -1177,6 +1178,85 @@ int netlbl_conn_setattr(struct sock *sk,
}
/**
+ * netlbl_sctp_setattr - Label an incoming sctp association socket using
+ * the correct protocol
+ * @sk: the socket to label
+ * @skb: the packet
+ * @secattr: the security attributes
+ *
+ * Description:
+ * Attach the correct label to the given socket using the security attributes
+ * specified in @secattr. Returns zero on success, negative values on failure.
+ *
+ */
+int netlbl_sctp_setattr(struct sock *sk,
+ struct sk_buff *skb,
+ const struct netlbl_lsm_secattr *secattr)
+{
+ int ret_val = -EINVAL;
+ struct netlbl_dommap_def *entry;
+ struct iphdr *hdr4;
+#if IS_ENABLED(CONFIG_IPV6)
+ struct ipv6hdr *hdr6;
+#endif
+
+ rcu_read_lock();
+ switch (sk->sk_family) {
+ case AF_INET:
+ hdr4 = ip_hdr(skb);
+
+ entry = netlbl_domhsh_getentry_af4(secattr->domain,
+ hdr4->saddr);
+ if (entry = NULL) {
+ ret_val = -ENOENT;
+ goto sctp_setattr_return;
+ }
+ switch (entry->type) {
+ case NETLBL_NLTYPE_CIPSOV4:
+ ret_val = cipso_v4_sock_setattr(sk, entry->cipso,
+ secattr);
+ break;
+ case NETLBL_NLTYPE_UNLABELED:
+ netlbl_sock_delattr(sk);
+ ret_val = 0;
+ break;
+ default:
+ ret_val = -ENOENT;
+ }
+ break;
+#if IS_ENABLED(CONFIG_IPV6)
+ case AF_INET6:
+ hdr6 = ipv6_hdr(skb);
+ entry = netlbl_domhsh_getentry_af6(secattr->domain,
+ &hdr6->saddr);
+ if (entry = NULL) {
+ ret_val = -ENOENT;
+ goto sctp_setattr_return;
+ }
+ switch (entry->type) {
+ case NETLBL_NLTYPE_CALIPSO:
+ ret_val = calipso_sock_setattr(sk, entry->calipso,
+ secattr);
+ break;
+ case NETLBL_NLTYPE_UNLABELED:
+ netlbl_sock_delattr(sk);
+ ret_val = 0;
+ break;
+ default:
+ ret_val = -ENOENT;
+ }
+ break;
+#endif /* IPv6 */
+ default:
+ ret_val = -EPROTONOSUPPORT;
+ }
+
+sctp_setattr_return:
+ rcu_read_unlock();
+ return ret_val;
+}
+
+/**
* netlbl_req_setattr - Label a request socket using the correct protocol
* @req: the request socket to label
* @secattr: the security attributes
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index 22dc1b9..c070dfc 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -1472,6 +1472,16 @@ int netlbl_unlabel_getattr(const struct sk_buff *skb,
iface = rcu_dereference(netlbl_unlhsh_def);
if (iface = NULL || !iface->valid)
goto unlabel_getattr_nolabel;
+
+#if IS_ENABLED(CONFIG_IPV6)
+ /* When resolving a fallback label, check the sk_buff version as
+ * it is possible (e.g. SCTP) to have family = PF_INET6 while
+ * receiving ip_hdr(skb)->version = 4.
+ */
+ if (family = PF_INET6 && ip_hdr(skb)->version = 4)
+ family = PF_INET;
+#endif /* IPv6 */
+
switch (family) {
case PF_INET: {
struct iphdr *hdr4;
--
2.13.6
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC PATCH 4/5] netlabel: Add SCTP support
@ 2017-10-17 13:58 ` Richard Haines
0 siblings, 0 replies; 12+ messages in thread
From: Richard Haines @ 2017-10-17 13:58 UTC (permalink / raw)
To: linux-security-module
Add support to label SCTP associations and cater for a situation where
family = PF_INET6 with an ip_hdr(skb)->version = 4.
Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
---
include/net/netlabel.h | 3 ++
net/netlabel/netlabel_kapi.c | 80 +++++++++++++++++++++++++++++++++++++++
net/netlabel/netlabel_unlabeled.c | 10 +++++
3 files changed, 93 insertions(+)
diff --git a/include/net/netlabel.h b/include/net/netlabel.h
index 72d6435..7348966 100644
--- a/include/net/netlabel.h
+++ b/include/net/netlabel.h
@@ -494,6 +494,9 @@ int netlbl_conn_setattr(struct sock *sk,
const struct netlbl_lsm_secattr *secattr);
int netlbl_req_setattr(struct request_sock *req,
const struct netlbl_lsm_secattr *secattr);
+int netlbl_sctp_setattr(struct sock *sk,
+ struct sk_buff *skb,
+ const struct netlbl_lsm_secattr *secattr);
void netlbl_req_delattr(struct request_sock *req);
int netlbl_skbuff_setattr(struct sk_buff *skb,
u16 family,
diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index ea7c670..1c82bbe 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -1121,6 +1121,7 @@ int netlbl_conn_setattr(struct sock *sk,
switch (addr->sa_family) {
case AF_INET:
addr4 = (struct sockaddr_in *)addr;
+
entry = netlbl_domhsh_getentry_af4(secattr->domain,
addr4->sin_addr.s_addr);
if (entry == NULL) {
@@ -1177,6 +1178,85 @@ int netlbl_conn_setattr(struct sock *sk,
}
/**
+ * netlbl_sctp_setattr - Label an incoming sctp association socket using
+ * the correct protocol
+ * @sk: the socket to label
+ * @skb: the packet
+ * @secattr: the security attributes
+ *
+ * Description:
+ * Attach the correct label to the given socket using the security attributes
+ * specified in @secattr. Returns zero on success, negative values on failure.
+ *
+ */
+int netlbl_sctp_setattr(struct sock *sk,
+ struct sk_buff *skb,
+ const struct netlbl_lsm_secattr *secattr)
+{
+ int ret_val = -EINVAL;
+ struct netlbl_dommap_def *entry;
+ struct iphdr *hdr4;
+#if IS_ENABLED(CONFIG_IPV6)
+ struct ipv6hdr *hdr6;
+#endif
+
+ rcu_read_lock();
+ switch (sk->sk_family) {
+ case AF_INET:
+ hdr4 = ip_hdr(skb);
+
+ entry = netlbl_domhsh_getentry_af4(secattr->domain,
+ hdr4->saddr);
+ if (entry == NULL) {
+ ret_val = -ENOENT;
+ goto sctp_setattr_return;
+ }
+ switch (entry->type) {
+ case NETLBL_NLTYPE_CIPSOV4:
+ ret_val = cipso_v4_sock_setattr(sk, entry->cipso,
+ secattr);
+ break;
+ case NETLBL_NLTYPE_UNLABELED:
+ netlbl_sock_delattr(sk);
+ ret_val = 0;
+ break;
+ default:
+ ret_val = -ENOENT;
+ }
+ break;
+#if IS_ENABLED(CONFIG_IPV6)
+ case AF_INET6:
+ hdr6 = ipv6_hdr(skb);
+ entry = netlbl_domhsh_getentry_af6(secattr->domain,
+ &hdr6->saddr);
+ if (entry == NULL) {
+ ret_val = -ENOENT;
+ goto sctp_setattr_return;
+ }
+ switch (entry->type) {
+ case NETLBL_NLTYPE_CALIPSO:
+ ret_val = calipso_sock_setattr(sk, entry->calipso,
+ secattr);
+ break;
+ case NETLBL_NLTYPE_UNLABELED:
+ netlbl_sock_delattr(sk);
+ ret_val = 0;
+ break;
+ default:
+ ret_val = -ENOENT;
+ }
+ break;
+#endif /* IPv6 */
+ default:
+ ret_val = -EPROTONOSUPPORT;
+ }
+
+sctp_setattr_return:
+ rcu_read_unlock();
+ return ret_val;
+}
+
+/**
* netlbl_req_setattr - Label a request socket using the correct protocol
* @req: the request socket to label
* @secattr: the security attributes
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index 22dc1b9..c070dfc 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -1472,6 +1472,16 @@ int netlbl_unlabel_getattr(const struct sk_buff *skb,
iface = rcu_dereference(netlbl_unlhsh_def);
if (iface == NULL || !iface->valid)
goto unlabel_getattr_nolabel;
+
+#if IS_ENABLED(CONFIG_IPV6)
+ /* When resolving a fallback label, check the sk_buff version as
+ * it is possible (e.g. SCTP) to have family = PF_INET6 while
+ * receiving ip_hdr(skb)->version = 4.
+ */
+ if (family == PF_INET6 && ip_hdr(skb)->version == 4)
+ family = PF_INET;
+#endif /* IPv6 */
+
switch (family) {
case PF_INET: {
struct iphdr *hdr4;
--
2.13.6
--
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] 12+ messages in thread
* Re: [RFC PATCH 4/5] netlabel: Add SCTP support
2017-10-17 13:58 ` Richard Haines
(?)
@ 2017-11-06 23:15 ` Paul Moore
-1 siblings, 0 replies; 12+ messages in thread
From: Paul Moore @ 2017-11-06 23:15 UTC (permalink / raw)
To: Richard Haines
Cc: selinux, netdev, linux-sctp, linux-security-module,
Vlad Yasevich, nhorman, Stephen Smalley, Eric Paris,
marcelo.leitner
On Tue, Oct 17, 2017 at 9:58 AM, Richard Haines
<richard_c_haines@btinternet.com> wrote:
> Add support to label SCTP associations and cater for a situation where
> family = PF_INET6 with an ip_hdr(skb)->version = 4.
>
> Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
> ---
> include/net/netlabel.h | 3 ++
> net/netlabel/netlabel_kapi.c | 80 +++++++++++++++++++++++++++++++++++++++
> net/netlabel/netlabel_unlabeled.c | 10 +++++
> 3 files changed, 93 insertions(+)
>
> diff --git a/include/net/netlabel.h b/include/net/netlabel.h
> index 72d6435..7348966 100644
> --- a/include/net/netlabel.h
> +++ b/include/net/netlabel.h
> @@ -494,6 +494,9 @@ int netlbl_conn_setattr(struct sock *sk,
> const struct netlbl_lsm_secattr *secattr);
> int netlbl_req_setattr(struct request_sock *req,
> const struct netlbl_lsm_secattr *secattr);
> +int netlbl_sctp_setattr(struct sock *sk,
> + struct sk_buff *skb,
> + const struct netlbl_lsm_secattr *secattr);
> void netlbl_req_delattr(struct request_sock *req);
> int netlbl_skbuff_setattr(struct sk_buff *skb,
> u16 family,
> diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
> index ea7c670..1c82bbe 100644
> --- a/net/netlabel/netlabel_kapi.c
> +++ b/net/netlabel/netlabel_kapi.c
> @@ -1121,6 +1121,7 @@ int netlbl_conn_setattr(struct sock *sk,
> switch (addr->sa_family) {
> case AF_INET:
> addr4 = (struct sockaddr_in *)addr;
> +
I'm guessing this bit of extra whitespace was an accident; but just in
case, drop it from this patch please.
> entry = netlbl_domhsh_getentry_af4(secattr->domain,
> addr4->sin_addr.s_addr);
> if (entry == NULL) {
> @@ -1177,6 +1178,85 @@ int netlbl_conn_setattr(struct sock *sk,
> }
>
> /**
> + * netlbl_sctp_setattr - Label an incoming sctp association socket using
> + * the correct protocol
> + * @sk: the socket to label
> + * @skb: the packet
> + * @secattr: the security attributes
> + *
> + * Description:
> + * Attach the correct label to the given socket using the security attributes
> + * specified in @secattr. Returns zero on success, negative values on failure.
> + *
> + */
> +int netlbl_sctp_setattr(struct sock *sk,
> + struct sk_buff *skb,
> + const struct netlbl_lsm_secattr *secattr)
> +{
> + int ret_val = -EINVAL;
> + struct netlbl_dommap_def *entry;
> + struct iphdr *hdr4;
> +#if IS_ENABLED(CONFIG_IPV6)
> + struct ipv6hdr *hdr6;
> +#endif
> +
> + rcu_read_lock();
> + switch (sk->sk_family) {
> + case AF_INET:
> + hdr4 = ip_hdr(skb);
> +
> + entry = netlbl_domhsh_getentry_af4(secattr->domain,
> + hdr4->saddr);
> + if (entry == NULL) {
> + ret_val = -ENOENT;
> + goto sctp_setattr_return;
> + }
> + switch (entry->type) {
> + case NETLBL_NLTYPE_CIPSOV4:
> + ret_val = cipso_v4_sock_setattr(sk, entry->cipso,
> + secattr);
> + break;
> + case NETLBL_NLTYPE_UNLABELED:
> + netlbl_sock_delattr(sk);
> + ret_val = 0;
> + break;
> + default:
> + ret_val = -ENOENT;
> + }
> + break;
> +#if IS_ENABLED(CONFIG_IPV6)
> + case AF_INET6:
> + hdr6 = ipv6_hdr(skb);
> + entry = netlbl_domhsh_getentry_af6(secattr->domain,
> + &hdr6->saddr);
> + if (entry == NULL) {
> + ret_val = -ENOENT;
> + goto sctp_setattr_return;
> + }
> + switch (entry->type) {
> + case NETLBL_NLTYPE_CALIPSO:
> + ret_val = calipso_sock_setattr(sk, entry->calipso,
> + secattr);
> + break;
> + case NETLBL_NLTYPE_UNLABELED:
> + netlbl_sock_delattr(sk);
> + ret_val = 0;
> + break;
> + default:
> + ret_val = -ENOENT;
> + }
> + break;
> +#endif /* IPv6 */
> + default:
> + ret_val = -EPROTONOSUPPORT;
> + }
> +
> +sctp_setattr_return:
> + rcu_read_unlock();
> + return ret_val;
> +}
It seems like we should try to leverage the code in
netlbl_conn_setattr() a bit more. I would suggest either tweaking the
callers to use a sockaddr struct and netlbl_conn_setattr(), or
implement netlbl_sctp_setattr() as a simple wrapper around
netlbl_conn_setattr() ... the former seems a bit cleaner, but I
suspect patch 5/5 will make it clear which approach is better.
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index 22dc1b9..c070dfc 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -1472,6 +1472,16 @@ int netlbl_unlabel_getattr(const struct sk_buff *skb,
> iface = rcu_dereference(netlbl_unlhsh_def);
> if (iface == NULL || !iface->valid)
> goto unlabel_getattr_nolabel;
> +
> +#if IS_ENABLED(CONFIG_IPV6)
> + /* When resolving a fallback label, check the sk_buff version as
> + * it is possible (e.g. SCTP) to have family = PF_INET6 while
> + * receiving ip_hdr(skb)->version = 4.
> + */
> + if (family == PF_INET6 && ip_hdr(skb)->version == 4)
> + family = PF_INET;
> +#endif /* IPv6 */
> +
It seems like this should be pulled out into it's own patch as a fix
that extends beyond SCTP, what do you think?
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 4/5] netlabel: Add SCTP support
@ 2017-11-06 23:15 ` Paul Moore
0 siblings, 0 replies; 12+ messages in thread
From: Paul Moore @ 2017-11-06 23:15 UTC (permalink / raw)
To: linux-security-module
On Tue, Oct 17, 2017 at 9:58 AM, Richard Haines
<richard_c_haines@btinternet.com> wrote:
> Add support to label SCTP associations and cater for a situation where
> family = PF_INET6 with an ip_hdr(skb)->version = 4.
>
> Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
> ---
> include/net/netlabel.h | 3 ++
> net/netlabel/netlabel_kapi.c | 80 +++++++++++++++++++++++++++++++++++++++
> net/netlabel/netlabel_unlabeled.c | 10 +++++
> 3 files changed, 93 insertions(+)
>
> diff --git a/include/net/netlabel.h b/include/net/netlabel.h
> index 72d6435..7348966 100644
> --- a/include/net/netlabel.h
> +++ b/include/net/netlabel.h
> @@ -494,6 +494,9 @@ int netlbl_conn_setattr(struct sock *sk,
> const struct netlbl_lsm_secattr *secattr);
> int netlbl_req_setattr(struct request_sock *req,
> const struct netlbl_lsm_secattr *secattr);
> +int netlbl_sctp_setattr(struct sock *sk,
> + struct sk_buff *skb,
> + const struct netlbl_lsm_secattr *secattr);
> void netlbl_req_delattr(struct request_sock *req);
> int netlbl_skbuff_setattr(struct sk_buff *skb,
> u16 family,
> diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
> index ea7c670..1c82bbe 100644
> --- a/net/netlabel/netlabel_kapi.c
> +++ b/net/netlabel/netlabel_kapi.c
> @@ -1121,6 +1121,7 @@ int netlbl_conn_setattr(struct sock *sk,
> switch (addr->sa_family) {
> case AF_INET:
> addr4 = (struct sockaddr_in *)addr;
> +
I'm guessing this bit of extra whitespace was an accident; but just in
case, drop it from this patch please.
> entry = netlbl_domhsh_getentry_af4(secattr->domain,
> addr4->sin_addr.s_addr);
> if (entry = NULL) {
> @@ -1177,6 +1178,85 @@ int netlbl_conn_setattr(struct sock *sk,
> }
>
> /**
> + * netlbl_sctp_setattr - Label an incoming sctp association socket using
> + * the correct protocol
> + * @sk: the socket to label
> + * @skb: the packet
> + * @secattr: the security attributes
> + *
> + * Description:
> + * Attach the correct label to the given socket using the security attributes
> + * specified in @secattr. Returns zero on success, negative values on failure.
> + *
> + */
> +int netlbl_sctp_setattr(struct sock *sk,
> + struct sk_buff *skb,
> + const struct netlbl_lsm_secattr *secattr)
> +{
> + int ret_val = -EINVAL;
> + struct netlbl_dommap_def *entry;
> + struct iphdr *hdr4;
> +#if IS_ENABLED(CONFIG_IPV6)
> + struct ipv6hdr *hdr6;
> +#endif
> +
> + rcu_read_lock();
> + switch (sk->sk_family) {
> + case AF_INET:
> + hdr4 = ip_hdr(skb);
> +
> + entry = netlbl_domhsh_getentry_af4(secattr->domain,
> + hdr4->saddr);
> + if (entry = NULL) {
> + ret_val = -ENOENT;
> + goto sctp_setattr_return;
> + }
> + switch (entry->type) {
> + case NETLBL_NLTYPE_CIPSOV4:
> + ret_val = cipso_v4_sock_setattr(sk, entry->cipso,
> + secattr);
> + break;
> + case NETLBL_NLTYPE_UNLABELED:
> + netlbl_sock_delattr(sk);
> + ret_val = 0;
> + break;
> + default:
> + ret_val = -ENOENT;
> + }
> + break;
> +#if IS_ENABLED(CONFIG_IPV6)
> + case AF_INET6:
> + hdr6 = ipv6_hdr(skb);
> + entry = netlbl_domhsh_getentry_af6(secattr->domain,
> + &hdr6->saddr);
> + if (entry = NULL) {
> + ret_val = -ENOENT;
> + goto sctp_setattr_return;
> + }
> + switch (entry->type) {
> + case NETLBL_NLTYPE_CALIPSO:
> + ret_val = calipso_sock_setattr(sk, entry->calipso,
> + secattr);
> + break;
> + case NETLBL_NLTYPE_UNLABELED:
> + netlbl_sock_delattr(sk);
> + ret_val = 0;
> + break;
> + default:
> + ret_val = -ENOENT;
> + }
> + break;
> +#endif /* IPv6 */
> + default:
> + ret_val = -EPROTONOSUPPORT;
> + }
> +
> +sctp_setattr_return:
> + rcu_read_unlock();
> + return ret_val;
> +}
It seems like we should try to leverage the code in
netlbl_conn_setattr() a bit more. I would suggest either tweaking the
callers to use a sockaddr struct and netlbl_conn_setattr(), or
implement netlbl_sctp_setattr() as a simple wrapper around
netlbl_conn_setattr() ... the former seems a bit cleaner, but I
suspect patch 5/5 will make it clear which approach is better.
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index 22dc1b9..c070dfc 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -1472,6 +1472,16 @@ int netlbl_unlabel_getattr(const struct sk_buff *skb,
> iface = rcu_dereference(netlbl_unlhsh_def);
> if (iface = NULL || !iface->valid)
> goto unlabel_getattr_nolabel;
> +
> +#if IS_ENABLED(CONFIG_IPV6)
> + /* When resolving a fallback label, check the sk_buff version as
> + * it is possible (e.g. SCTP) to have family = PF_INET6 while
> + * receiving ip_hdr(skb)->version = 4.
> + */
> + if (family = PF_INET6 && ip_hdr(skb)->version = 4)
> + family = PF_INET;
> +#endif /* IPv6 */
> +
It seems like this should be pulled out into it's own patch as a fix
that extends beyond SCTP, what do you think?
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH 4/5] netlabel: Add SCTP support
@ 2017-11-06 23:15 ` Paul Moore
0 siblings, 0 replies; 12+ messages in thread
From: Paul Moore @ 2017-11-06 23:15 UTC (permalink / raw)
To: linux-security-module
On Tue, Oct 17, 2017 at 9:58 AM, Richard Haines
<richard_c_haines@btinternet.com> wrote:
> Add support to label SCTP associations and cater for a situation where
> family = PF_INET6 with an ip_hdr(skb)->version = 4.
>
> Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
> ---
> include/net/netlabel.h | 3 ++
> net/netlabel/netlabel_kapi.c | 80 +++++++++++++++++++++++++++++++++++++++
> net/netlabel/netlabel_unlabeled.c | 10 +++++
> 3 files changed, 93 insertions(+)
>
> diff --git a/include/net/netlabel.h b/include/net/netlabel.h
> index 72d6435..7348966 100644
> --- a/include/net/netlabel.h
> +++ b/include/net/netlabel.h
> @@ -494,6 +494,9 @@ int netlbl_conn_setattr(struct sock *sk,
> const struct netlbl_lsm_secattr *secattr);
> int netlbl_req_setattr(struct request_sock *req,
> const struct netlbl_lsm_secattr *secattr);
> +int netlbl_sctp_setattr(struct sock *sk,
> + struct sk_buff *skb,
> + const struct netlbl_lsm_secattr *secattr);
> void netlbl_req_delattr(struct request_sock *req);
> int netlbl_skbuff_setattr(struct sk_buff *skb,
> u16 family,
> diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
> index ea7c670..1c82bbe 100644
> --- a/net/netlabel/netlabel_kapi.c
> +++ b/net/netlabel/netlabel_kapi.c
> @@ -1121,6 +1121,7 @@ int netlbl_conn_setattr(struct sock *sk,
> switch (addr->sa_family) {
> case AF_INET:
> addr4 = (struct sockaddr_in *)addr;
> +
I'm guessing this bit of extra whitespace was an accident; but just in
case, drop it from this patch please.
> entry = netlbl_domhsh_getentry_af4(secattr->domain,
> addr4->sin_addr.s_addr);
> if (entry == NULL) {
> @@ -1177,6 +1178,85 @@ int netlbl_conn_setattr(struct sock *sk,
> }
>
> /**
> + * netlbl_sctp_setattr - Label an incoming sctp association socket using
> + * the correct protocol
> + * @sk: the socket to label
> + * @skb: the packet
> + * @secattr: the security attributes
> + *
> + * Description:
> + * Attach the correct label to the given socket using the security attributes
> + * specified in @secattr. Returns zero on success, negative values on failure.
> + *
> + */
> +int netlbl_sctp_setattr(struct sock *sk,
> + struct sk_buff *skb,
> + const struct netlbl_lsm_secattr *secattr)
> +{
> + int ret_val = -EINVAL;
> + struct netlbl_dommap_def *entry;
> + struct iphdr *hdr4;
> +#if IS_ENABLED(CONFIG_IPV6)
> + struct ipv6hdr *hdr6;
> +#endif
> +
> + rcu_read_lock();
> + switch (sk->sk_family) {
> + case AF_INET:
> + hdr4 = ip_hdr(skb);
> +
> + entry = netlbl_domhsh_getentry_af4(secattr->domain,
> + hdr4->saddr);
> + if (entry == NULL) {
> + ret_val = -ENOENT;
> + goto sctp_setattr_return;
> + }
> + switch (entry->type) {
> + case NETLBL_NLTYPE_CIPSOV4:
> + ret_val = cipso_v4_sock_setattr(sk, entry->cipso,
> + secattr);
> + break;
> + case NETLBL_NLTYPE_UNLABELED:
> + netlbl_sock_delattr(sk);
> + ret_val = 0;
> + break;
> + default:
> + ret_val = -ENOENT;
> + }
> + break;
> +#if IS_ENABLED(CONFIG_IPV6)
> + case AF_INET6:
> + hdr6 = ipv6_hdr(skb);
> + entry = netlbl_domhsh_getentry_af6(secattr->domain,
> + &hdr6->saddr);
> + if (entry == NULL) {
> + ret_val = -ENOENT;
> + goto sctp_setattr_return;
> + }
> + switch (entry->type) {
> + case NETLBL_NLTYPE_CALIPSO:
> + ret_val = calipso_sock_setattr(sk, entry->calipso,
> + secattr);
> + break;
> + case NETLBL_NLTYPE_UNLABELED:
> + netlbl_sock_delattr(sk);
> + ret_val = 0;
> + break;
> + default:
> + ret_val = -ENOENT;
> + }
> + break;
> +#endif /* IPv6 */
> + default:
> + ret_val = -EPROTONOSUPPORT;
> + }
> +
> +sctp_setattr_return:
> + rcu_read_unlock();
> + return ret_val;
> +}
It seems like we should try to leverage the code in
netlbl_conn_setattr() a bit more. I would suggest either tweaking the
callers to use a sockaddr struct and netlbl_conn_setattr(), or
implement netlbl_sctp_setattr() as a simple wrapper around
netlbl_conn_setattr() ... the former seems a bit cleaner, but I
suspect patch 5/5 will make it clear which approach is better.
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index 22dc1b9..c070dfc 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -1472,6 +1472,16 @@ int netlbl_unlabel_getattr(const struct sk_buff *skb,
> iface = rcu_dereference(netlbl_unlhsh_def);
> if (iface == NULL || !iface->valid)
> goto unlabel_getattr_nolabel;
> +
> +#if IS_ENABLED(CONFIG_IPV6)
> + /* When resolving a fallback label, check the sk_buff version as
> + * it is possible (e.g. SCTP) to have family = PF_INET6 while
> + * receiving ip_hdr(skb)->version = 4.
> + */
> + if (family == PF_INET6 && ip_hdr(skb)->version == 4)
> + family = PF_INET;
> +#endif /* IPv6 */
> +
It seems like this should be pulled out into it's own patch as a fix
that extends beyond SCTP, what do you think?
--
paul moore
www.paul-moore.com
--
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] 12+ messages in thread
* Re: [RFC PATCH 4/5] netlabel: Add SCTP support
2017-11-06 23:15 ` Paul Moore
(?)
@ 2017-11-13 20:50 ` Richard Haines
-1 siblings, 0 replies; 12+ messages in thread
From: Richard Haines @ 2017-11-13 20:50 UTC (permalink / raw)
To: Paul Moore
Cc: selinux, netdev, linux-sctp, linux-security-module,
Vlad Yasevich, nhorman, Stephen Smalley, Eric Paris,
marcelo.leitner
On Mon, 2017-11-06 at 18:15 -0500, Paul Moore wrote:
> On Tue, Oct 17, 2017 at 9:58 AM, Richard Haines
> <richard_c_haines@btinternet.com> wrote:
> > Add support to label SCTP associations and cater for a situation
> > where
> > family = PF_INET6 with an ip_hdr(skb)->version = 4.
> >
> > Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
> > ---
> > include/net/netlabel.h | 3 ++
> > net/netlabel/netlabel_kapi.c | 80
> > +++++++++++++++++++++++++++++++++++++++
> > net/netlabel/netlabel_unlabeled.c | 10 +++++
> > 3 files changed, 93 insertions(+)
> >
> > diff --git a/include/net/netlabel.h b/include/net/netlabel.h
> > index 72d6435..7348966 100644
> > --- a/include/net/netlabel.h
> > +++ b/include/net/netlabel.h
> > @@ -494,6 +494,9 @@ int netlbl_conn_setattr(struct sock *sk,
> > const struct netlbl_lsm_secattr *secattr);
> > int netlbl_req_setattr(struct request_sock *req,
> > const struct netlbl_lsm_secattr *secattr);
> > +int netlbl_sctp_setattr(struct sock *sk,
> > + struct sk_buff *skb,
> > + const struct netlbl_lsm_secattr *secattr);
> > void netlbl_req_delattr(struct request_sock *req);
> > int netlbl_skbuff_setattr(struct sk_buff *skb,
> > u16 family,
> > diff --git a/net/netlabel/netlabel_kapi.c
> > b/net/netlabel/netlabel_kapi.c
> > index ea7c670..1c82bbe 100644
> > --- a/net/netlabel/netlabel_kapi.c
> > +++ b/net/netlabel/netlabel_kapi.c
> > @@ -1121,6 +1121,7 @@ int netlbl_conn_setattr(struct sock *sk,
> > switch (addr->sa_family) {
> > case AF_INET:
> > addr4 = (struct sockaddr_in *)addr;
> > +
>
> I'm guessing this bit of extra whitespace was an accident; but just
> in
> case, drop it from this patch please.
Done
>
> > entry = netlbl_domhsh_getentry_af4(secattr->domain,
> > addr4-
> > >sin_addr.s_addr);
> > if (entry == NULL) {
> > @@ -1177,6 +1178,85 @@ int netlbl_conn_setattr(struct sock *sk,
> > }
> >
> > /**
> > + * netlbl_sctp_setattr - Label an incoming sctp association socket
> > using
> > + * the correct protocol
> > + * @sk: the socket to label
> > + * @skb: the packet
> > + * @secattr: the security attributes
> > + *
> > + * Description:
> > + * Attach the correct label to the given socket using the security
> > attributes
> > + * specified in @secattr. Returns zero on success, negative
> > values on failure.
> > + *
> > + */
> > +int netlbl_sctp_setattr(struct sock *sk,
> > + struct sk_buff *skb,
> > + const struct netlbl_lsm_secattr *secattr)
> > +{
> > + int ret_val = -EINVAL;
> > + struct netlbl_dommap_def *entry;
> > + struct iphdr *hdr4;
> > +#if IS_ENABLED(CONFIG_IPV6)
> > + struct ipv6hdr *hdr6;
> > +#endif
> > +
> > + rcu_read_lock();
> > + switch (sk->sk_family) {
> > + case AF_INET:
> > + hdr4 = ip_hdr(skb);
> > +
> > + entry = netlbl_domhsh_getentry_af4(secattr->domain,
> > + hdr4->saddr);
> > + if (entry == NULL) {
> > + ret_val = -ENOENT;
> > + goto sctp_setattr_return;
> > + }
> > + switch (entry->type) {
> > + case NETLBL_NLTYPE_CIPSOV4:
> > + ret_val = cipso_v4_sock_setattr(sk, entry-
> > >cipso,
> > + secattr);
> > + break;
> > + case NETLBL_NLTYPE_UNLABELED:
> > + netlbl_sock_delattr(sk);
> > + ret_val = 0;
> > + break;
> > + default:
> > + ret_val = -ENOENT;
> > + }
> > + break;
> > +#if IS_ENABLED(CONFIG_IPV6)
> > + case AF_INET6:
> > + hdr6 = ipv6_hdr(skb);
> > + entry = netlbl_domhsh_getentry_af6(secattr->domain,
> > + &hdr6->saddr);
> > + if (entry == NULL) {
> > + ret_val = -ENOENT;
> > + goto sctp_setattr_return;
> > + }
> > + switch (entry->type) {
> > + case NETLBL_NLTYPE_CALIPSO:
> > + ret_val = calipso_sock_setattr(sk, entry-
> > >calipso,
> > + secattr);
> > + break;
> > + case NETLBL_NLTYPE_UNLABELED:
> > + netlbl_sock_delattr(sk);
> > + ret_val = 0;
> > + break;
> > + default:
> > + ret_val = -ENOENT;
> > + }
> > + break;
> > +#endif /* IPv6 */
> > + default:
> > + ret_val = -EPROTONOSUPPORT;
> > + }
> > +
> > +sctp_setattr_return:
> > + rcu_read_unlock();
> > + return ret_val;
> > +}
>
> It seems like we should try to leverage the code in
> netlbl_conn_setattr() a bit more. I would suggest either tweaking
> the
> callers to use a sockaddr struct and netlbl_conn_setattr(), or
> implement netlbl_sctp_setattr() as a simple wrapper around
> netlbl_conn_setattr() ... the former seems a bit cleaner, but I
> suspect patch 5/5 will make it clear which approach is better.
I've now modified selinux_netlbl_sctp_assoc_request() to extract the ip
address info from skb and add to a sockaddr struct, then call
netlbl_conn_setattr(). This removes any specific SCTP code from
netlabel_kapi.c
>
> > diff --git a/net/netlabel/netlabel_unlabeled.c
> > b/net/netlabel/netlabel_unlabeled.c
> > index 22dc1b9..c070dfc 100644
> > --- a/net/netlabel/netlabel_unlabeled.c
> > +++ b/net/netlabel/netlabel_unlabeled.c
> > @@ -1472,6 +1472,16 @@ int netlbl_unlabel_getattr(const struct
> > sk_buff *skb,
> > iface = rcu_dereference(netlbl_unlhsh_def);
> > if (iface == NULL || !iface->valid)
> > goto unlabel_getattr_nolabel;
> > +
> > +#if IS_ENABLED(CONFIG_IPV6)
> > + /* When resolving a fallback label, check the sk_buff
> > version as
> > + * it is possible (e.g. SCTP) to have family = PF_INET6
> > while
> > + * receiving ip_hdr(skb)->version = 4.
> > + */
> > + if (family == PF_INET6 && ip_hdr(skb)->version == 4)
> > + family = PF_INET;
> > +#endif /* IPv6 */
> > +
>
> It seems like this should be pulled out into it's own patch as a fix
> that extends beyond SCTP, what do you think?
I'll submit this as a separate NetLabel patch today.
Thanks for all your comments on the patchset, will probably take a few
weeks to respond to them all.
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 4/5] netlabel: Add SCTP support
@ 2017-11-13 20:50 ` Richard Haines
0 siblings, 0 replies; 12+ messages in thread
From: Richard Haines @ 2017-11-13 20:50 UTC (permalink / raw)
To: linux-security-module
On Mon, 2017-11-06 at 18:15 -0500, Paul Moore wrote:
> On Tue, Oct 17, 2017 at 9:58 AM, Richard Haines
> <richard_c_haines@btinternet.com> wrote:
> > Add support to label SCTP associations and cater for a situation
> > where
> > family = PF_INET6 with an ip_hdr(skb)->version = 4.
> >
> > Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
> > ---
> > include/net/netlabel.h | 3 ++
> > net/netlabel/netlabel_kapi.c | 80
> > +++++++++++++++++++++++++++++++++++++++
> > net/netlabel/netlabel_unlabeled.c | 10 +++++
> > 3 files changed, 93 insertions(+)
> >
> > diff --git a/include/net/netlabel.h b/include/net/netlabel.h
> > index 72d6435..7348966 100644
> > --- a/include/net/netlabel.h
> > +++ b/include/net/netlabel.h
> > @@ -494,6 +494,9 @@ int netlbl_conn_setattr(struct sock *sk,
> > const struct netlbl_lsm_secattr *secattr);
> > int netlbl_req_setattr(struct request_sock *req,
> > const struct netlbl_lsm_secattr *secattr);
> > +int netlbl_sctp_setattr(struct sock *sk,
> > + struct sk_buff *skb,
> > + const struct netlbl_lsm_secattr *secattr);
> > void netlbl_req_delattr(struct request_sock *req);
> > int netlbl_skbuff_setattr(struct sk_buff *skb,
> > u16 family,
> > diff --git a/net/netlabel/netlabel_kapi.c
> > b/net/netlabel/netlabel_kapi.c
> > index ea7c670..1c82bbe 100644
> > --- a/net/netlabel/netlabel_kapi.c
> > +++ b/net/netlabel/netlabel_kapi.c
> > @@ -1121,6 +1121,7 @@ int netlbl_conn_setattr(struct sock *sk,
> > switch (addr->sa_family) {
> > case AF_INET:
> > addr4 = (struct sockaddr_in *)addr;
> > +
>
> I'm guessing this bit of extra whitespace was an accident; but just
> in
> case, drop it from this patch please.
Done
>
> > entry = netlbl_domhsh_getentry_af4(secattr->domain,
> > addr4-
> > >sin_addr.s_addr);
> > if (entry = NULL) {
> > @@ -1177,6 +1178,85 @@ int netlbl_conn_setattr(struct sock *sk,
> > }
> >
> > /**
> > + * netlbl_sctp_setattr - Label an incoming sctp association socket
> > using
> > + * the correct protocol
> > + * @sk: the socket to label
> > + * @skb: the packet
> > + * @secattr: the security attributes
> > + *
> > + * Description:
> > + * Attach the correct label to the given socket using the security
> > attributes
> > + * specified in @secattr. Returns zero on success, negative
> > values on failure.
> > + *
> > + */
> > +int netlbl_sctp_setattr(struct sock *sk,
> > + struct sk_buff *skb,
> > + const struct netlbl_lsm_secattr *secattr)
> > +{
> > + int ret_val = -EINVAL;
> > + struct netlbl_dommap_def *entry;
> > + struct iphdr *hdr4;
> > +#if IS_ENABLED(CONFIG_IPV6)
> > + struct ipv6hdr *hdr6;
> > +#endif
> > +
> > + rcu_read_lock();
> > + switch (sk->sk_family) {
> > + case AF_INET:
> > + hdr4 = ip_hdr(skb);
> > +
> > + entry = netlbl_domhsh_getentry_af4(secattr->domain,
> > + hdr4->saddr);
> > + if (entry = NULL) {
> > + ret_val = -ENOENT;
> > + goto sctp_setattr_return;
> > + }
> > + switch (entry->type) {
> > + case NETLBL_NLTYPE_CIPSOV4:
> > + ret_val = cipso_v4_sock_setattr(sk, entry-
> > >cipso,
> > + secattr);
> > + break;
> > + case NETLBL_NLTYPE_UNLABELED:
> > + netlbl_sock_delattr(sk);
> > + ret_val = 0;
> > + break;
> > + default:
> > + ret_val = -ENOENT;
> > + }
> > + break;
> > +#if IS_ENABLED(CONFIG_IPV6)
> > + case AF_INET6:
> > + hdr6 = ipv6_hdr(skb);
> > + entry = netlbl_domhsh_getentry_af6(secattr->domain,
> > + &hdr6->saddr);
> > + if (entry = NULL) {
> > + ret_val = -ENOENT;
> > + goto sctp_setattr_return;
> > + }
> > + switch (entry->type) {
> > + case NETLBL_NLTYPE_CALIPSO:
> > + ret_val = calipso_sock_setattr(sk, entry-
> > >calipso,
> > + secattr);
> > + break;
> > + case NETLBL_NLTYPE_UNLABELED:
> > + netlbl_sock_delattr(sk);
> > + ret_val = 0;
> > + break;
> > + default:
> > + ret_val = -ENOENT;
> > + }
> > + break;
> > +#endif /* IPv6 */
> > + default:
> > + ret_val = -EPROTONOSUPPORT;
> > + }
> > +
> > +sctp_setattr_return:
> > + rcu_read_unlock();
> > + return ret_val;
> > +}
>
> It seems like we should try to leverage the code in
> netlbl_conn_setattr() a bit more. I would suggest either tweaking
> the
> callers to use a sockaddr struct and netlbl_conn_setattr(), or
> implement netlbl_sctp_setattr() as a simple wrapper around
> netlbl_conn_setattr() ... the former seems a bit cleaner, but I
> suspect patch 5/5 will make it clear which approach is better.
I've now modified selinux_netlbl_sctp_assoc_request() to extract the ip
address info from skb and add to a sockaddr struct, then call
netlbl_conn_setattr(). This removes any specific SCTP code from
netlabel_kapi.c
>
> > diff --git a/net/netlabel/netlabel_unlabeled.c
> > b/net/netlabel/netlabel_unlabeled.c
> > index 22dc1b9..c070dfc 100644
> > --- a/net/netlabel/netlabel_unlabeled.c
> > +++ b/net/netlabel/netlabel_unlabeled.c
> > @@ -1472,6 +1472,16 @@ int netlbl_unlabel_getattr(const struct
> > sk_buff *skb,
> > iface = rcu_dereference(netlbl_unlhsh_def);
> > if (iface = NULL || !iface->valid)
> > goto unlabel_getattr_nolabel;
> > +
> > +#if IS_ENABLED(CONFIG_IPV6)
> > + /* When resolving a fallback label, check the sk_buff
> > version as
> > + * it is possible (e.g. SCTP) to have family = PF_INET6
> > while
> > + * receiving ip_hdr(skb)->version = 4.
> > + */
> > + if (family = PF_INET6 && ip_hdr(skb)->version = 4)
> > + family = PF_INET;
> > +#endif /* IPv6 */
> > +
>
> It seems like this should be pulled out into it's own patch as a fix
> that extends beyond SCTP, what do you think?
I'll submit this as a separate NetLabel patch today.
Thanks for all your comments on the patchset, will probably take a few
weeks to respond to them all.
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH 4/5] netlabel: Add SCTP support
@ 2017-11-13 20:50 ` Richard Haines
0 siblings, 0 replies; 12+ messages in thread
From: Richard Haines @ 2017-11-13 20:50 UTC (permalink / raw)
To: linux-security-module
On Mon, 2017-11-06 at 18:15 -0500, Paul Moore wrote:
> On Tue, Oct 17, 2017 at 9:58 AM, Richard Haines
> <richard_c_haines@btinternet.com> wrote:
> > Add support to label SCTP associations and cater for a situation
> > where
> > family = PF_INET6 with an ip_hdr(skb)->version = 4.
> >
> > Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
> > ---
> > include/net/netlabel.h | 3 ++
> > net/netlabel/netlabel_kapi.c | 80
> > +++++++++++++++++++++++++++++++++++++++
> > net/netlabel/netlabel_unlabeled.c | 10 +++++
> > 3 files changed, 93 insertions(+)
> >
> > diff --git a/include/net/netlabel.h b/include/net/netlabel.h
> > index 72d6435..7348966 100644
> > --- a/include/net/netlabel.h
> > +++ b/include/net/netlabel.h
> > @@ -494,6 +494,9 @@ int netlbl_conn_setattr(struct sock *sk,
> > const struct netlbl_lsm_secattr *secattr);
> > int netlbl_req_setattr(struct request_sock *req,
> > const struct netlbl_lsm_secattr *secattr);
> > +int netlbl_sctp_setattr(struct sock *sk,
> > + struct sk_buff *skb,
> > + const struct netlbl_lsm_secattr *secattr);
> > void netlbl_req_delattr(struct request_sock *req);
> > int netlbl_skbuff_setattr(struct sk_buff *skb,
> > u16 family,
> > diff --git a/net/netlabel/netlabel_kapi.c
> > b/net/netlabel/netlabel_kapi.c
> > index ea7c670..1c82bbe 100644
> > --- a/net/netlabel/netlabel_kapi.c
> > +++ b/net/netlabel/netlabel_kapi.c
> > @@ -1121,6 +1121,7 @@ int netlbl_conn_setattr(struct sock *sk,
> > switch (addr->sa_family) {
> > case AF_INET:
> > addr4 = (struct sockaddr_in *)addr;
> > +
>
> I'm guessing this bit of extra whitespace was an accident; but just
> in
> case, drop it from this patch please.
Done
>
> > entry = netlbl_domhsh_getentry_af4(secattr->domain,
> > addr4-
> > >sin_addr.s_addr);
> > if (entry == NULL) {
> > @@ -1177,6 +1178,85 @@ int netlbl_conn_setattr(struct sock *sk,
> > }
> >
> > /**
> > + * netlbl_sctp_setattr - Label an incoming sctp association socket
> > using
> > + * the correct protocol
> > + * @sk: the socket to label
> > + * @skb: the packet
> > + * @secattr: the security attributes
> > + *
> > + * Description:
> > + * Attach the correct label to the given socket using the security
> > attributes
> > + * specified in @secattr. Returns zero on success, negative
> > values on failure.
> > + *
> > + */
> > +int netlbl_sctp_setattr(struct sock *sk,
> > + struct sk_buff *skb,
> > + const struct netlbl_lsm_secattr *secattr)
> > +{
> > + int ret_val = -EINVAL;
> > + struct netlbl_dommap_def *entry;
> > + struct iphdr *hdr4;
> > +#if IS_ENABLED(CONFIG_IPV6)
> > + struct ipv6hdr *hdr6;
> > +#endif
> > +
> > + rcu_read_lock();
> > + switch (sk->sk_family) {
> > + case AF_INET:
> > + hdr4 = ip_hdr(skb);
> > +
> > + entry = netlbl_domhsh_getentry_af4(secattr->domain,
> > + hdr4->saddr);
> > + if (entry == NULL) {
> > + ret_val = -ENOENT;
> > + goto sctp_setattr_return;
> > + }
> > + switch (entry->type) {
> > + case NETLBL_NLTYPE_CIPSOV4:
> > + ret_val = cipso_v4_sock_setattr(sk, entry-
> > >cipso,
> > + secattr);
> > + break;
> > + case NETLBL_NLTYPE_UNLABELED:
> > + netlbl_sock_delattr(sk);
> > + ret_val = 0;
> > + break;
> > + default:
> > + ret_val = -ENOENT;
> > + }
> > + break;
> > +#if IS_ENABLED(CONFIG_IPV6)
> > + case AF_INET6:
> > + hdr6 = ipv6_hdr(skb);
> > + entry = netlbl_domhsh_getentry_af6(secattr->domain,
> > + &hdr6->saddr);
> > + if (entry == NULL) {
> > + ret_val = -ENOENT;
> > + goto sctp_setattr_return;
> > + }
> > + switch (entry->type) {
> > + case NETLBL_NLTYPE_CALIPSO:
> > + ret_val = calipso_sock_setattr(sk, entry-
> > >calipso,
> > + secattr);
> > + break;
> > + case NETLBL_NLTYPE_UNLABELED:
> > + netlbl_sock_delattr(sk);
> > + ret_val = 0;
> > + break;
> > + default:
> > + ret_val = -ENOENT;
> > + }
> > + break;
> > +#endif /* IPv6 */
> > + default:
> > + ret_val = -EPROTONOSUPPORT;
> > + }
> > +
> > +sctp_setattr_return:
> > + rcu_read_unlock();
> > + return ret_val;
> > +}
>
> It seems like we should try to leverage the code in
> netlbl_conn_setattr() a bit more. I would suggest either tweaking
> the
> callers to use a sockaddr struct and netlbl_conn_setattr(), or
> implement netlbl_sctp_setattr() as a simple wrapper around
> netlbl_conn_setattr() ... the former seems a bit cleaner, but I
> suspect patch 5/5 will make it clear which approach is better.
I've now modified selinux_netlbl_sctp_assoc_request() to extract the ip
address info from skb and add to a sockaddr struct, then call
netlbl_conn_setattr(). This removes any specific SCTP code from
netlabel_kapi.c
>
> > diff --git a/net/netlabel/netlabel_unlabeled.c
> > b/net/netlabel/netlabel_unlabeled.c
> > index 22dc1b9..c070dfc 100644
> > --- a/net/netlabel/netlabel_unlabeled.c
> > +++ b/net/netlabel/netlabel_unlabeled.c
> > @@ -1472,6 +1472,16 @@ int netlbl_unlabel_getattr(const struct
> > sk_buff *skb,
> > iface = rcu_dereference(netlbl_unlhsh_def);
> > if (iface == NULL || !iface->valid)
> > goto unlabel_getattr_nolabel;
> > +
> > +#if IS_ENABLED(CONFIG_IPV6)
> > + /* When resolving a fallback label, check the sk_buff
> > version as
> > + * it is possible (e.g. SCTP) to have family = PF_INET6
> > while
> > + * receiving ip_hdr(skb)->version = 4.
> > + */
> > + if (family == PF_INET6 && ip_hdr(skb)->version == 4)
> > + family = PF_INET;
> > +#endif /* IPv6 */
> > +
>
> It seems like this should be pulled out into it's own patch as a fix
> that extends beyond SCTP, what do you think?
I'll submit this as a separate NetLabel patch today.
Thanks for all your comments on the patchset, will probably take a few
weeks to respond to them all.
>
--
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] 12+ messages in thread
* Re: [RFC PATCH 4/5] netlabel: Add SCTP support
2017-11-13 20:50 ` Richard Haines
(?)
@ 2017-11-13 22:26 ` Paul Moore
-1 siblings, 0 replies; 12+ messages in thread
From: Paul Moore @ 2017-11-13 22:26 UTC (permalink / raw)
To: Richard Haines
Cc: selinux, netdev, linux-sctp, linux-security-module,
Vlad Yasevich, nhorman, Stephen Smalley, Eric Paris,
marcelo.leitner
On Mon, Nov 13, 2017 at 3:50 PM, Richard Haines
<richard_c_haines@btinternet.com> wrote:
> On Mon, 2017-11-06 at 18:15 -0500, Paul Moore wrote:
>> On Tue, Oct 17, 2017 at 9:58 AM, Richard Haines
>> <richard_c_haines@btinternet.com> wrote:
>> > Add support to label SCTP associations and cater for a situation
>> > where
>> > family = PF_INET6 with an ip_hdr(skb)->version = 4.
>> >
>> > Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
>> > ---
>> > include/net/netlabel.h | 3 ++
>> > net/netlabel/netlabel_kapi.c | 80
>> > +++++++++++++++++++++++++++++++++++++++
>> > net/netlabel/netlabel_unlabeled.c | 10 +++++
>> > 3 files changed, 93 insertions(+)
>> > /**
>> > + * netlbl_sctp_setattr - Label an incoming sctp association socket
>> > using
>> > + * the correct protocol
>> > + * @sk: the socket to label
>> > + * @skb: the packet
>> > + * @secattr: the security attributes
>> > + *
>> > + * Description:
>> > + * Attach the correct label to the given socket using the security
>> > attributes
>> > + * specified in @secattr. Returns zero on success, negative
>> > values on failure.
>> > + *
>> > + */
>> > +int netlbl_sctp_setattr(struct sock *sk,
>> > + struct sk_buff *skb,
>> > + const struct netlbl_lsm_secattr *secattr)
>> > +{
>> > + int ret_val = -EINVAL;
>> > + struct netlbl_dommap_def *entry;
>> > + struct iphdr *hdr4;
>> > +#if IS_ENABLED(CONFIG_IPV6)
>> > + struct ipv6hdr *hdr6;
>> > +#endif
>> > +
>> > + rcu_read_lock();
>> > + switch (sk->sk_family) {
>> > + case AF_INET:
>> > + hdr4 = ip_hdr(skb);
>> > +
>> > + entry = netlbl_domhsh_getentry_af4(secattr->domain,
>> > + hdr4->saddr);
>> > + if (entry == NULL) {
>> > + ret_val = -ENOENT;
>> > + goto sctp_setattr_return;
>> > + }
>> > + switch (entry->type) {
>> > + case NETLBL_NLTYPE_CIPSOV4:
>> > + ret_val = cipso_v4_sock_setattr(sk, entry-
>> > >cipso,
>> > + secattr);
>> > + break;
>> > + case NETLBL_NLTYPE_UNLABELED:
>> > + netlbl_sock_delattr(sk);
>> > + ret_val = 0;
>> > + break;
>> > + default:
>> > + ret_val = -ENOENT;
>> > + }
>> > + break;
>> > +#if IS_ENABLED(CONFIG_IPV6)
>> > + case AF_INET6:
>> > + hdr6 = ipv6_hdr(skb);
>> > + entry = netlbl_domhsh_getentry_af6(secattr->domain,
>> > + &hdr6->saddr);
>> > + if (entry == NULL) {
>> > + ret_val = -ENOENT;
>> > + goto sctp_setattr_return;
>> > + }
>> > + switch (entry->type) {
>> > + case NETLBL_NLTYPE_CALIPSO:
>> > + ret_val = calipso_sock_setattr(sk, entry-
>> > >calipso,
>> > + secattr);
>> > + break;
>> > + case NETLBL_NLTYPE_UNLABELED:
>> > + netlbl_sock_delattr(sk);
>> > + ret_val = 0;
>> > + break;
>> > + default:
>> > + ret_val = -ENOENT;
>> > + }
>> > + break;
>> > +#endif /* IPv6 */
>> > + default:
>> > + ret_val = -EPROTONOSUPPORT;
>> > + }
>> > +
>> > +sctp_setattr_return:
>> > + rcu_read_unlock();
>> > + return ret_val;
>> > +}
>>
>> It seems like we should try to leverage the code in
>> netlbl_conn_setattr() a bit more. I would suggest either tweaking
>> the
>> callers to use a sockaddr struct and netlbl_conn_setattr(), or
>> implement netlbl_sctp_setattr() as a simple wrapper around
>> netlbl_conn_setattr() ... the former seems a bit cleaner, but I
>> suspect patch 5/5 will make it clear which approach is better.
>
> I've now modified selinux_netlbl_sctp_assoc_request() to extract the ip
> address info from skb and add to a sockaddr struct, then call
> netlbl_conn_setattr(). This removes any specific SCTP code from
> netlabel_kapi.c
Great, I think that's probably the best option.
>> > diff --git a/net/netlabel/netlabel_unlabeled.c
>> > b/net/netlabel/netlabel_unlabeled.c
>> > index 22dc1b9..c070dfc 100644
>> > --- a/net/netlabel/netlabel_unlabeled.c
>> > +++ b/net/netlabel/netlabel_unlabeled.c
>> > @@ -1472,6 +1472,16 @@ int netlbl_unlabel_getattr(const struct
>> > sk_buff *skb,
>> > iface = rcu_dereference(netlbl_unlhsh_def);
>> > if (iface == NULL || !iface->valid)
>> > goto unlabel_getattr_nolabel;
>> > +
>> > +#if IS_ENABLED(CONFIG_IPV6)
>> > + /* When resolving a fallback label, check the sk_buff
>> > version as
>> > + * it is possible (e.g. SCTP) to have family = PF_INET6
>> > while
>> > + * receiving ip_hdr(skb)->version = 4.
>> > + */
>> > + if (family == PF_INET6 && ip_hdr(skb)->version == 4)
>> > + family = PF_INET;
>> > +#endif /* IPv6 */
>> > +
>>
>> It seems like this should be pulled out into it's own patch as a fix
>> that extends beyond SCTP, what do you think?
>
> I'll submit this as a separate NetLabel patch today.
Saw it and ACK'd it, thank you.
> Thanks for all your comments on the patchset, will probably take a few
> weeks to respond to them all.
No worries, we've waited some time for someone to implement proper
SCTP support, a few more weeks isn't going to hurt anyone :)
Also, apologies for some confusion on my part regarding SCTP in my
latest comments; I've since learned I wasn't thinking about endpoints
and associations correctly.
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 4/5] netlabel: Add SCTP support
@ 2017-11-13 22:26 ` Paul Moore
0 siblings, 0 replies; 12+ messages in thread
From: Paul Moore @ 2017-11-13 22:26 UTC (permalink / raw)
To: linux-security-module
On Mon, Nov 13, 2017 at 3:50 PM, Richard Haines
<richard_c_haines@btinternet.com> wrote:
> On Mon, 2017-11-06 at 18:15 -0500, Paul Moore wrote:
>> On Tue, Oct 17, 2017 at 9:58 AM, Richard Haines
>> <richard_c_haines@btinternet.com> wrote:
>> > Add support to label SCTP associations and cater for a situation
>> > where
>> > family = PF_INET6 with an ip_hdr(skb)->version = 4.
>> >
>> > Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
>> > ---
>> > include/net/netlabel.h | 3 ++
>> > net/netlabel/netlabel_kapi.c | 80
>> > +++++++++++++++++++++++++++++++++++++++
>> > net/netlabel/netlabel_unlabeled.c | 10 +++++
>> > 3 files changed, 93 insertions(+)
>> > /**
>> > + * netlbl_sctp_setattr - Label an incoming sctp association socket
>> > using
>> > + * the correct protocol
>> > + * @sk: the socket to label
>> > + * @skb: the packet
>> > + * @secattr: the security attributes
>> > + *
>> > + * Description:
>> > + * Attach the correct label to the given socket using the security
>> > attributes
>> > + * specified in @secattr. Returns zero on success, negative
>> > values on failure.
>> > + *
>> > + */
>> > +int netlbl_sctp_setattr(struct sock *sk,
>> > + struct sk_buff *skb,
>> > + const struct netlbl_lsm_secattr *secattr)
>> > +{
>> > + int ret_val = -EINVAL;
>> > + struct netlbl_dommap_def *entry;
>> > + struct iphdr *hdr4;
>> > +#if IS_ENABLED(CONFIG_IPV6)
>> > + struct ipv6hdr *hdr6;
>> > +#endif
>> > +
>> > + rcu_read_lock();
>> > + switch (sk->sk_family) {
>> > + case AF_INET:
>> > + hdr4 = ip_hdr(skb);
>> > +
>> > + entry = netlbl_domhsh_getentry_af4(secattr->domain,
>> > + hdr4->saddr);
>> > + if (entry = NULL) {
>> > + ret_val = -ENOENT;
>> > + goto sctp_setattr_return;
>> > + }
>> > + switch (entry->type) {
>> > + case NETLBL_NLTYPE_CIPSOV4:
>> > + ret_val = cipso_v4_sock_setattr(sk, entry-
>> > >cipso,
>> > + secattr);
>> > + break;
>> > + case NETLBL_NLTYPE_UNLABELED:
>> > + netlbl_sock_delattr(sk);
>> > + ret_val = 0;
>> > + break;
>> > + default:
>> > + ret_val = -ENOENT;
>> > + }
>> > + break;
>> > +#if IS_ENABLED(CONFIG_IPV6)
>> > + case AF_INET6:
>> > + hdr6 = ipv6_hdr(skb);
>> > + entry = netlbl_domhsh_getentry_af6(secattr->domain,
>> > + &hdr6->saddr);
>> > + if (entry = NULL) {
>> > + ret_val = -ENOENT;
>> > + goto sctp_setattr_return;
>> > + }
>> > + switch (entry->type) {
>> > + case NETLBL_NLTYPE_CALIPSO:
>> > + ret_val = calipso_sock_setattr(sk, entry-
>> > >calipso,
>> > + secattr);
>> > + break;
>> > + case NETLBL_NLTYPE_UNLABELED:
>> > + netlbl_sock_delattr(sk);
>> > + ret_val = 0;
>> > + break;
>> > + default:
>> > + ret_val = -ENOENT;
>> > + }
>> > + break;
>> > +#endif /* IPv6 */
>> > + default:
>> > + ret_val = -EPROTONOSUPPORT;
>> > + }
>> > +
>> > +sctp_setattr_return:
>> > + rcu_read_unlock();
>> > + return ret_val;
>> > +}
>>
>> It seems like we should try to leverage the code in
>> netlbl_conn_setattr() a bit more. I would suggest either tweaking
>> the
>> callers to use a sockaddr struct and netlbl_conn_setattr(), or
>> implement netlbl_sctp_setattr() as a simple wrapper around
>> netlbl_conn_setattr() ... the former seems a bit cleaner, but I
>> suspect patch 5/5 will make it clear which approach is better.
>
> I've now modified selinux_netlbl_sctp_assoc_request() to extract the ip
> address info from skb and add to a sockaddr struct, then call
> netlbl_conn_setattr(). This removes any specific SCTP code from
> netlabel_kapi.c
Great, I think that's probably the best option.
>> > diff --git a/net/netlabel/netlabel_unlabeled.c
>> > b/net/netlabel/netlabel_unlabeled.c
>> > index 22dc1b9..c070dfc 100644
>> > --- a/net/netlabel/netlabel_unlabeled.c
>> > +++ b/net/netlabel/netlabel_unlabeled.c
>> > @@ -1472,6 +1472,16 @@ int netlbl_unlabel_getattr(const struct
>> > sk_buff *skb,
>> > iface = rcu_dereference(netlbl_unlhsh_def);
>> > if (iface = NULL || !iface->valid)
>> > goto unlabel_getattr_nolabel;
>> > +
>> > +#if IS_ENABLED(CONFIG_IPV6)
>> > + /* When resolving a fallback label, check the sk_buff
>> > version as
>> > + * it is possible (e.g. SCTP) to have family = PF_INET6
>> > while
>> > + * receiving ip_hdr(skb)->version = 4.
>> > + */
>> > + if (family = PF_INET6 && ip_hdr(skb)->version = 4)
>> > + family = PF_INET;
>> > +#endif /* IPv6 */
>> > +
>>
>> It seems like this should be pulled out into it's own patch as a fix
>> that extends beyond SCTP, what do you think?
>
> I'll submit this as a separate NetLabel patch today.
Saw it and ACK'd it, thank you.
> Thanks for all your comments on the patchset, will probably take a few
> weeks to respond to them all.
No worries, we've waited some time for someone to implement proper
SCTP support, a few more weeks isn't going to hurt anyone :)
Also, apologies for some confusion on my part regarding SCTP in my
latest comments; I've since learned I wasn't thinking about endpoints
and associations correctly.
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH 4/5] netlabel: Add SCTP support
@ 2017-11-13 22:26 ` Paul Moore
0 siblings, 0 replies; 12+ messages in thread
From: Paul Moore @ 2017-11-13 22:26 UTC (permalink / raw)
To: linux-security-module
On Mon, Nov 13, 2017 at 3:50 PM, Richard Haines
<richard_c_haines@btinternet.com> wrote:
> On Mon, 2017-11-06 at 18:15 -0500, Paul Moore wrote:
>> On Tue, Oct 17, 2017 at 9:58 AM, Richard Haines
>> <richard_c_haines@btinternet.com> wrote:
>> > Add support to label SCTP associations and cater for a situation
>> > where
>> > family = PF_INET6 with an ip_hdr(skb)->version = 4.
>> >
>> > Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
>> > ---
>> > include/net/netlabel.h | 3 ++
>> > net/netlabel/netlabel_kapi.c | 80
>> > +++++++++++++++++++++++++++++++++++++++
>> > net/netlabel/netlabel_unlabeled.c | 10 +++++
>> > 3 files changed, 93 insertions(+)
>> > /**
>> > + * netlbl_sctp_setattr - Label an incoming sctp association socket
>> > using
>> > + * the correct protocol
>> > + * @sk: the socket to label
>> > + * @skb: the packet
>> > + * @secattr: the security attributes
>> > + *
>> > + * Description:
>> > + * Attach the correct label to the given socket using the security
>> > attributes
>> > + * specified in @secattr. Returns zero on success, negative
>> > values on failure.
>> > + *
>> > + */
>> > +int netlbl_sctp_setattr(struct sock *sk,
>> > + struct sk_buff *skb,
>> > + const struct netlbl_lsm_secattr *secattr)
>> > +{
>> > + int ret_val = -EINVAL;
>> > + struct netlbl_dommap_def *entry;
>> > + struct iphdr *hdr4;
>> > +#if IS_ENABLED(CONFIG_IPV6)
>> > + struct ipv6hdr *hdr6;
>> > +#endif
>> > +
>> > + rcu_read_lock();
>> > + switch (sk->sk_family) {
>> > + case AF_INET:
>> > + hdr4 = ip_hdr(skb);
>> > +
>> > + entry = netlbl_domhsh_getentry_af4(secattr->domain,
>> > + hdr4->saddr);
>> > + if (entry == NULL) {
>> > + ret_val = -ENOENT;
>> > + goto sctp_setattr_return;
>> > + }
>> > + switch (entry->type) {
>> > + case NETLBL_NLTYPE_CIPSOV4:
>> > + ret_val = cipso_v4_sock_setattr(sk, entry-
>> > >cipso,
>> > + secattr);
>> > + break;
>> > + case NETLBL_NLTYPE_UNLABELED:
>> > + netlbl_sock_delattr(sk);
>> > + ret_val = 0;
>> > + break;
>> > + default:
>> > + ret_val = -ENOENT;
>> > + }
>> > + break;
>> > +#if IS_ENABLED(CONFIG_IPV6)
>> > + case AF_INET6:
>> > + hdr6 = ipv6_hdr(skb);
>> > + entry = netlbl_domhsh_getentry_af6(secattr->domain,
>> > + &hdr6->saddr);
>> > + if (entry == NULL) {
>> > + ret_val = -ENOENT;
>> > + goto sctp_setattr_return;
>> > + }
>> > + switch (entry->type) {
>> > + case NETLBL_NLTYPE_CALIPSO:
>> > + ret_val = calipso_sock_setattr(sk, entry-
>> > >calipso,
>> > + secattr);
>> > + break;
>> > + case NETLBL_NLTYPE_UNLABELED:
>> > + netlbl_sock_delattr(sk);
>> > + ret_val = 0;
>> > + break;
>> > + default:
>> > + ret_val = -ENOENT;
>> > + }
>> > + break;
>> > +#endif /* IPv6 */
>> > + default:
>> > + ret_val = -EPROTONOSUPPORT;
>> > + }
>> > +
>> > +sctp_setattr_return:
>> > + rcu_read_unlock();
>> > + return ret_val;
>> > +}
>>
>> It seems like we should try to leverage the code in
>> netlbl_conn_setattr() a bit more. I would suggest either tweaking
>> the
>> callers to use a sockaddr struct and netlbl_conn_setattr(), or
>> implement netlbl_sctp_setattr() as a simple wrapper around
>> netlbl_conn_setattr() ... the former seems a bit cleaner, but I
>> suspect patch 5/5 will make it clear which approach is better.
>
> I've now modified selinux_netlbl_sctp_assoc_request() to extract the ip
> address info from skb and add to a sockaddr struct, then call
> netlbl_conn_setattr(). This removes any specific SCTP code from
> netlabel_kapi.c
Great, I think that's probably the best option.
>> > diff --git a/net/netlabel/netlabel_unlabeled.c
>> > b/net/netlabel/netlabel_unlabeled.c
>> > index 22dc1b9..c070dfc 100644
>> > --- a/net/netlabel/netlabel_unlabeled.c
>> > +++ b/net/netlabel/netlabel_unlabeled.c
>> > @@ -1472,6 +1472,16 @@ int netlbl_unlabel_getattr(const struct
>> > sk_buff *skb,
>> > iface = rcu_dereference(netlbl_unlhsh_def);
>> > if (iface == NULL || !iface->valid)
>> > goto unlabel_getattr_nolabel;
>> > +
>> > +#if IS_ENABLED(CONFIG_IPV6)
>> > + /* When resolving a fallback label, check the sk_buff
>> > version as
>> > + * it is possible (e.g. SCTP) to have family = PF_INET6
>> > while
>> > + * receiving ip_hdr(skb)->version = 4.
>> > + */
>> > + if (family == PF_INET6 && ip_hdr(skb)->version == 4)
>> > + family = PF_INET;
>> > +#endif /* IPv6 */
>> > +
>>
>> It seems like this should be pulled out into it's own patch as a fix
>> that extends beyond SCTP, what do you think?
>
> I'll submit this as a separate NetLabel patch today.
Saw it and ACK'd it, thank you.
> Thanks for all your comments on the patchset, will probably take a few
> weeks to respond to them all.
No worries, we've waited some time for someone to implement proper
SCTP support, a few more weeks isn't going to hurt anyone :)
Also, apologies for some confusion on my part regarding SCTP in my
latest comments; I've since learned I wasn't thinking about endpoints
and associations correctly.
--
paul moore
www.paul-moore.com
--
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] 12+ messages in thread
end of thread, other threads:[~2017-11-13 22:26 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-17 13:58 [RFC PATCH 4/5] netlabel: Add SCTP support Richard Haines
2017-10-17 13:58 ` Richard Haines
2017-10-17 13:58 ` Richard Haines
2017-11-06 23:15 ` Paul Moore
2017-11-06 23:15 ` Paul Moore
2017-11-06 23:15 ` Paul Moore
2017-11-13 20:50 ` Richard Haines
2017-11-13 20:50 ` Richard Haines
2017-11-13 20:50 ` Richard Haines
2017-11-13 22:26 ` Paul Moore
2017-11-13 22:26 ` Paul Moore
2017-11-13 22:26 ` Paul Moore
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.