Re: [RFC PATCH 16/17] calipso: Add validation of CALIPSO option.
On Tue, Dec 22, 2015 at 10:47:43PM +0100, Hannes Frederic Sowa wrote: > On 22.12.2015 17:59, Huw Davies wrote: > > I'm confused about this one. AFAICS, this will drop packets that we > > can't process. We don't send the icmp error, but I can certainly add > > that. Is that what you mean? > > Actually, the implementation of calipso_validate will accept the packets > because it defaults to return true if we don't compile the module. At > least we should drop the packet if it is not loaded. I am in favor of > adding the parameter problem icmp error. So, yes, I think it should be > added. Yet the option value is 0x07, i.e. the two highest bits are both zero which according to: https://tools.ietf.org/html/rfc2460#section-4.2 means we should just skip it. https://tools.ietf.org/html/rfc5570#section-5.1.1 reaffirms that. In terms of sending an icmp on error while validating: https://tools.ietf.org/html/rfc5570#section-6.2.2 is pretty conservative in that case too. Most errors should just be silently dropped. Huw. -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 16/17] calipso: Add validation of CALIPSO option.
On Tue, Dec 22, 2015 at 02:50:20PM +0100, Hannes Frederic Sowa wrote:
> On 22.12.2015 12:46, Huw Davies wrote:
> >
> > +/* CALIPSO RFC 5570 */
> > +
> > +static bool ipv6_hop_calipso(struct sk_buff *skb, int optoff)
> > +{
> > + const unsigned char *nh = skb_network_header(skb);
> > +
> > + if (nh[optoff + 1] < 8)
> > + goto drop;
> > +
> > + if (nh[optoff + 6] * 4 + 8 > nh[optoff + 1])
> > + goto drop;
> > +
> > + if (!calipso_validate(skb, nh + optoff))
> > + goto drop;
> > +
> > + return true;
> > +
> > +drop:
> > + kfree_skb(skb);
> > + return false;
> > +}
> > +
>
> Formally, if an extension header could not be processed, the packet
> should be discarded and an icmp error parameter extension should be
> send. I think we shouldn't let those packets pass here.
Thanks for your comments Hannes, I'm looking into your other
suggestions.
I'm confused about this one. AFAICS, this will drop packets that we
can't process. We don't send the icmp error, but I can certainly add
that. Is that what you mean?
Thanks,
Huw.
--
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 16/17] calipso: Add validation of CALIPSO option.
On 22.12.2015 12:46, Huw Davies wrote:
>
> +/* CALIPSO RFC 5570 */
> +
> +static bool ipv6_hop_calipso(struct sk_buff *skb, int optoff)
> +{
> + const unsigned char *nh = skb_network_header(skb);
> +
> + if (nh[optoff + 1] < 8)
> + goto drop;
> +
> + if (nh[optoff + 6] * 4 + 8 > nh[optoff + 1])
> + goto drop;
> +
> + if (!calipso_validate(skb, nh + optoff))
> + goto drop;
> +
> + return true;
> +
> +drop:
> + kfree_skb(skb);
> + return false;
> +}
> +
Formally, if an extension header could not be processed, the packet
should be discarded and an icmp error parameter extension should be
send. I think we shouldn't let those packets pass here.
Thanks,
Hannes
--
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
