calipso: Add validation of CALIPSO option.
Subsystems (5)
RFC PATCH v3 17/19 calipso: Add validation of CALIPSO option.
We check lengths, checksum and the DOI. We leave checking of the level and categories for the socket layer.
--- include/net/calipso.h | 6 ++++++ net/ipv6/calipso.c | 42 ++++++++++++++++++++++++++++++++++++++++++ net/ipv6/exthdrs.c | 27 +++++++++++++++++++++++++++ 3 files changed, 75 insertions(+)
include/net/calipso.h +6 -0
@@ -65,6 +65,7 @@ struct calipso_doi {#ifdef CONFIG_NETLABEL int __init calipso_init (void );void calipso_exit (void ); +bool calipso_validate (const struct sk_buff *skb,const unsigned char *option);#else static inline int __init calipso_init (void ) {
@@ -74,6 +75,11 @@ static inline int __init calipso_init(void)static inline void calipso_exit (void ) { } +static inline bool calipso_validate (const struct sk_buff *skb, +const unsigned char *option) +{ +return true; +}#endif /* CONFIG_NETLABEL */ #endif /* _CALIPSO_H */
net/ipv6/calipso.c +42 -0
48 lines not on this page · raw message
net/ipv6/exthdrs.c +27 -0
@@ -43,6 +43,7 @@#include <net/ndisc.h> #include <net/ip6_route.h> #include <net/addrconf.h> +#include <net/calipso.h> #if IS_ENABLED (CONFIG_IPV6_MIP6 )#include <net/xfrm.h> #endif
@@ -603,6 +604,28 @@ drop:return false; } +/* 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; +} +static const struct tlvtype_proc tlvprochopopt_lst[] = { { .type =IPV6_TLV_ROUTERALERT ,
@@ -612,6 +635,10 @@ static const struct tlvtype_proc tlvprochopopt_lst[] = { .type = IPV6_TLV_JUMBO, .func = ipv6_hop_jumbo, }, + { + .type = IPV6_TLV_CALIPSO, + .func = ipv6_hop_calipso, + }, { -1, } };
Signature · 1 line
2.7.0
Paul Moore
maintainerQuoted 62 lines · Huw Davies
We check lengths, checksum and the DOI. We leave checking of the level and categories for the socket layer.
Signed-off-by Huw Davies <[email protected]>--- include/net/calipso.h | 6 ++++++ net/ipv6/calipso.c | 42 ++++++++++++++++++++++++++++++++++++++++++ net/ipv6/exthdrs.c | 27 +++++++++++++++++++++++++++ 3 files changed, 75 insertions(+)include/net/calipso.h +6
-0@@ -65,6 +65,7 @@ struct calipso_doi {#ifdef CONFIG_NETLABEL int __init calipso_init (void );void calipso_exit (void ); +bool calipso_validate (const struct sk_buff *skb,const unsigned char *option);#else static inline int __init calipso_init (void ) {@@ -74,6 +75,11 @@ static inline int __init calipso_init(void)static inline void calipso_exit (void ) { } +static inline bool calipso_validate (const struct sk_buff *skb, +const unsigned char *option) +{ +return true; +}#endif /* CONFIG_NETLABEL */ #endif /* _CALIPSO_H */ net/ipv6/calipso.c +20
-0@@ -321,6 +321,48 @@ doi_walk_return: }/** +* calipso_validate - Validate a CALIPSO option +* @skb: the packet +* @option: the start of the option +* +* Description: +* This routine is called to validate a CALIPSO option. +* If the option is valid then a zero value is returned. If the +* option is invalid then a non-zero value is returned and +* representing the offset to the offending portion of the option. +* +* The caller should have already checked that the length of the +* option (including the TLV header) is >= 10 and that the catmap +* length is consistent with the option length. +* +* We leave checks on the level and categories to the socket layer. +*/ +bool calipso_validate (const struct sk_buff *skb,const unsigned char *option) +{ +struct calipso_doi *doi_def; +int ret_val;
net/ipv6/calipso.c doi_walk_return: · line 353
Quoted 10 lines
net/ipv6/calipso.c +10
-0+u16 crc, len = option[1 ] +2 ; +static const u8 zero[2 ]; + +/* The original CRC runs over the option including the TLV header +* with the CRC-16 field (at offset 8) zeroed out. */ + crc =crc_ccitt (0xffff , option,8 ); + crc =crc_ccitt (crc, zero,sizeof (zero)); +if (len >10 ) + crc =crc_ccitt (crc, option +10 , len -10 ); + crc = ~crc;
I should have caught this in the v2 patchset when I mentioned it with respect to the CRC generation, but why not simply do 'crc = ~crc_cccitt(...);'?
Also, while I'm looking at this, why not do the CRC verification in ipv6_hop_calipso()? The only thing we should need to do here is the DOI lookup/verification so that we still work correctly when CONFIG_NETLABEL=n; all the core protocol stuff, e.g. length and checksum validation, should be done in the core stack functions, e.g. ipv6_hop_calipso().
Quoted 74 lines
net/ipv6/calipso.c +12
-0+if (option[8 ] != (crc &0xff ) || option[9 ] != ((crc >>8 ) &0xff )) +return false; + +rcu_read_lock (); + doi_def =calipso_doi_search (get_unaligned_be32 (option +2 )); + ret_val = !!doi_def; +rcu_read_unlock (); + +return ret_val; +} + +/** * calipso_map_cat_hton - Perform a category mapping from host to network * @doi_def: the DOI definition * @secattr: the security attributes net/ipv6/exthdrs.c +27
-1@@ -43,6 +43,7 @@#include <net/ndisc.h> #include <net/ip6_route.h> #include <net/addrconf.h> +#include <net/calipso.h> #if IS_ENABLED (CONFIG_IPV6_MIP6 )#include <net/xfrm.h> #endif @@ -603,6 +604,28 @@ drop:return false; } +/* 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; +} +static const struct tlvtype_proc tlvprochopopt_lst[] = { { .type =IPV6_TLV_ROUTERALERT ,@@ -612,6 +635,10 @@ static const struct tlvtype_proc tlvprochopopt_lst[] = { .type = IPV6_TLV_JUMBO, .func = ipv6_hop_jumbo, }, + { + .type = IPV6_TLV_CALIPSO, + .func = ipv6_hop_calipso, + }, { -1, } }; --2.7.0
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 2 lines
paul moore www.paul-moore.com
A message that is not in the archive (sent off-list or not yet received): CAHC9VhTRp8bHgwxZ2YNj=pa2a4PtO=8jJT1siG99GiFYnwm53Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Quoted 69 lines · Paul Moore
Quoted 68 lines · Huw Davies
We check lengths, checksum and the DOI. We leave checking of the level and categories for the socket layer.
Signed-off-by Huw Davies <[email protected]>--- include/net/calipso.h | 6 ++++++ net/ipv6/calipso.c | 42 ++++++++++++++++++++++++++++++++++++++++++ net/ipv6/exthdrs.c | 27 +++++++++++++++++++++++++++ 3 files changed, 75 insertions(+)include/net/calipso.h +6
-0@@ -65,6 +65,7 @@ struct calipso_doi {#ifdef CONFIG_NETLABEL int __init calipso_init (void );void calipso_exit (void ); +bool calipso_validate (const struct sk_buff *skb,const unsigned char *option);#else static inline int __init calipso_init (void ) {@@ -74,6 +75,11 @@ static inline int __init calipso_init(void)static inline void calipso_exit (void ) { } +static inline bool calipso_validate (const struct sk_buff *skb, +const unsigned char *option) +{ +return true; +}#endif /* CONFIG_NETLABEL */ #endif /* _CALIPSO_H */ net/ipv6/calipso.c +26
-0@@ -321,6 +321,48 @@ doi_walk_return: }/** +* calipso_validate - Validate a CALIPSO option +* @skb: the packet +* @option: the start of the option +* +* Description: +* This routine is called to validate a CALIPSO option. +* If the option is valid then a zero value is returned. If the +* option is invalid then a non-zero value is returned and +* representing the offset to the offending portion of the option. +* +* The caller should have already checked that the length of the +* option (including the TLV header) is >= 10 and that the catmap +* length is consistent with the option length. +* +* We leave checks on the level and categories to the socket layer. +*/ +bool calipso_validate (const struct sk_buff *skb,const unsigned char *option) +{ +struct calipso_doi *doi_def; +int ret_val; +u16 crc, len = option[1 ] +2 ; +static const u8 zero[2 ]; + +/* The original CRC runs over the option including the TLV header +* with the CRC-16 field (at offset 8) zeroed out. */ + crc =crc_ccitt (0xffff , option,8 );
Quoted 8 lines
net/ipv6/calipso.c doi_walk_return: · line 353
Quoted 4 lines
net/ipv6/calipso.c +4
-0+ crc =crc_ccitt (crc, zero,sizeof (zero)); +if (len >10 ) + crc =crc_ccitt (crc, option +10 , len -10 ); + crc = ~crc;I should have caught this in the v2 patchset when I mentioned it with respect to the CRC generation, but why not simply do 'crc = ~crc_cccitt(...);'?
Simply because the final crc_ccitt() is inside an if statement. Since len is guaranteed to be >= 10, I could dispense with the if and have crc_ccitt() handle having its final argument equal to zero (which it does just fine). Then I could do as you suggest.
Quoted 6 lines
Also, while I'm looking at this, why not do the CRC verification in
ipv6_hop_calipso()? The only thing we should need to do here is the DOI lookup/verification so that we still work correctly whenCONFIG_NETLABEL=n; all the core protocol stuff, e.g. length and checksum validation, should be done in the core stack functions, e.g.ipv6_hop_calipso().
The only reason was to not bring in CONFIG_CRC_CCITT if CONFIG_NETLABEL=n (this is currently selected in net/netlabel/Kconfig).
If folks are happy for me to do so, I could change this to:
select CONFIG_CRC_CCITT
under menuconfig IPV6
Then the crc check could move to exthdrs.c:ipv6_hop_calipso(). Would that be ok?
Thanks,
Huw.
Quoted 67 lines
Quoted 67 lines
net/ipv6/calipso.c +12
-0+if (option[8 ] != (crc &0xff ) || option[9 ] != ((crc >>8 ) &0xff )) +return false; + +rcu_read_lock (); + doi_def =calipso_doi_search (get_unaligned_be32 (option +2 )); + ret_val = !!doi_def; +rcu_read_unlock (); + +return ret_val; +} + +/** * calipso_map_cat_hton - Perform a category mapping from host to network * @doi_def: the DOI definition * @secattr: the security attributes net/ipv6/exthdrs.c +27
-0@@ -43,6 +43,7 @@#include <net/ndisc.h> #include <net/ip6_route.h> #include <net/addrconf.h> +#include <net/calipso.h> #if IS_ENABLED (CONFIG_IPV6_MIP6 )#include <net/xfrm.h> #endif @@ -603,6 +604,28 @@ drop:return false; } +/* 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; +} +static const struct tlvtype_proc tlvprochopopt_lst[] = { { .type =IPV6_TLV_ROUTERALERT ,@@ -612,6 +635,10 @@ static const struct tlvtype_proc tlvprochopopt_lst[] = { .type = IPV6_TLV_JUMBO, .func = ipv6_hop_jumbo, }, + { + .type = IPV6_TLV_CALIPSO, + .func = ipv6_hop_calipso, + }, { -1, } };
Quoted 11 lines
Quoted 7 lines
--
2.7.0--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.htmlSignature · 2 lines
paul moore www.paul-moore.com
_______________________________________________
Selinux mailing list
[email protected]
To unsubscribe, send email to [email protected]
To get help, send an email containing "help" to [email protected]
Paul Moore
maintainerQuoted 79 lines · Huw Davies
Quoted 69 lines · Paul Moore
Quoted 68 lines · Huw Davies
We check lengths, checksum and the DOI. We leave checking of the level and categories for the socket layer.
Signed-off-by Huw Davies <[email protected]>--- include/net/calipso.h | 6 ++++++ net/ipv6/calipso.c | 42 ++++++++++++++++++++++++++++++++++++++++++ net/ipv6/exthdrs.c | 27 +++++++++++++++++++++++++++ 3 files changed, 75 insertions(+)include/net/calipso.h +6
-0@@ -65,6 +65,7 @@ struct calipso_doi {#ifdef CONFIG_NETLABEL int __init calipso_init (void );void calipso_exit (void ); +bool calipso_validate (const struct sk_buff *skb,const unsigned char *option);#else static inline int __init calipso_init (void ) {@@ -74,6 +75,11 @@ static inline int __init calipso_init(void)static inline void calipso_exit (void ) { } +static inline bool calipso_validate (const struct sk_buff *skb, +const unsigned char *option) +{ +return true; +}#endif /* CONFIG_NETLABEL */ #endif /* _CALIPSO_H */ net/ipv6/calipso.c +26
-0@@ -321,6 +321,48 @@ doi_walk_return: }/** +* calipso_validate - Validate a CALIPSO option +* @skb: the packet +* @option: the start of the option +* +* Description: +* This routine is called to validate a CALIPSO option. +* If the option is valid then a zero value is returned. If the +* option is invalid then a non-zero value is returned and +* representing the offset to the offending portion of the option. +* +* The caller should have already checked that the length of the +* option (including the TLV header) is >= 10 and that the catmap +* length is consistent with the option length. +* +* We leave checks on the level and categories to the socket layer. +*/ +bool calipso_validate (const struct sk_buff *skb,const unsigned char *option) +{ +struct calipso_doi *doi_def; +int ret_val; +u16 crc, len = option[1 ] +2 ; +static const u8 zero[2 ]; + +/* The original CRC runs over the option including the TLV header +* with the CRC-16 field (at offset 8) zeroed out. */ + crc =crc_ccitt (0xffff , option,8 );Quoted 8 lines
net/ipv6/calipso.c doi_walk_return: · line 353
Quoted 4 lines
net/ipv6/calipso.c +4
-0+ crc =crc_ccitt (crc, zero,sizeof (zero)); +if (len >10 ) + crc =crc_ccitt (crc, option +10 , len -10 ); + crc = ~crc;I should have caught this in the v2 patchset when I mentioned it with respect to the CRC generation, but why not simply do 'crc = ~crc_cccitt(...);'?
Quoted 4 lines
Simply because the final
crc_ccitt()is inside an if statement. Since len is guaranteed to be >= 10, I could dispense with the if and havecrc_ccitt()handle having its final argument equal to zero (which it does just fine). Then I could do as you suggest.
Yes, I get that it is in an if-conditional; I probably should have been more specific. I was thinking of something more like this:
if (len > 10)
crc = ~crc(crc, option + 10, ...);
else
crc = ~crc;... although now that I've typed it out, I'm not sure it's an improvement. I would just ignore me ;)
Quoted 10 lines
Quoted 6 lines
Also, while I'm looking at this, why not do the CRC verification in
ipv6_hop_calipso()? The only thing we should need to do here is the DOI lookup/verification so that we still work correctly whenCONFIG_NETLABEL=n; all the core protocol stuff, e.g. length and checksum validation, should be done in the core stack functions, e.g.ipv6_hop_calipso().The only reason was to not bring in
CONFIG_CRC_CCITTifCONFIG_NETLABEL=n(this is currently selected innet/netlabel/Kconfig).
Quoted 6 lines
If folks are happy for me to do so, I could change this to:
selectCONFIG_CRC_CCITT
under menuconfig IPV6Then the crc check could move to exthdrs.c:ipv6_hop_calipso(). Would that be ok?
Thanks, that explains it.
I think there is value in verifying the checksum, but I completely understand if the netdev folks don't want to pull in the CRC_CCITT code, especially since currently only a few network drivers/protocols pull it into the build.
DaveM, your thoughts?
Signature · 2 lines
paul moore www.paul-moore.com