| Message ID | 20120531200922.6265.81763.stgit@sifl |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
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(). -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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:
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(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html