[nft,1/4] src: make hash seed attribute optional
diff mbox

Message ID 9d2ade5563d42e65466eef399a5b1b9e14954463.1477170966.git.nevola@gmail.com
State Changes Requested
Delegated to: Pablo Neira
Headers show

Commit Message

Laura Garcia Liebana Oct. 22, 2016, 9:34 p.m. UTC
The hash expression requires a seed attribute to call the jhash
operation, eg.

 # nft add rule x y meta mark set jhash ip saddr . ip daddr mod 2 \
	seed 0xdeadbeef

With this patch the seed attribute is optional and it's generated by a
random function from userspace, eg.

 # nft add rule x y meta mark set jhash ip saddr . ip daddr mod 2

To generate a secure random number it has been included the libbsd
library dependency by default, that implements the arc4random()
function generator. But it's possible to get rid of this dependency
applying the option --without-arc4random during the configure of the
package.

Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Laura Garcia Liebana <nevola@gmail.com>
---
 configure.ac       | 14 +++++++++++++-
 include/hash.h     | 10 ++++++++++
 src/parser_bison.y |  5 +++++
 tests/py/ip/hash.t |  2 ++
 4 files changed, 30 insertions(+), 1 deletion(-)

Comments

Pablo Neira Ayuso Oct. 27, 2016, 5:07 p.m. UTC | #1
On Sat, Oct 22, 2016 at 11:34:15PM +0200, Laura Garcia Liebana wrote:
> The hash expression requires a seed attribute to call the jhash
> operation, eg.
> 
>  # nft add rule x y meta mark set jhash ip saddr . ip daddr mod 2 \
> 	seed 0xdeadbeef
> 
> With this patch the seed attribute is optional and it's generated by a
> random function from userspace, eg.
> 
>  # nft add rule x y meta mark set jhash ip saddr . ip daddr mod 2
> 
> To generate a secure random number it has been included the libbsd
> library dependency by default, that implements the arc4random()
> function generator. But it's possible to get rid of this dependency
> applying the option --without-arc4random during the configure of the
> package.
> 
> Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Signed-off-by: Laura Garcia Liebana <nevola@gmail.com>
> ---
>  configure.ac       | 14 +++++++++++++-
>  include/hash.h     | 10 ++++++++++
>  src/parser_bison.y |  5 +++++
>  tests/py/ip/hash.t |  2 ++
>  4 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 7e0b75c..8c93981 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -108,6 +108,17 @@ AC_DEFINE([HAVE_LIBXTABLES], [1], [0])
>  AC_SUBST(with_libxtables)
>  AM_CONDITIONAL([BUILD_XTABLES], [test "x$with_libxtables" == xyes])
>  
> +AC_ARG_WITH([arc4random], [AS_HELP_STRING([--without-arc4random],
> +            [disable arc4random (libbsd dev support)])],
> +            [], [with_arc4random=yes])
> +AS_IF([test "x$with_arc4random" != xno], [
> +AC_CHECK_LIB([bsd], [arc4random], ,
> +	     AC_MSG_ERROR([No suitable version of libbsd dev found]))
> +AC_DEFINE([HAVE_LIBBSD], [1], [])
> +])
> +AC_SUBST(with_arc4random)
> +AM_CONDITIONAL([BUILD_ARC4RANDOM], [test "x$with_arc4random" != xno])

We have getrandom() already around for a while:

https://lwn.net/Articles/605828/

Main problem is that your libc version may not yet support this. But
in case HAVE_GETRANDOM is not set, otherwise fallback on the poorman
version by now.

>  # Checks for header files.
>  AC_HEADER_STDC
>  AC_HEADER_ASSERT
> @@ -158,4 +169,5 @@ nft configuration:
>    enable debugging:		${with_debug}
>    use mini-gmp:			${with_mini_gmp}
>    enable pdf documentation:	${enable_pdf_doc}
> -  libxtables support:		${with_libxtables}"
> +  libxtables support:		${with_libxtables}
> +  arc4random support:		${with_arc4random}"

