Allow learning iroutes with network made up of all 0s (only if netbits < 8)
PATCH v2 1/1 Allow learning iroutes with network made up of all 0s (only if netbits < 8)
It is plausible for a user to be willing to add a route for a network made up of all 0s via a VPN client (i.e. 0.0.0.0/1), therefore such iroute should be supported.
As of now the option parsing code will accept such iroute, but the learning routine will (silently) reject it after a sanity check.
Such check prevents routes with network made up of all 0s to be learnt at all..
Change the sanity check so that it will reject iroutes to network made up of 0s only when netbits is greater than 7.
The reason for choosing 7 is because anything within 0.0.0.0/8 is not really routable among networks.
While at it, make the sanity check louder so that it can print the reason why a route is being rejected.
Trac: #726
---
v2:
- rebased on top of latest master
src/openvpn/mroute.c | 37 +++++++++++++++++++++++++++++++------ src/openvpn/mroute.h | 3 ++- src/openvpn/multi.c | 9 ++++----- 3 files changed, 37 insertions(+), 12 deletions(-)
src/openvpn/mroute.c +31 -6
@@ -65,25 +65,50 @@ is_mac_mcast_maddr(const struct mroute_addr *addr)* Don't learn certain addresses. */ bool -mroute_learnable_address (const struct mroute_addr *addr) +mroute_learnable_address (const struct mroute_addr *addr,int msglevel, +struct gc_arena *gc) {int i; -bool not_all_zeros = false; -bool not_all_ones = false; +bool all_zeros = true; +bool all_ones = true;for (i =0 ; i < addr->len; ++i) {int b = addr->raw_addr[i];if (b !=0x00 ) { - not_all_zeros = true; + all_zeros = false; }if (b !=0xFF ) { - not_all_ones = true; + all_ones = false; } } -return not_all_zeros && not_all_ones && !is_mac_mcast_maddr (addr); + +/* only networkss shorter than 8 bits are allowed to be all 0s. */ +if (all_zeros + && !((addr->type &MR_WITH_NETBITS ) && (addr->netbits <8 ))) + { +msg (msglevel,"Can't learn %s: network is all 0s, but netbits >= 8" , +mroute_addr_print (addr, gc)); +return false; + } + +if (all_ones) + { +msg (msglevel,"Can't learn %s: network is all 1s" , +mroute_addr_print (addr, gc)); +return false; + } + +if (is_mac_mcast_maddr (addr)) + { +msg (msglevel,"Can't learn %s: network is a multicast address" , +mroute_addr_print (addr, gc)); +return false; + } + +return true; } static inline void
src/openvpn/mroute.h +2 -1
@@ -141,7 +141,8 @@ bool mroute_extract_openvpn_sockaddr(struct mroute_addr *addr,const struct openvpn_sockaddr *osaddr, bool use_port); -bool mroute_learnable_address (const struct mroute_addr *addr); +bool mroute_learnable_address (const struct mroute_addr *addr,int msglevel, +struct gc_arena *gc);uint32_t mroute_addr_hash_function (const void *key,uint32_t iv);
src/openvpn/multi.c +4 -5
@@ -1074,6 +1074,7 @@ multi_learn_addr(struct multi_context *m,struct hash_bucket *bucket =hash_bucket (m->vhash, hv);struct multi_route *oldroute =NULL ;struct multi_instance *owner =NULL ; +struct gc_arena gc =gc_new ();/* if route currently exists, get the instance which owns it */ he =hash_lookup_fast (m->vhash, bucket, addr, hv);
@@ -1087,11 +1088,10 @@ multi_learn_addr(struct multi_context *m, }/* do we need to add address to hash table? */ -if ((!owner || owner != mi) - &&mroute_learnable_address (addr) +if ((!owner || owner != mi) &&mroute_learnable_address (addr,D_MULTI_LOW , + &gc) && !mroute_addr_equal (addr, &m->local)) { -struct gc_arena gc =gc_new ();struct multi_route *newroute;bool learn_succeeded = false;
@@ -1148,9 +1148,8 @@ multi_learn_addr(struct multi_context *m, {free (newroute); } - -gc_free (&gc); } +gc_free (&gc);return owner; }
Signature · 1 line
2.15.1
PATCH v3 1/1 Allow learning iroutes with network made up of all 0s (only if netbits < 8)
It is plausible for a user to be willing to add a route for a network made up of all 0s via a VPN client (i.e. 0.0.0.0/1), therefore such iroute should be supported.
As of now the option parsing code will accept such iroute, but the learning routine will (silently) reject it after a sanity check.
Such check prevents routes with network made up of all 0s to be learnt at all..
Change the sanity check so that it will reject iroutes to network made up of 0s only when netbits is greater than 7.
The reason for choosing 7 is because anything within 0.0.0.0/8 is not really routable among networks.
While at it, make the sanity check louder so that it can print the reason why a route is being rejected.
Trac: #726
---
v3:
- rebased on top of master
- removed msglevel argument as per David's suggestion
v2:
- rebased on to pof master
src/openvpn/mroute.c | 36 ++++++++++++++++++++++++++++++------ src/openvpn/mroute.h | 3 ++- src/openvpn/multi.c | 8 +++----- 3 files changed, 35 insertions(+), 12 deletions(-)
src/openvpn/mroute.c +30 -6
@@ -65,25 +65,49 @@ is_mac_mcast_maddr(const struct mroute_addr *addr)* Don't learn certain addresses. */ bool -mroute_learnable_address (const struct mroute_addr *addr) +mroute_learnable_address (const struct mroute_addr *addr,struct gc_arena *gc) {int i; -bool not_all_zeros = false; -bool not_all_ones = false; +bool all_zeros = true; +bool all_ones = true;for (i =0 ; i < addr->len; ++i) {int b = addr->raw_addr[i];if (b !=0x00 ) { - not_all_zeros = true; + all_zeros = false; }if (b !=0xFF ) { - not_all_ones = true; + all_ones = false; } } -return not_all_zeros && not_all_ones && !is_mac_mcast_maddr (addr); + +/* only networkss shorter than 8 bits are allowed to be all 0s. */ +if (all_zeros + && !((addr->type &MR_WITH_NETBITS ) && (addr->netbits <8 ))) + { +msg (D_MULTI_LOW ,"Can't learn %s: network is all 0s, but netbits >= 8" , +mroute_addr_print (addr, gc)); +return false; + } + +if (all_ones) + { +msg (D_MULTI_LOW ,"Can't learn %s: network is all 1s" , +mroute_addr_print (addr, gc)); +return false; + } + +if (is_mac_mcast_maddr (addr)) + { +msg (D_MULTI_LOW ,"Can't learn %s: network is a multicast address" , +mroute_addr_print (addr, gc)); +return false; + } + +return true; } static inline void
src/openvpn/mroute.h +2 -1
@@ -141,7 +141,8 @@ bool mroute_extract_openvpn_sockaddr(struct mroute_addr *addr,const struct openvpn_sockaddr *osaddr, bool use_port); -bool mroute_learnable_address (const struct mroute_addr *addr); +bool mroute_learnable_address (const struct mroute_addr *addr, +struct gc_arena *gc);uint32_t mroute_addr_hash_function (const void *key,uint32_t iv);
src/openvpn/multi.c +3 -5
@@ -1074,6 +1074,7 @@ multi_learn_addr(struct multi_context *m,struct hash_bucket *bucket =hash_bucket (m->vhash, hv);struct multi_route *oldroute =NULL ;struct multi_instance *owner =NULL ; +struct gc_arena gc =gc_new ();/* if route currently exists, get the instance which owns it */ he =hash_lookup_fast (m->vhash, bucket, addr, hv);
@@ -1087,11 +1088,9 @@ multi_learn_addr(struct multi_context *m, }/* do we need to add address to hash table? */ -if ((!owner || owner != mi) - &&mroute_learnable_address (addr) +if ((!owner || owner != mi) &&mroute_learnable_address (addr, &gc) && !mroute_addr_equal (addr, &m->local)) { -struct gc_arena gc =gc_new ();struct multi_route *newroute;bool learn_succeeded = false;
@@ -1148,9 +1147,8 @@ multi_learn_addr(struct multi_context *m, {free (newroute); } - -gc_free (&gc); } +gc_free (&gc);return owner; }
Signature · 1 line
2.15.1
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
ACK. Code makes sense and seems not to break an existing server configuration I have.
Your patch has been applied to the following branches
commit a19c56db9bd42b7b8c4a8f353f7db92781397cec (master) commit 80614c7c547c84e0884cf2bde1dbbac76b4767c3 (release/2.4)
Author: Antonio Quartulli Date: Wed Dec 6 23:43:56 2017 +0800
Allow learning iroutes with network made up of all 0s (only if netbits < 8)
Trac: #726
Signed-off-by: Antonio Quartulli <[email protected]...>
Acked-by: David Sommerseth <[email protected]...>
Message-Id: <[email protected]...>
URL: https://www.mail-archive.com/[email protected]/msg16044.html
Signed-off-by: David Sommerseth <[email protected]...>- --
kind regards,
David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
iQIcBAEBCgAGBQJaKGJuAAoJEIbPlEyWcf3yEWoQAJ9T+3H4EzmfvH3JKuLoQZpc galt9+wDH1raFEGBUAECf+wnEgNA+QGbM7Sv0E1xXaqAfuRup7A1GJyIKqN2yaLN SF2RMiihmnu9S33qUkiBMzH9KiU974G6zwZVRtd+NpeEak7a3PaHM0nyIyOvuX5Y cm/npJTAvgolxW7WvPQrmuJ9qwKFitGD4d4KMU6AmA7Bs+Jfk2MFwfjpZTx9UxEa YR9EdZU7kVxF/H5mai8xdsTkLUpccdywaDet2TOjEYU+4gTZdBEP5afRrO8VU27b U6ynTmny94LYGBG1mnSj4f/l3gMSfPWLKm/nw1s0FyoBRKme0Wp637NJfqRBQj25 KQJXye8dR80Dlf2UjDpEARNPeb81CNFpYx2q6tnO1iMjLXH+SB10Glmp4qh81nuA Gp4DyNRSj8DGx/ocWqVKldCVpH4swlFVG2HNA3b6E2pFt5Y+0qmIsizywxbT0NzX aL2FDNU+ibhgoFcW08pEFUtFmmMlAfq4Nb3cMJziebQYEVyFFupM4ajZ41Esxd2+ 1XfwHrLblgNHQ5j3+p6uO1Hgy4lb4IyrNEqmEFzX+MbcLimva4dWvaTccFHHt0jX 4snpPc7zN4s9Ir/pc5hQUd9mkY/Vd1NrOnCwBJh0KB9WTivBCMFk1Qo+AOJEfl7e B0SE4yDZTUWM8ANmavQw
=77MP
-----END PGP SIGNATURE-----