* [PATCH] cipso: handle CIPSO options correctly when NetLabel is disabled
@ 2012-05-31 20:09 Paul Moore
2012-05-31 23:07 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Paul Moore @ 2012-05-31 20:09 UTC (permalink / raw)
To: netdev; +Cc: linux-security-module, stable
When NetLabel is not enabled, e.g. CONFIG_NETLABEL=n, and the system
receives a CIPSO tagged packet it is dropped (cipso_v4_validate()
returns non-zero). In most cases this is the correct and desired
behavior, however, in the case where we are simply forwarding the
traffic, e.g. acting as a network bridge, this becomes a problem.
This patch fixes the forwarding problem by providing the basic CIPSO
validation code directly in ip_options_compile() without the need for
the NetLabel or CIPSO code. The new validation code can not perform
any of the CIPSO option label/value verification that
cipso_v4_validate() does, but it can verify the basic CIPSO option
format.
The behavior when NetLabel is enabled is unchanged.
Signed-off-by: Paul Moore <pmoore@redhat.com>
---
net/ipv4/ip_options.c | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
index 708b994..ca2c919 100644
--- a/net/ipv4/ip_options.c
+++ b/net/ipv4/ip_options.c
@@ -439,10 +439,30 @@ int ip_options_compile(struct net *net,
goto error;
}
opt->cipso = optptr - iph;
+#ifndef CONFIG_NETLABEL
+ if (optlen < 8) {
+ pp_ptr = optptr + 1;
+ goto error;
+ }
+ if (get_unaligned_be32(&optptr[2]) != 0) {
+ unsigned int iter;
+ for (iter = 6; iter < optlen;) {
+ if (optptr[iter+1] > (optlen - iter)) {
+ pp_ptr = optptr + iter;
+ goto error;
+ }
+ iter += optptr[iter + 1];
+ }
+ } else {
+ pp_ptr = optptr + 2;
+ goto error;
+ }
+#else
if (cipso_v4_validate(skb, &optptr)) {
pp_ptr = optptr;
goto error;
}
+#endif /* CONFIG_NETLABEL */
break;
case IPOPT_SEC:
case IPOPT_SID:
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] cipso: handle CIPSO options correctly when NetLabel is disabled
2012-05-31 20:09 [PATCH] cipso: handle CIPSO options correctly when NetLabel is disabled Paul Moore
@ 2012-05-31 23:07 ` David Miller
2012-06-01 13:14 ` Paul Moore
0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2012-05-31 23:07 UTC (permalink / raw)
To: pmoore; +Cc: netdev, linux-security-module, stable
From: Paul Moore <pmoore@redhat.com>
Date: Thu, 31 May 2012 16:09:23 -0400
> When NetLabel is not enabled, e.g. CONFIG_NETLABEL=n, and the system
> receives a CIPSO tagged packet it is dropped (cipso_v4_validate()
> returns non-zero). In most cases this is the correct and desired
> behavior, however, in the case where we are simply forwarding the
> traffic, e.g. acting as a network bridge, this becomes a problem.
>
> This patch fixes the forwarding problem by providing the basic CIPSO
> validation code directly in ip_options_compile() without the need for
> the NetLabel or CIPSO code. The new validation code can not perform
> any of the CIPSO option label/value verification that
> cipso_v4_validate() does, but it can verify the basic CIPSO option
> format.
>
> The behavior when NetLabel is enabled is unchanged.
>
> Signed-off-by: Paul Moore <pmoore@redhat.com>
I don't like this at all.
The only conclusion I can come to is that cipso_v4_validate() is doing
the wrong thing when NETLABEL is disabled.
There is never a good reason to crap all over a function with ifdefs.
This is especially true when it's being done to paper over a function
with poor semantics.
The whole idea is to abstract and put all of this kind of logic into
cipso_v4_validate().
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] cipso: handle CIPSO options correctly when NetLabel is disabled
2012-05-31 23:07 ` David Miller
@ 2012-06-01 13:14 ` Paul Moore
0 siblings, 0 replies; 3+ messages in thread
From: Paul Moore @ 2012-06-01 13:14 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-security-module
On Thursday, May 31, 2012 07:07:05 PM David Miller wrote:
> From: Paul Moore <pmoore@redhat.com>
> Date: Thu, 31 May 2012 16:09:23 -0400
>
> > When NetLabel is not enabled, e.g. CONFIG_NETLABEL=n, and the system
> > receives a CIPSO tagged packet it is dropped (cipso_v4_validate()
> > returns non-zero). In most cases this is the correct and desired
> > behavior, however, in the case where we are simply forwarding the
> > traffic, e.g. acting as a network bridge, this becomes a problem.
> >
> > This patch fixes the forwarding problem by providing the basic CIPSO
> > validation code directly in ip_options_compile() without the need for
> > the NetLabel or CIPSO code. The new validation code can not perform
> > any of the CIPSO option label/value verification that
> > cipso_v4_validate() does, but it can verify the basic CIPSO option
> > format.
> >
> > The behavior when NetLabel is enabled is unchanged.
> >
> > Signed-off-by: Paul Moore <pmoore@redhat.com>
>
> I don't like this at all.
>
> The only conclusion I can come to is that cipso_v4_validate() is doing
> the wrong thing when NETLABEL is disabled.
>
> There is never a good reason to crap all over a function with ifdefs.
> This is especially true when it's being done to paper over a function
> with poor semantics.
>
> The whole idea is to abstract and put all of this kind of logic into
> cipso_v4_validate().
I originally had the #ifdef'd code in the non-CONFIG_NETLABEL
cipso_v4_validate() in include/net/cipso_ipv4.h but thought it was too much
code to put there. No worries, I'll just move it back and resubmit.
--
paul moore
security and virtualization @ redhat
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-06-01 13:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-31 20:09 [PATCH] cipso: handle CIPSO options correctly when NetLabel is disabled Paul Moore
2012-05-31 23:07 ` David Miller
2012-06-01 13:14 ` Paul Moore
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).