It would be good to indicate here what random approach we follow, just
for the record.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pablo Neira Ayuso Oct. 27, 2016, 5:13 p.m. UTC | #2
On Thu, Oct 27, 2016 at 07:07:50PM +0200, Pablo Neira Ayuso wrote:
> On Sat, Oct 22, 2016 at 11:34:15PM +0200, Laura Garcia Liebana wrote:
> > The hash expression requires a seed attribute to call the jhash
> > operation, eg.
> > 
> >  # nft add rule x y meta mark set jhash ip saddr . ip daddr mod 2 \
> > 	seed 0xdeadbeef
> > 
> > With this patch the seed attribute is optional and it's generated by a
> > random function from userspace, eg.
> > 
> >  # nft add rule x y meta mark set jhash ip saddr . ip daddr mod 2
> > 
> > To generate a secure random number it has been included the libbsd
> > library dependency by default, that implements the arc4random()
> > function generator. But it's possible to get rid of this dependency
> > applying the option --without-arc4random during the configure of the
> > package.
> > 
> > Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > Signed-off-by: Laura Garcia Liebana <nevola@gmail.com>
> > ---
> >  configure.ac       | 14 +++++++++++++-
> >  include/hash.h     | 10 ++++++++++
> >  src/parser_bison.y |  5 +++++
> >  tests/py/ip/hash.t |  2 ++
> >  4 files changed, 30 insertions(+), 1 deletion(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 7e0b75c..8c93981 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -108,6 +108,17 @@ AC_DEFINE([HAVE_LIBXTABLES], [1], [0])
> >  AC_SUBST(with_libxtables)
> >  AM_CONDITIONAL([BUILD_XTABLES], [test "x$with_libxtables" == xyes])
> >  
> > +AC_ARG_WITH([arc4random], [AS_HELP_STRING([--without-arc4random],
> > +            [disable arc4random (libbsd dev support)])],
> > +            [], [with_arc4random=yes])
> > +AS_IF([test "x$with_arc4random" != xno], [
> > +AC_CHECK_LIB([bsd], [arc4random], ,
> > +	     AC_MSG_ERROR([No suitable version of libbsd dev found]))
> > +AC_DEFINE([HAVE_LIBBSD], [1], [])
> > +])
> > +AC_SUBST(with_arc4random)
> > +AM_CONDITIONAL([BUILD_ARC4RANDOM], [test "x$with_arc4random" != xno])
> 
> We have getrandom() already around for a while:
> 
> https://lwn.net/Articles/605828/
> 
> Main problem is that your libc version may not yet support this. But
> in case HAVE_GETRANDOM is not set, otherwise fallback on the poorman
> version by now.

I mean, we can add this to configure.ac:

AC_CHECK_FUNCS(getrandom)

So config.h will define HAVE_GETRANDOM if available. This constant
will tell us what implementation we can use for this.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/configure.ac b/configure.ac
index 7e0b75c..8c93981 100644
--- a/configure.ac
+++ b/configure.ac
@@ -108,6 +108,17 @@  AC_DEFINE([HAVE_LIBXTABLES], [1], [0])
 AC_SUBST(with_libxtables)
 AM_CONDITIONAL([BUILD_XTABLES], [test "x$with_libxtables" == xyes])
 
+AC_ARG_WITH([arc4random], [AS_HELP_STRING([--without-arc4random],
+            [disable arc4random (libbsd dev support)])],
+            [], [with_arc4random=yes])
+AS_IF([test "x$with_arc4random" != xno], [
+AC_CHECK_LIB([bsd], [arc4random], ,
+	     AC_MSG_ERROR([No suitable version of libbsd dev found]))
+AC_DEFINE([HAVE_LIBBSD], [1], [])
+])
+AC_SUBST(with_arc4random)
+AM_CONDITIONAL([BUILD_ARC4RANDOM], [test "x$with_arc4random" != xno])
+
 # Checks for header files.
 AC_HEADER_STDC
 AC_HEADER_ASSERT
@@ -158,4 +169,5 @@  nft configuration:
   enable debugging:		${with_debug}
   use mini-gmp:			${with_mini_gmp}
   enable pdf documentation:	${enable_pdf_doc}
-  libxtables support:		${with_libxtables}"
+  libxtables support:		${with_libxtables}
+  arc4random support:		${with_arc4random}"
diff --git a/include/hash.h b/include/hash.h
index bc8c86a..5350cb2 100644
--- a/include/hash.h
+++ b/include/hash.h
@@ -1,6 +1,16 @@ 
 #ifndef NFTABLES_HASH_H
 #define NFTABLES_HASH_H
 
+#ifdef HAVE_LIBBSD
+#include <bsd/stdlib.h>
+#define getrandom()	(arc4random() % ((uint32_t)RAND_MAX + 1))
+
+#else
+#include <time.h>
+#include <stdlib.h>
+#define getrandom()	({ srand(time(NULL)); (uint32_t)rand(); })
+#endif
+
 extern struct expr *hash_expr_alloc(const struct location *loc,
 				    uint32_t modulus, uint32_t seed);
 
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 36dbc8d..0fa469d 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -2485,6 +2485,11 @@  hash_expr		:	JHASH	expr	MOD	NUM	SEED	NUM
 				$$ = hash_expr_alloc(&@$, $4, $6);
 				$$->hash.expr = $2;
 			}
+			|	JHASH	expr	MOD	NUM
+			{
+				$$ = hash_expr_alloc(&@$, $4, getrandom());
+				$$->hash.expr = $2;
+			}
 			;
 
 ct_expr			: 	CT	ct_key
diff --git a/tests/py/ip/hash.t b/tests/py/ip/hash.t
index 6dfa965..85f9b18 100644
--- a/tests/py/ip/hash.t
+++ b/tests/py/ip/hash.t
@@ -2,4 +2,6 @@ 
 *ip;test-ip4;pre
 
 ct mark set jhash ip saddr . ip daddr mod 2 seed 0xdeadbeef;ok
+ct mark set jhash ip saddr . ip daddr mod 2;ok
 dnat to jhash ip saddr mod 2 seed 0xdeadbeef map { 0 : 192.168.20.100, 1 : 192.168.30.100 };ok
+dnat to jhash ip saddr mod 2 map { 0 : 192.168.20.100, 1 : 192.168.30.100 };ok