GTSM (TTL security)/RFC 5082 support?
Is anyone currently working on adding GTSM support to bird? It should be possible to support it for both Linux and FreeBSD where available as a kernel-level supported socket option, and I am considering trying my hand at it as a way to get to know the bird codebase a bit better before we decide to deploy it at work... -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Henrique de Moraes Holschuh wrote:
Is anyone currently working on adding GTSM support to bird?
It should be possible to support it for both Linux and FreeBSD where available as a kernel-level supported socket option, and I am considering trying my hand at it as a way to get to know the bird codebase a bit better before we decide to deploy it at work...
Review/comments are welcome Patch adds: * new sk_set_min_ttl() function to set minimum received TTL * new BGP (cisco-like) config option: ttl_secutity hops <value> Tested on FreeBSD, however linux part should work too. Kernel support required: Linux: IP_MINTTL is supported on 2.6.34+ IPV6_MINHOPCNT is supported on 2.6.35+ *BSD: IP_MINTTL is supported since long time ago IPV6_MINHOPCNT is not supported (at least on FreeBSD at the moment) Btw, FreeBSD IP_MINTTL support was broken somewhere between 8.1 and 8.2, 8.2-R+ should work -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.14 (FreeBSD) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk5Hp74ACgkQwcJ4iSZ1q2nyJwCfZBEdpVuwDypVoyvldOUYEPrA T3wAoKDE22Xtm7KkRJ+PC7jhgUdr/uvh =i4l8 -----END PGP SIGNATURE----- Index: proto/bgp/bgp.c =================================================================== --- proto/bgp/bgp.c (revision 4980) +++ proto/bgp/bgp.c (working copy) @@ -574,7 +574,11 @@ bgp_connect(struct bgp_proto *p) /* Enter Connect s->saddr = p->source_addr; s->daddr = p->cf->remote_ip; s->dport = BGP_PORT; - s->ttl = p->cf->multihop ? : 1; + /* TTL security support */ + if (p->cf->min_ttl > 0) + s->ttl = 255; + else + s->ttl = p->cf->multihop ? : 1; s->rbsize = BGP_RX_BUFFER_SIZE; s->tbsize = BGP_TX_BUFFER_SIZE; s->tos = IP_PREC_INTERNET_CONTROL; @@ -589,6 +593,17 @@ bgp_connect(struct bgp_proto *p) /* Enter Connect bgp_sock_err(s, 0); return; } + /* Set minimal receive TTL if needed */ + if (p->cf->min_ttl > 0) + { + log(L_ERR "Setting minimum received TTL to %d", p->cf->min_ttl); + if (sk_set_min_ttl(s, p->cf->min_ttl) != 0) + { + log(L_ERR "TTL security configuration failed, closing session"); + bgp_sock_err(s, 0); + return; + } + } DBG("BGP: Waiting for connect success\n"); bgp_start_timer(conn->connect_retry_timer, p->cf->connect_retry_time); } @@ -629,7 +644,17 @@ bgp_incoming_connection(sock *sk, int dummy UNUSED bgp_setup_conn(p, &p->incoming_conn); bgp_setup_sk(&p->incoming_conn, sk); - sk_set_ttl(sk, p->cf->multihop ? : 1); + /* TTL security support */ + if (p->cf->min_ttl > 0) + { + if ((sk_set_ttl(sk, 255) != 0) || (sk_set_min_ttl(sk, p->cf->min_ttl) != 0)) + { + log(L_ERR "TTL security configuration failed, closing session"); + goto err; + } + } + else + sk_set_ttl(sk, p->cf->multihop ? : 1); bgp_send_open(&p->incoming_conn); return 0; } Index: proto/bgp/config.Y =================================================================== --- proto/bgp/config.Y (revision 4980) +++ proto/bgp/config.Y (working copy) @@ -25,7 +25,7 @@ CF_KEYWORDS(BGP, LOCAL, NEIGHBOR, AS, HOLD, TIME, ADVERTISE, IPV4, CAPABILITIES, LIMIT, PASSIVE, PREFER, OLDER, MISSING, LLADDR, DROP, IGNORE, ROUTE, REFRESH, INTERPRET, COMMUNITIES, BGP_ORIGINATOR_ID, BGP_CLUSTER_LIST, IGP, TABLE, - GATEWAY, DIRECT, RECURSIVE, MED) + GATEWAY, DIRECT, RECURSIVE, MED, TTL_SECURITY, HOPS) CF_GRAMMAR @@ -49,6 +49,7 @@ bgp_proto_start: proto_start BGP { BGP_CFG->advertise_ipv4 = 1; BGP_CFG->interpret_communities = 1; BGP_CFG->default_local_pref = 100; + BGP_CFG->min_ttl = 0; } ; @@ -98,6 +99,7 @@ bgp_proto: | bgp_proto PASSIVE bool ';' { BGP_CFG->passive = $3; } | bgp_proto INTERPRET COMMUNITIES bool ';' { BGP_CFG->interpret_communities = $4; } | bgp_proto IGP TABLE rtable ';' { BGP_CFG->igp_table = $4; } + | bgp_proto TTL_SECURITY HOPS expr ';' { BGP_CFG->min_ttl = $4; } ; CF_ADDTO(dynamic_attr, BGP_ORIGIN Index: proto/bgp/bgp.h =================================================================== --- proto/bgp/bgp.h (revision 4980) +++ proto/bgp/bgp.h (working copy) @@ -20,6 +20,7 @@ struct bgp_config { u32 local_as, remote_as; ip_addr remote_ip; int multihop; /* Number of hops if multihop */ + int min_ttl; /* minimum TTL for TTL security */ ip_addr source_addr; /* Source address to use */ int next_hop_self; /* Always set next hop to local IP address */ int missing_lladdr; /* What we will do when we don' know link-local addr, see MLL_* */ Index: lib/socket.h =================================================================== --- lib/socket.h (revision 4980) +++ lib/socket.h (working copy) @@ -54,7 +54,8 @@ int sk_send(sock *, unsigned len); /* Send data, < int sk_send_to(sock *, unsigned len, ip_addr to, unsigned port); /* sk_send to given destination */ void sk_reallocate(sock *); /* Free and allocate tbuf & rbuf */ void sk_dump_all(void); -int sk_set_ttl(sock *s, int ttl); /* Set TTL for given socket */ +int sk_set_ttl(sock *s, int ttl); /* Set transmit TTL for given socket */ +int sk_set_min_ttl(sock *s, int ttl); /* Set minimal accepted TTL for given socket */ /* Add or remove security associations for given passive socket */ int sk_set_md5_auth(sock *s, ip_addr a, char *passwd); Index: sysdep/linux/sysio.h =================================================================== --- sysdep/linux/sysio.h (revision 4980) +++ sysdep/linux/sysio.h (working copy) @@ -309,3 +309,37 @@ sysio_prepare_tx_cmsgs(sock *s, struct msghdr *msg */ #endif + +static int +sk_set_min_ttl4(sock *s, int ttl) +{ +#ifdef IP_MINTTL + if (setsockopt(s->fd, IPPROTO_IP, IP_MINTTL, &ttl, sizeof(ttl)) == -1) + { + log(L_ERR "sk_set_min_ttl4: setsockopt: %m"); + return -1; + } + + return 0; +#else + log(L_ERR "IP_MINTTL options is not supported by your kernel"); + return -1; +#endif +} + +static int +sk_set_min_ttl6(sock *s, int ttl) +{ +#ifdef IPV6_MINHOPCNT + if (setsockopt(s->fd, IPPROTO_IPV6, IPV6_MINHOPCNT, &ttl, sizeof(ttl)) == -1) + { + log(L_ERR "sk_set_min_ttl6: setsockopt: %m"); + return -1; + } + + return 0; +#else + log(L_ERR "IPV6_MINHOPCNT options is not supported by your kernel"); + return -1; +#endif +} Index: sysdep/unix/io.c =================================================================== --- sysdep/unix/io.c (revision 4980) +++ sysdep/unix/io.c (working copy) @@ -805,7 +805,7 @@ bad: } /** - * sk_set_ttl - set TTL for given socket. + * sk_set_ttl - set transmit TTL for given socket. * @s: socket * @ttl: TTL value * @@ -828,7 +828,29 @@ sk_set_ttl(sock *s, int ttl) return (err ? -1 : 0); } +/** + * sk_set_min_ttl - set minimal accepted TTL for given socket. + * @s: socket + * @ttl: TTL value + * + * Can be used in TTL security implementation + * + * Result: 0 for success, -1 for an error. + */ +int +sk_set_min_ttl(sock *s, int ttl) +{ + int err; +#ifdef IPV6 + err = sk_set_min_ttl6(s, ttl); +#else + err = sk_set_min_ttl4(s, ttl); +#endif + + return err; +} + /** * sk_set_md5_auth - add / remove MD5 security association for given socket. * @s: socket Index: sysdep/bsd/sysio.h =================================================================== --- sysdep/bsd/sysio.h (revision 4980) +++ sysdep/bsd/sysio.h (working copy) @@ -237,3 +237,23 @@ sk_set_md5_auth_int(sock *s, sockaddr *sa, char *p return rv; } + +static int +sk_set_min_ttl4(sock *s, int ttl) +{ + if (setsockopt(s->fd, IPPROTO_IP, IP_MINTTL, &ttl, sizeof(ttl)) == -1) + { + log(L_ERR "sk_set_min_ttl4: setsockopt: %m"); + return -1; + } + + return 0; +} + +static int +sk_set_min_ttl6(sock *s, int ttl) +{ + log(L_ERR "sk_set_min_ttl6: not supported"); + return -1; +} +
On Sun, Aug 14, 2011 at 02:47:27PM +0400, Alexander V. Chernikov wrote:
Review/comments are welcome
Patch adds:
* new sk_set_min_ttl() function to set minimum received TTL * new BGP (cisco-like) config option: ttl_secutity hops <value>
Tested on FreeBSD, however linux part should work too.
Great! We could merge it soon. Several comments inside.
Index: proto/bgp/bgp.c =================================================================== --- proto/bgp/bgp.c (revision 4980) +++ proto/bgp/bgp.c (working copy) @@ -574,7 +574,11 @@ bgp_connect(struct bgp_proto *p) /* Enter Connect s->saddr = p->source_addr; s->daddr = p->cf->remote_ip; s->dport = BGP_PORT; - s->ttl = p->cf->multihop ? : 1; + /* TTL security support */ + if (p->cf->min_ttl > 0) + s->ttl = 255; + else + s->ttl = p->cf->multihop ? : 1; s->rbsize = BGP_RX_BUFFER_SIZE; s->tbsize = BGP_TX_BUFFER_SIZE; s->tos = IP_PREC_INTERNET_CONTROL; @@ -589,6 +593,17 @@ bgp_connect(struct bgp_proto *p) /* Enter Connect bgp_sock_err(s, 0); return; } + /* Set minimal receive TTL if needed */ + if (p->cf->min_ttl > 0) + {
+ log(L_ERR "Setting minimum received TTL to %d", p->cf->min_ttl);
Probably should not be here (there is no error), some fugitive debugging message?
+ if (sk_set_min_ttl(s, p->cf->min_ttl) != 0) + { + log(L_ERR "TTL security configuration failed, closing session"); + bgp_sock_err(s, 0); + return; + } + }
Shouldn't be better to set min TTL before sk_open? Also a common idiom is to test negative error return values using 'fn() < 0'. Both these comments are probably unnecessary nitpicky. ;-)
Index: proto/bgp/config.Y =================================================================== --- proto/bgp/config.Y (revision 4980) +++ proto/bgp/config.Y (working copy) @@ -25,7 +25,7 @@ CF_KEYWORDS(BGP, LOCAL, NEIGHBOR, AS, HOLD, TIME, ADVERTISE, IPV4, CAPABILITIES, LIMIT, PASSIVE, PREFER, OLDER, MISSING, LLADDR, DROP, IGNORE, ROUTE, REFRESH, INTERPRET, COMMUNITIES, BGP_ORIGINATOR_ID, BGP_CLUSTER_LIST, IGP, TABLE, - GATEWAY, DIRECT, RECURSIVE, MED) + GATEWAY, DIRECT, RECURSIVE, MED, TTL_SECURITY, HOPS)
CF_GRAMMAR
@@ -49,6 +49,7 @@ bgp_proto_start: proto_start BGP { BGP_CFG->advertise_ipv4 = 1; BGP_CFG->interpret_communities = 1; BGP_CFG->default_local_pref = 100; + BGP_CFG->min_ttl = 0;
BGP_CFG fields are already zeroed (somewhere in proto_config_new()), no need to set that.
| bgp_proto IGP TABLE rtable ';' { BGP_CFG->igp_table = $4; } + | bgp_proto TTL_SECURITY HOPS expr ';' { BGP_CFG->min_ttl = $4; }
It is very unBIRDy to have a config option with underscore. Perhaps TTL SECURITY HOPS, or just MIN TTL? (MIN TTL is probably much better name as we do not specify the number of hops, but the complement (255 - hops), if i understand it correctly.)
+static int +sk_set_min_ttl4(sock *s, int ttl) +{ +#ifdef IP_MINTTL + if (setsockopt(s->fd, IPPROTO_IP, IP_MINTTL, &ttl, sizeof(ttl)) == -1) + { + log(L_ERR "sk_set_min_ttl4: setsockopt: %m"); + return -1; + } + + return 0; +#else + log(L_ERR "IP_MINTTL options is not supported by your kernel"); + return -1; +#endif +}
It is much better to define IP_MINTTL if it is missing: #ifndef IP_MINTTL #define IP_MINTTL XXX #endif Compile time kernel feature autodetection is IMHO a pretty poor policy, when software is usually distributed in binary form (as is common in Linux distributions), compiled on some unknown machine, where user chooses kernel versions independently.
+ +static int +sk_set_min_ttl6(sock *s, int ttl) +{ +#ifdef IPV6_MINHOPCNT + if (setsockopt(s->fd, IPPROTO_IPV6, IPV6_MINHOPCNT, &ttl, sizeof(ttl)) == -1) + { + log(L_ERR "sk_set_min_ttl6: setsockopt: %m"); + return -1; + } + + return 0; +#else + log(L_ERR "IPV6_MINHOPCNT options is not supported by your kernel"); + return -1; +#endif
ditto The new config option should be also documented in doc/bird.sgml . -- Elen sila lumenn' omentielvo Ondrej 'SanTiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Ondrej Zajicek wrote:
On Sun, Aug 14, 2011 at 02:47:27PM +0400, Alexander V. Chernikov wrote:
Review/comments are welcome
Patch adds:
* new sk_set_min_ttl() function to set minimum received TTL * new BGP (cisco-like) config option: ttl_secutity hops <value>
Tested on FreeBSD, however linux part should work too.
Great! We could merge it soon. Several comments inside.
Index: proto/bgp/bgp.c =================================================================== --- proto/bgp/bgp.c (revision 4980) +++ proto/bgp/bgp.c (working copy) @@ -574,7 +574,11 @@ bgp_connect(struct bgp_proto *p) /* Enter Connect s->saddr = p->source_addr; s->daddr = p->cf->remote_ip; s->dport = BGP_PORT; - s->ttl = p->cf->multihop ? : 1; + /* TTL security support */ + if (p->cf->min_ttl > 0) + s->ttl = 255; + else + s->ttl = p->cf->multihop ? : 1; s->rbsize = BGP_RX_BUFFER_SIZE; s->tbsize = BGP_TX_BUFFER_SIZE; s->tos = IP_PREC_INTERNET_CONTROL; @@ -589,6 +593,17 @@ bgp_connect(struct bgp_proto *p) /* Enter Connect bgp_sock_err(s, 0); return; } + /* Set minimal receive TTL if needed */ + if (p->cf->min_ttl > 0) + {
+ log(L_ERR "Setting minimum received TTL to %d", p->cf->min_ttl);
Probably should not be here (there is no error), some fugitive debugging message? Ups. It should be removed or converted to DBG() :)
+ if (sk_set_min_ttl(s, p->cf->min_ttl) != 0) + { + log(L_ERR "TTL security configuration failed, closing session"); + bgp_sock_err(s, 0); + return; + } + }
Shouldn't be better to set min TTL before sk_open? Not sure. Not many callers need this, so adding another min_ttl field seems unnecessary IMHO. Anyway, you will need to specify minimum ttl directly in case of new connection from listening socket.
Also a common idiom is to test negative error return values using 'fn() < 0'.
Okay :) There are so many idioms/coding styles in different software projects that sometimes it's hard to switch.
Both these comments are probably unnecessary nitpicky. ;-)
No problem :)
Index: proto/bgp/config.Y =================================================================== --- proto/bgp/config.Y (revision 4980) +++ proto/bgp/config.Y (working copy) @@ -25,7 +25,7 @@ CF_KEYWORDS(BGP, LOCAL, NEIGHBOR, AS, HOLD, TIME, ADVERTISE, IPV4, CAPABILITIES, LIMIT, PASSIVE, PREFER, OLDER, MISSING, LLADDR, DROP, IGNORE, ROUTE, REFRESH, INTERPRET, COMMUNITIES, BGP_ORIGINATOR_ID, BGP_CLUSTER_LIST, IGP, TABLE, - GATEWAY, DIRECT, RECURSIVE, MED) + GATEWAY, DIRECT, RECURSIVE, MED, TTL_SECURITY, HOPS)
CF_GRAMMAR
@@ -49,6 +49,7 @@ bgp_proto_start: proto_start BGP { BGP_CFG->advertise_ipv4 = 1; BGP_CFG->interpret_communities = 1; BGP_CFG->default_local_pref = 100; + BGP_CFG->min_ttl = 0;
BGP_CFG fields are already zeroed (somewhere in proto_config_new()), no need to set that.
Ups :)
| bgp_proto IGP TABLE rtable ';' { BGP_CFG->igp_table = $4; } + | bgp_proto TTL_SECURITY HOPS expr ';' { BGP_CFG->min_ttl = $4; }
It is very unBIRDy to have a config option with underscore.
Em, well, BGP still heavily uses underscores: BGP_ORIGINATOR_ID, BGP_CLUSTER_LIST, BGP_PATH, BGP_LOCAL_PREF, BGP_MED, BGP_ORIGIN, BGP_NEXT_HOP, BGP_ATOMIC_AGGR, BGP_AGGREGATOR,BGP_COMMUNITY,
Perhaps TTL SECURITY HOPS, or just MIN TTL? 'TTL SECURITY HOPS' sounds good and is at least used by cisco.
(MIN TTL is probably much better name as we do not specify the number of hops, but the complement (255 - hops), if i understand it correctly.)
Well, actually we're specifying minimal TTL packet needs to have in its packet header to be accepted. Packets with lower TTL are silently dropped. If we name this option 'min ttl' or 'min hops' it will: * be confised with 'multihop' option * not be associated with enabling TTL security We can also make 'TTL SECURITY' boolean option and use 'multihop' option value (like 255 - hops + 1)
+static int +sk_set_min_ttl4(sock *s, int ttl) +{ +#ifdef IP_MINTTL + if (setsockopt(s->fd, IPPROTO_IP, IP_MINTTL, &ttl, sizeof(ttl)) == -1) + { + log(L_ERR "sk_set_min_ttl4: setsockopt: %m"); + return -1; + } + + return 0; +#else + log(L_ERR "IP_MINTTL options is not supported by your kernel"); + return -1; +#endif +}
It is much better to define IP_MINTTL if it is missing:
#ifndef IP_MINTTL #define IP_MINTTL XXX #endif
Compile time kernel feature autodetection is IMHO a pretty poor policy, when software is usually distributed in binary form (as is common in Linux distributions), compiled on some unknown machine, where user chooses kernel versions independently. Understood.
+ +static int +sk_set_min_ttl6(sock *s, int ttl) +{ +#ifdef IPV6_MINHOPCNT + if (setsockopt(s->fd, IPPROTO_IPV6, IPV6_MINHOPCNT, &ttl, sizeof(ttl)) == -1) + { + log(L_ERR "sk_set_min_ttl6: setsockopt: %m"); + return -1; + } + + return 0; +#else + log(L_ERR "IPV6_MINHOPCNT options is not supported by your kernel"); + return -1; +#endif
ditto
The new config option should be also documented in doc/bird.sgml .
Should I supply updated patch?
-----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.14 (FreeBSD) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk5H6SsACgkQwcJ4iSZ1q2nwBwCgkNLG9iFeu/pSckH3vtn2n67B ITQAnAuIG34ZleiFDUskWoSXLIPPPS/h =oe0H -----END PGP SIGNATURE-----
On Sun, Aug 14, 2011 at 07:26:36PM +0400, Alexander V. Chernikov wrote:
+ if (sk_set_min_ttl(s, p->cf->min_ttl) != 0) + { + log(L_ERR "TTL security configuration failed, closing session"); + bgp_sock_err(s, 0); + return; + } + }
Shouldn't be better to set min TTL before sk_open? Not sure. Not many callers need this, so adding another min_ttl field seems unnecessary IMHO. Anyway, you will need to specify minimum ttl directly in case of new connection from listening socket.
You are right.
Perhaps TTL SECURITY HOPS, or just MIN TTL? 'TTL SECURITY HOPS' sounds good and is at least used by cisco.
(MIN TTL is probably much better name as we do not specify the number of hops, but the complement (255 - hops), if i understand it correctly.)
Well, actually we're specifying minimal TTL packet needs to have in its packet header to be accepted. Packets with lower TTL are silently dropped.
If we name this option 'min ttl' or 'min hops' it will:
* be confised with 'multihop' option * not be associated with enabling TTL security
We can also make 'TTL SECURITY' boolean option and use 'multihop' option value (like 255 - hops + 1)
This is probably the best alternative. Note that 'multihop' value is an original TTL (i.e. a path length in number of networks/edges), so it would be: multihop ? 256 - multihop : 255 .
The new config option should be also documented in doc/bird.sgml .
Should I supply updated patch?
That would be great (esp. if it would contain updated documentation ;-) ). -- Elen sila lumenn' omentielvo Ondrej 'SanTiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Ondrej Zajicek wrote:
On Sun, Aug 14, 2011 at 07:26:36PM +0400, Alexander V. Chernikov wrote:
+ if (sk_set_min_ttl(s, p->cf->min_ttl) != 0) + { + log(L_ERR "TTL security configuration failed, closing session"); + bgp_sock_err(s, 0); + return; + } + } Shouldn't be better to set min TTL before sk_open? Not sure. Not many callers need this, so adding another min_ttl field seems unnecessary IMHO. Anyway, you will need to specify minimum ttl directly in case of new connection from listening socket.
You are right.
Perhaps TTL SECURITY HOPS, or just MIN TTL? 'TTL SECURITY HOPS' sounds good and is at least used by cisco. (MIN TTL is probably much better name as we do not specify the number of hops, but the complement (255 - hops), if i understand it correctly.)
Well, actually we're specifying minimal TTL packet needs to have in its packet header to be accepted. Packets with lower TTL are silently dropped.
If we name this option 'min ttl' or 'min hops' it will:
* be confised with 'multihop' option * not be associated with enabling TTL security
We can also make 'TTL SECURITY' boolean option and use 'multihop' option value (like 255 - hops + 1)
This is probably the best alternative. Note that 'multihop' value is an original TTL (i.e. a path length in number of networks/edges), so it would be: multihop ? 256 - multihop : 255 . Unfortunately, we can't distinguish between enabled/disabled multihop if ttl security is on :(
I've also set listening socket TTL to 255 since it is required by RFC (from ttlsec-enabled neithbour SA received from bird can definitely be associated with protected session and dropped if its TTL is not within range). We can increase socket TTL conditionally though it will require a bit more code (initial configuration/reconfiguration) but I see no problem in increased (64->255) TTL. There are some minor changes in bgp_open not directly related to RFC.
The new config option should be also documented in doc/bird.sgml . Should I supply updated patch?
That would be great (esp. if it would contain updated documentation ;-) ).
Done :) -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.14 (FreeBSD) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEUEARECAAYFAk5KA+cACgkQwcJ4iSZ1q2lURACWL2A2ZByuQvQlcD22sSUAh/wH /ACdHi34hXiv1Ki8hU6Y1iZUfAC02ac= =jLE4 -----END PGP SIGNATURE----- Index: proto/bgp/bgp.c =================================================================== --- proto/bgp/bgp.c (revision 4980) +++ proto/bgp/bgp.c (working copy) @@ -93,6 +93,7 @@ { struct config *cfg = p->cf->c.global; bgp_counter++; + int errcause; if (!bgp_listen_sk) bgp_listen_sk = bgp_setup_listen_sk(cfg->listen_bgp_addr, cfg->listen_bgp_port, cfg->listen_bgp_flags); @@ -100,10 +101,8 @@ if (!bgp_listen_sk) { bgp_counter--; - p->p.disabled = 1; - bgp_store_error(p, NULL, BE_MISC, BEM_NO_SOCKET); - proto_notify_state(&p->p, PS_DOWN); - return -1; + errcause = BEM_NO_SOCKET; + goto err; } if (!bgp_linpool) @@ -115,14 +114,17 @@ if (rv < 0) { bgp_close(p, 0); - p->p.disabled = 1; - bgp_store_error(p, NULL, BE_MISC, BEM_INVALID_MD5); - proto_notify_state(&p->p, PS_DOWN); - return -1; + goto err; } } return 0; + +err: + p->p.disabled = 1; + bgp_store_error(p, NULL, BE_MISC, errcause); + proto_notify_state(&p->p, PS_DOWN); + return -1; } static void @@ -574,7 +576,11 @@ s->saddr = p->source_addr; s->daddr = p->cf->remote_ip; s->dport = BGP_PORT; - s->ttl = p->cf->multihop ? : 1; + /* TTL security support */ + if (p->cf->ttl_sec_hops) + s->ttl = 255; + else + s->ttl = p->cf->multihop ? : 1; s->rbsize = BGP_RX_BUFFER_SIZE; s->tbsize = BGP_TX_BUFFER_SIZE; s->tos = IP_PREC_INTERNET_CONTROL; @@ -589,6 +595,17 @@ bgp_sock_err(s, 0); return; } + /* Set minimal receive TTL if needed */ + if (p->cf->ttl_sec_hops) + { + DBG("Setting minimum received TTL to %d", 256 - p->cf->ttl_sec_hops); + if (sk_set_min_ttl(s, 256 - p->cf->ttl_sec_hops) < 0) + { + log(L_ERR "TTL security configuration failed, closing session"); + bgp_sock_err(s, 0); + return; + } + } DBG("BGP: Waiting for connect success\n"); bgp_start_timer(conn->connect_retry_timer, p->cf->connect_retry_time); } @@ -629,7 +646,17 @@ bgp_setup_conn(p, &p->incoming_conn); bgp_setup_sk(&p->incoming_conn, sk); - sk_set_ttl(sk, p->cf->multihop ? : 1); + /* TTL security support */ + if (p->cf->ttl_sec_hops) + { + if ((sk_set_ttl(sk, 255) < 0) || (sk_set_min_ttl(sk, 256 - p->cf->ttl_sec_hops) < 0)) + { + log(L_ERR "TTL security configuration failed, closing session"); + goto err; + } + } + else + sk_set_ttl(sk, p->cf->multihop ? : 1); bgp_send_open(&p->incoming_conn); return 0; } @@ -656,6 +683,7 @@ sock *s = sk_new(&root_pool); DBG("BGP: Creating listening socket\n"); s->type = SK_TCP_PASSIVE; + s->ttl = 255; s->saddr = addr; s->sport = port ? port : BGP_PORT; s->flags = flags ? 0 : SKF_V6ONLY; @@ -670,8 +698,7 @@ rfree(s); return NULL; } - else - return s; + return s; } static void @@ -972,6 +999,12 @@ if (c->multihop && (c->gw_mode == GW_DIRECT)) cf_error("Multihop BGP cannot use direct gateway mode"); + if (c->ttl_sec_hops && ((c->ttl_sec_hops < 1) || (c->ttl_sec_hops > 254))) + cf_error("Wrong hops value for TTL security. Accepted: 1..254"); + + if (c->multihop && ((c->multihop < 1) || (c->multihop > 255))) + cf_error("Wrong multihop value. Accepted: 1..255"); + /* Different default based on rs_client */ if (!c->missing_lladdr) c->missing_lladdr = c->rs_client ? MLL_IGNORE : MLL_SELF; Index: proto/bgp/config.Y =================================================================== --- proto/bgp/config.Y (revision 4980) +++ proto/bgp/config.Y (working copy) @@ -25,7 +25,7 @@ ADVERTISE, IPV4, CAPABILITIES, LIMIT, PASSIVE, PREFER, OLDER, MISSING, LLADDR, DROP, IGNORE, ROUTE, REFRESH, INTERPRET, COMMUNITIES, BGP_ORIGINATOR_ID, BGP_CLUSTER_LIST, IGP, TABLE, - GATEWAY, DIRECT, RECURSIVE, MED) + GATEWAY, DIRECT, RECURSIVE, MED, TTL, SECURITY, HOPS) CF_GRAMMAR @@ -98,6 +98,7 @@ | bgp_proto PASSIVE bool ';' { BGP_CFG->passive = $3; } | bgp_proto INTERPRET COMMUNITIES bool ';' { BGP_CFG->interpret_communities = $4; } | bgp_proto IGP TABLE rtable ';' { BGP_CFG->igp_table = $4; } + | bgp_proto TTL SECURITY HOPS expr ';' { BGP_CFG->ttl_sec_hops = $5; } ; CF_ADDTO(dynamic_attr, BGP_ORIGIN Index: proto/bgp/bgp.h =================================================================== --- proto/bgp/bgp.h (revision 4980) +++ proto/bgp/bgp.h (working copy) @@ -20,6 +20,7 @@ u32 local_as, remote_as; ip_addr remote_ip; int multihop; /* Number of hops if multihop */ + int ttl_sec_hops; /* Number of hops if TTL security is enabled */ ip_addr source_addr; /* Source address to use */ int next_hop_self; /* Always set next hop to local IP address */ int missing_lladdr; /* What we will do when we don' know link-local addr, see MLL_* */ Index: doc/bird.sgml =================================================================== --- doc/bird.sgml (revision 4980) +++ doc/bird.sgml (working copy) @@ -1136,6 +1136,14 @@ as an IGP routing table. Default: the same as the table BGP is connected to. + <tag>ttl security hops <m/number/</tag> Use RFC 5082 (TTL security mechanism). + Maximum hop count has to be supplied. Note target PC (e.g. bird instance) + also counts as hop, so minimum hop count is 1. If ttl security is enabled + value supplied in <cf/multihop/ is ignored. Default: disabled. + Kernel support required: Linux: 2.6.34+ (IPv4), 2.6.35+ (IPv6), BSD: + since long ago. Note that full (ICMP protection, for example) RFC5082 + support is provided by Linux only. + <tag>password <m/string/</tag> Use this password for MD5 authentication of BGP sessions. Default: no authentication. Password has to be set by external utility (e.g. setkey(8)) on BSD systems. Index: lib/socket.h =================================================================== --- lib/socket.h (revision 4980) +++ lib/socket.h (working copy) @@ -54,7 +54,8 @@ int sk_send_to(sock *, unsigned len, ip_addr to, unsigned port); /* sk_send to given destination */ void sk_reallocate(sock *); /* Free and allocate tbuf & rbuf */ void sk_dump_all(void); -int sk_set_ttl(sock *s, int ttl); /* Set TTL for given socket */ +int sk_set_ttl(sock *s, int ttl); /* Set transmit TTL for given socket */ +int sk_set_min_ttl(sock *s, int ttl); /* Set minimal accepted TTL for given socket */ /* Add or remove security associations for given passive socket */ int sk_set_md5_auth(sock *s, ip_addr a, char *passwd); Index: sysdep/linux/sysio.h =================================================================== --- sysdep/linux/sysio.h (revision 4980) +++ sysdep/linux/sysio.h (working copy) @@ -309,3 +309,35 @@ */ #endif + +#ifndef IP_MINTTL +#define IP_MINTTL 21 +#endif + +#ifndef IPV6_MINHOPCOUNT +#define IPV6_MINHOPCOUNT 73 +#endif + +static int +sk_set_min_ttl4(sock *s, int ttl) +{ + if (setsockopt(s->fd, IPPROTO_IP, IP_MINTTL, &ttl, sizeof(ttl)) == -1) + { + log(L_ERR "sk_set_min_ttl4: setsockopt: %m"); + return -1; + } + + return 0; +} + +static int +sk_set_min_ttl6(sock *s, int ttl) +{ + if (setsockopt(s->fd, IPPROTO_IPV6, IPV6_MINHOPCNT, &ttl, sizeof(ttl)) == -1) + { + log(L_ERR "sk_set_min_ttl6: setsockopt: %m"); + return -1; + } + + return 0; +} Index: sysdep/unix/io.c =================================================================== --- sysdep/unix/io.c (revision 4980) +++ sysdep/unix/io.c (working copy) @@ -805,7 +805,7 @@ } /** - * sk_set_ttl - set TTL for given socket. + * sk_set_ttl - set transmit TTL for given socket. * @s: socket * @ttl: TTL value * @@ -828,7 +828,29 @@ return (err ? -1 : 0); } +/** + * sk_set_min_ttl - set minimal accepted TTL for given socket. + * @s: socket + * @ttl: TTL value + * + * Can be used in TTL security implementation + * + * Result: 0 for success, -1 for an error. + */ +int +sk_set_min_ttl(sock *s, int ttl) +{ + int err; +#ifdef IPV6 + err = sk_set_min_ttl6(s, ttl); +#else + err = sk_set_min_ttl4(s, ttl); +#endif + + return err; +} + /** * sk_set_md5_auth - add / remove MD5 security association for given socket. * @s: socket Index: sysdep/bsd/sysio.h =================================================================== --- sysdep/bsd/sysio.h (revision 4980) +++ sysdep/bsd/sysio.h (working copy) @@ -237,3 +237,23 @@ return rv; } + +static int +sk_set_min_ttl4(sock *s, int ttl) +{ + if (setsockopt(s->fd, IPPROTO_IP, IP_MINTTL, &ttl, sizeof(ttl)) == -1) + { + log(L_ERR "sk_set_min_ttl4: setsockopt: %m"); + return -1; + } + + return 0; +} + +static int +sk_set_min_ttl6(sock *s, int ttl) +{ + log(L_ERR "sk_set_min_ttl6: not supported"); + return -1; +} +
On Tue, Aug 16, 2011 at 09:45:11AM +0400, Alexander V. Chernikov wrote:
This is probably the best alternative. Note that 'multihop' value is an original TTL (i.e. a path length in number of networks/edges), so it would be: multihop ? 256 - multihop : 255 . Unfortunately, we can't distinguish between enabled/disabled multihop if ttl security is on :(
I don't see why - the argument of multihop option is the same value (number of hops - networks, not routers) as the the argument of 'ttl security hops' option - see attached code, just the relevant part of diff. Or i miss something? Enabled/disabled multihop has many other implications than just TTL, so even if TTL security is used, user have to set that option if BGP session is multihop.
I've also set listening socket TTL to 255 since it is required by RFC (from ttlsec-enabled neithbour SA received from bird can definitely be associated with protected session and dropped if its TTL is not within range). We can increase socket TTL conditionally though it will require a bit more code (initial configuration/reconfiguration) but I see no problem in increased (64->255) TTL.
OK
There are some minor changes in bgp_open not directly related to RFC.
And one bug in these changes (errcause not filled in the second case) ;-)
The new config option should be also documented in doc/bird.sgml . Should I supply updated patch?
That would be great (esp. if it would contain updated documentation ;-) ).
Done :)
Thanks, i will do some minor clean ups and merge it with the boolean modification. -- Elen sila lumenn' omentielvo Ondrej 'SanTiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
On 16.08.2011 19:15, Ondrej Zajicek wrote:
On Tue, Aug 16, 2011 at 09:45:11AM +0400, Alexander V. Chernikov wrote:
This is probably the best alternative. Note that 'multihop' value is an original TTL (i.e. a path length in number of networks/edges), so it would be: multihop ? 256 - multihop : 255 . Unfortunately, we can't distinguish between enabled/disabled multihop if ttl security is on :(
I don't see why - the argument of multihop option is the same value (number of hops - networks, not routers) as the the argument of 'ttl security hops' option - see attached code, just the relevant part of diff. Or i miss something? Nono. You are absolutely right :) ENOSLEEP issue :)
Enabled/disabled multihop has many other implications than just TTL, so even if TTL security is used, user have to set that option if BGP session is multihop.
I've also set listening socket TTL to 255 since it is required by RFC (from ttlsec-enabled neithbour SA received from bird can definitely be associated with protected session and dropped if its TTL is not within range). We can increase socket TTL conditionally though it will require a bit more code (initial configuration/reconfiguration) but I see no problem in increased (64->255) TTL.
OK
There are some minor changes in bgp_open not directly related to RFC.
And one bug in these changes (errcause not filled in the second case) ;-)
The new config option should be also documented in doc/bird.sgml . Should I supply updated patch?
That would be great (esp. if it would contain updated documentation ;-) ).
Done :)
Thanks, i will do some minor clean ups and merge it with the boolean modification.
Thanks!
On Sun, 14 Aug 2011, Alexander V. Chernikov wrote:
Henrique de Moraes Holschuh wrote:
Is anyone currently working on adding GTSM support to bird?
It should be possible to support it for both Linux and FreeBSD where available as a kernel-level supported socket option, and I am considering trying my hand at it as a way to get to know the bird codebase a bit better before we decide to deploy it at work...
Review/comments are welcome
Thank you. I will try to be useful with some testing and help a bit writing up the documentation changes, then :-) One thing I think is worth documenting is that at least Linux implements full RFC5082 GTSM behaviour, i.e. it _also_ TTL-filters related ICMP traffic. It would be nice to know whether Cisco and FreeBSD do full GTSM or just pre-RFC5082 GTSH (i.e. no ICMP protection). Anyway, I think I found a problem in the patch:
* new BGP (cisco-like) config option: ttl_secutity hops <value>
At least in Linux, and I believe BSD does it the same way (since Linux is supposed to have copied the BSD behaviour), what the patch currently does is "ttl_security min_ttl <value>", where "min_ttl = 255 - hops". I assume that you wanted the ttl_security option to behave like it does in Cisco, i.e. you'd use "ttl_security hops 1" to set outgoing TTL to 255 and accept inbound TTL >= 254. I did check the Linux kernel implementation, and it expects the minimum acceptable TTL in the setsockopt() call, not the hop count. I've also checked the IPv6 code, and it works exactly in the same way. IMHO, it would be best to change the min_ttl parameter to max_hops, so that you can convert it to whatever the underlying OS wants in sysdep/. Alternatively, the conversion could be done in the parser. We should probably range-check things in the parser as well... -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Henrique de Moraes Holschuh wrote:
On Sun, 14 Aug 2011, Alexander V. Chernikov wrote:
Henrique de Moraes Holschuh wrote:
Is anyone currently working on adding GTSM support to bird?
It should be possible to support it for both Linux and FreeBSD where available as a kernel-level supported socket option, and I am considering trying my hand at it as a way to get to know the bird codebase a bit better before we decide to deploy it at work...
Review/comments are welcome
Thank you. I will try to be useful with some testing and help a bit writing up the documentation changes, then :-)
One thing I think is worth documenting is that at least Linux implements full RFC5082 GTSM behaviour, i.e. it _also_ TTL-filters related ICMP traffic. It would be nice to know whether Cisco and FreeBSD do full GTSM or just pre-RFC5082 GTSH (i.e. no ICMP protection). Not sure about cisco (docs I'm aware of specify RFC 3682 as GTSM source). FreeBSD does not (at the moment) provide ICMP protection. 9.1/8.3 will.
Anyway, I think I found a problem in the patch:
* new BGP (cisco-like) config option: ttl_secutity hops <value>
At least in Linux, and I believe BSD does it the same way (since Linux is supposed to have copied the BSD behaviour), what the patch currently does is "ttl_security min_ttl <value>", where "min_ttl = 255 - hops".
I assume that you wanted the ttl_security option to behave like it does in Cisco, i.e. you'd use "ttl_security hops 1" to set outgoing TTL to 255 and accept inbound TTL >= 254. Ups. Yes
I did check the Linux kernel implementation, and it expects the minimum acceptable TTL in the setsockopt() call, not the hop count. I've also checked the IPv6 code, and it works exactly in the same way.
IMHO, it would be best to change the min_ttl parameter to max_hops, so that you can convert it to whatever the underlying OS wants in sysdep/. Alternatively, the conversion could be done in the parser.
We should probably range-check things in the parser as well... Yes, thanks
-----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.14 (FreeBSD) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk5IM1wACgkQwcJ4iSZ1q2mbqACglte9nz5tcwDj9hbyL1WeKll9 hm8An0Cl9XRCSnPj0IJ9GMFziqV4Awk6 =OyuY -----END PGP SIGNATURE-----
participants (3)
-
Alexander V. Chernikov -
Henrique de Moraes Holschuh -
Ondrej Zajicek