[nft PATCH] tests/py: Set a fixed timezone in nft-test.py

[nft PATCH] tests/py: Set a fixed timezone in nft-test.py

[Phil Sutter]

Payload generated for 'meta time' matches depends on host's timezone and
DST setting. To produce constant output, set a fixed timezone in
nft-test.py. Choose UTC-2 since most payloads are correct then, adjust
the remaining two tests.

Fixes: 0518ea3f70d8c ("tests: add meta time test cases")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/py/any/meta.t         | 2 +-
 tests/py/any/meta.t.payload | 2 +-
 tests/py/nft-test.py        | 1 +
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/py/any/meta.t b/tests/py/any/meta.t
index 86e5d258605dc..327f973f1bd5a 100644
--- a/tests/py/any/meta.t
+++ b/tests/py/any/meta.t
@@ -205,7 +205,7 @@ meta random eq 1;ok;meta random 1
 meta random gt 1000000;ok;meta random > 1000000
 
 meta time "1970-05-23 21:07:14" drop;ok
-meta time 12341234 drop;ok;meta time "1970-05-23 21:07:14" drop
+meta time 12341234 drop;ok;meta time "1970-05-23 22:07:14" drop
 meta time "2019-06-21 17:00:00" drop;ok
 meta time "2019-07-01 00:00:00" drop;ok
 meta time "2019-07-01 00:01:00" drop;ok
diff --git a/tests/py/any/meta.t.payload b/tests/py/any/meta.t.payload
index 402caae5cad8c..486d7aa566ea3 100644
--- a/tests/py/any/meta.t.payload
+++ b/tests/py/any/meta.t.payload
@@ -1050,7 +1050,7 @@ ip test-ip4 input
 # meta time "1970-05-23 21:07:14" drop
 ip meta-test input
   [ meta load time => reg 1 ]
-  [ cmp eq reg 1 0x74a8f400 0x002bd849 ]
+  [ cmp eq reg 1 0x43f05400 0x002bd503 ]
   [ immediate reg 0 drop ]
 
 # meta time 12341234 drop
diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py
index ce42b5ddb1cca..6edca3c6a5a2f 100755
--- a/tests/py/nft-test.py
+++ b/tests/py/nft-test.py
@@ -24,6 +24,7 @@ import tempfile
 
 TESTS_PATH = os.path.dirname(os.path.abspath(__file__))
 sys.path.insert(0, os.path.join(TESTS_PATH, '../../py/'))
+os.environ['TZ'] = 'UTC-2'
 
 from nftables import Nftables
 
-- 
2.24.0

Re: [nft PATCH] tests/py: Set a fixed timezone in nft-test.py

[Ander Juaristi]

I had a hard time getting time zones right. Thank you.

Acked-by: Ander Juaristi <a@juaristi.eus>

El 2019-11-16 22:32, Phil Sutter escribió:
> Payload generated for 'meta time' matches depends on host's timezone 
> and
> DST setting. To produce constant output, set a fixed timezone in
> nft-test.py. Choose UTC-2 since most payloads are correct then, adjust
> the remaining two tests.
> 
> Fixes: 0518ea3f70d8c ("tests: add meta time test cases")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  tests/py/any/meta.t         | 2 +-
>  tests/py/any/meta.t.payload | 2 +-
>  tests/py/nft-test.py        | 1 +
>  3 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/py/any/meta.t b/tests/py/any/meta.t
> index 86e5d258605dc..327f973f1bd5a 100644
> --- a/tests/py/any/meta.t
> +++ b/tests/py/any/meta.t
> @@ -205,7 +205,7 @@ meta random eq 1;ok;meta random 1
>  meta random gt 1000000;ok;meta random > 1000000
> 
>  meta time "1970-05-23 21:07:14" drop;ok
> -meta time 12341234 drop;ok;meta time "1970-05-23 21:07:14" drop
> +meta time 12341234 drop;ok;meta time "1970-05-23 22:07:14" drop
>  meta time "2019-06-21 17:00:00" drop;ok
>  meta time "2019-07-01 00:00:00" drop;ok
>  meta time "2019-07-01 00:01:00" drop;ok
> diff --git a/tests/py/any/meta.t.payload b/tests/py/any/meta.t.payload
> index 402caae5cad8c..486d7aa566ea3 100644
> --- a/tests/py/any/meta.t.payload
> +++ b/tests/py/any/meta.t.payload
> @@ -1050,7 +1050,7 @@ ip test-ip4 input
>  # meta time "1970-05-23 21:07:14" drop
>  ip meta-test input
>    [ meta load time => reg 1 ]
> -  [ cmp eq reg 1 0x74a8f400 0x002bd849 ]
> +  [ cmp eq reg 1 0x43f05400 0x002bd503 ]
>    [ immediate reg 0 drop ]
> 
>  # meta time 12341234 drop
> diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py
> index ce42b5ddb1cca..6edca3c6a5a2f 100755
> --- a/tests/py/nft-test.py
> +++ b/tests/py/nft-test.py
> @@ -24,6 +24,7 @@ import tempfile
> 
>  TESTS_PATH = os.path.dirname(os.path.abspath(__file__))
>  sys.path.insert(0, os.path.join(TESTS_PATH, '../../py/'))
> +os.environ['TZ'] = 'UTC-2'
> 
>  from nftables import Nftables

Re: [nft PATCH] tests/py: Set a fixed timezone in nft-test.py

[Pablo Neira Ayuso]

Hi Phil,

On Sat, Nov 16, 2019 at 10:32:18PM +0100, Phil Sutter wrote:
> Payload generated for 'meta time' matches depends on host's timezone and
> DST setting. To produce constant output, set a fixed timezone in
> nft-test.py. Choose UTC-2 since most payloads are correct then, adjust
> the remaining two tests.

This means that the ruleset listing for the user changes when daylight
saving occurs, right? Just like it happened to our tests.

Thanks.

Re: [nft PATCH] tests/py: Set a fixed timezone in nft-test.py

[Phil Sutter]

Hi Pablo,

On Mon, Nov 18, 2019 at 07:34:59PM +0100, Pablo Neira Ayuso wrote:
> On Sat, Nov 16, 2019 at 10:32:18PM +0100, Phil Sutter wrote:
> > Payload generated for 'meta time' matches depends on host's timezone and
> > DST setting. To produce constant output, set a fixed timezone in
> > nft-test.py. Choose UTC-2 since most payloads are correct then, adjust
> > the remaining two tests.
> 
> This means that the ruleset listing for the user changes when daylight
> saving occurs, right? Just like it happened to our tests.

I think UTC-2 is a fixed offset to a time that doesn't change. During
DST in Europe, our timezone changes from CET to CEST, or from UTC+1 to
UTC+2. So by specifying the same TZ no matter if DST applies or not, the
offset added to the current time should always remain the same.

Cheers, Phil

Re: [nft PATCH] tests/py: Set a fixed timezone in nft-test.py

[Ander Juaristi]

El 2019-11-18 19:34, Pablo Neira Ayuso escribió:
> Hi Phil,
> 
> On Sat, Nov 16, 2019 at 10:32:18PM +0100, Phil Sutter wrote:
>> Payload generated for 'meta time' matches depends on host's timezone 
>> and
>> DST setting. To produce constant output, set a fixed timezone in
>> nft-test.py. Choose UTC-2 since most payloads are correct then, adjust
>> the remaining two tests.
> 
> This means that the ruleset listing for the user changes when daylight
> saving occurs, right? Just like it happened to our tests.

It shouldn't, as the date is converted to a timestamp that doesn't take 
DST into account (using timegm(3), which is Linux-specific).

The problem is that payloads are hard-coded in the tests.

Correct me if I'm missing something.

> 
> Thanks.

Re: [nft PATCH] tests/py: Set a fixed timezone in nft-test.py

[Pablo Neira Ayuso]

On Tue, Nov 19, 2019 at 12:06:23PM +0100, Ander Juaristi wrote:
> El 2019-11-18 19:34, Pablo Neira Ayuso escribió:
> > Hi Phil,
> > 
> > On Sat, Nov 16, 2019 at 10:32:18PM +0100, Phil Sutter wrote:
> > > Payload generated for 'meta time' matches depends on host's timezone
> > > and
> > > DST setting. To produce constant output, set a fixed timezone in
> > > nft-test.py. Choose UTC-2 since most payloads are correct then, adjust
> > > the remaining two tests.
> > 
> > This means that the ruleset listing for the user changes when daylight
> > saving occurs, right? Just like it happened to our tests.
> 
> It shouldn't, as the date is converted to a timestamp that doesn't take DST
> into account (using timegm(3), which is Linux-specific).
> 
> The problem is that payloads are hard-coded in the tests.
> 
> Correct me if I'm missing something.

I see, so it's just the _snprintf() function in the library. I
remember we found another problem with these on big endian, it would
be probably to move them to libnftables at some point.

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Re: [nft PATCH] tests/py: Set a fixed timezone in nft-test.py

[Phil Sutter]

Hi,

On Tue, Nov 19, 2019 at 11:12:36PM +0100, Pablo Neira Ayuso wrote:
> On Tue, Nov 19, 2019 at 12:06:23PM +0100, Ander Juaristi wrote:
> > El 2019-11-18 19:34, Pablo Neira Ayuso escribió:
> > > Hi Phil,
> > > 
> > > On Sat, Nov 16, 2019 at 10:32:18PM +0100, Phil Sutter wrote:
> > > > Payload generated for 'meta time' matches depends on host's timezone
> > > > and
> > > > DST setting. To produce constant output, set a fixed timezone in
> > > > nft-test.py. Choose UTC-2 since most payloads are correct then, adjust
> > > > the remaining two tests.
> > > 
> > > This means that the ruleset listing for the user changes when daylight
> > > saving occurs, right? Just like it happened to our tests.
> > 
> > It shouldn't, as the date is converted to a timestamp that doesn't take DST
> > into account (using timegm(3), which is Linux-specific).
> > 
> > The problem is that payloads are hard-coded in the tests.
> > 
> > Correct me if I'm missing something.
> 
> I see, so it's just the _snprintf() function in the library. I
> remember we found another problem with these on big endian, it would
> be probably to move them to libnftables at some point.
> 
> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Ah, now that I reread your question, I finally got it. And you're right:
If DST occurs, time values will change. This is clear from looking at
hour_type_print(): Whatever the kernel returned gets cur_tm->tm_gmtoff
added to it. Here, this is either 3600 or 7200 depending on whether DST
is active or not.

The other alternative would be to make kernel DST-aware, I don't think
that's the case.

Cheers, Phil

Re: [nft PATCH] tests/py: Set a fixed timezone in nft-test.py

[Pablo Neira Ayuso]

On Tue, Nov 19, 2019 at 11:20:23PM +0100, Phil Sutter wrote:
> Hi,
> 
> On Tue, Nov 19, 2019 at 11:12:36PM +0100, Pablo Neira Ayuso wrote:
> > On Tue, Nov 19, 2019 at 12:06:23PM +0100, Ander Juaristi wrote:
> > > El 2019-11-18 19:34, Pablo Neira Ayuso escribió:
> > > > Hi Phil,
> > > > 
> > > > On Sat, Nov 16, 2019 at 10:32:18PM +0100, Phil Sutter wrote:
> > > > > Payload generated for 'meta time' matches depends on host's timezone
> > > > > and
> > > > > DST setting. To produce constant output, set a fixed timezone in
> > > > > nft-test.py. Choose UTC-2 since most payloads are correct then, adjust
> > > > > the remaining two tests.
> > > > 
> > > > This means that the ruleset listing for the user changes when daylight
> > > > saving occurs, right? Just like it happened to our tests.
> > > 
> > > It shouldn't, as the date is converted to a timestamp that doesn't take DST
> > > into account (using timegm(3), which is Linux-specific).
> > > 
> > > The problem is that payloads are hard-coded in the tests.
> > > 
> > > Correct me if I'm missing something.
> > 
> > I see, so it's just the _snprintf() function in the library. I
> > remember we found another problem with these on big endian, it would
> > be probably to move them to libnftables at some point.
> > 
> > Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> Ah, now that I reread your question, I finally got it. And you're right:
> If DST occurs, time values will change. This is clear from looking at
> hour_type_print(): Whatever the kernel returned gets cur_tm->tm_gmtoff
> added to it. Here, this is either 3600 or 7200 depending on whether DST
> is active or not.
> 
> The other alternative would be to make kernel DST-aware, I don't think
> that's the case.

Hm, so the ruleset listing changes after DST.

If the user specifies the time according to the userspace timezone,
then ruleset listing should not change?

I'm still trying to understand if this is a real problem.

Re: [nft PATCH] tests/py: Set a fixed timezone in nft-test.py

[Phil Sutter]

Hi Pablo,

On Tue, Nov 19, 2019 at 11:33:18PM +0100, Pablo Neira Ayuso wrote:
> On Tue, Nov 19, 2019 at 11:20:23PM +0100, Phil Sutter wrote:
> > Hi,
> > 
> > On Tue, Nov 19, 2019 at 11:12:36PM +0100, Pablo Neira Ayuso wrote:
> > > On Tue, Nov 19, 2019 at 12:06:23PM +0100, Ander Juaristi wrote:
> > > > El 2019-11-18 19:34, Pablo Neira Ayuso escribió:
> > > > > Hi Phil,
> > > > > 
> > > > > On Sat, Nov 16, 2019 at 10:32:18PM +0100, Phil Sutter wrote:
> > > > > > Payload generated for 'meta time' matches depends on host's timezone
> > > > > > and
> > > > > > DST setting. To produce constant output, set a fixed timezone in
> > > > > > nft-test.py. Choose UTC-2 since most payloads are correct then, adjust
> > > > > > the remaining two tests.
> > > > > 
> > > > > This means that the ruleset listing for the user changes when daylight
> > > > > saving occurs, right? Just like it happened to our tests.
> > > > 
> > > > It shouldn't, as the date is converted to a timestamp that doesn't take DST
> > > > into account (using timegm(3), which is Linux-specific).
> > > > 
> > > > The problem is that payloads are hard-coded in the tests.
> > > > 
> > > > Correct me if I'm missing something.
> > > 
> > > I see, so it's just the _snprintf() function in the library. I
> > > remember we found another problem with these on big endian, it would
> > > be probably to move them to libnftables at some point.
> > > 
> > > Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > 
> > Ah, now that I reread your question, I finally got it. And you're right:
> > If DST occurs, time values will change. This is clear from looking at
> > hour_type_print(): Whatever the kernel returned gets cur_tm->tm_gmtoff
> > added to it. Here, this is either 3600 or 7200 depending on whether DST
> > is active or not.
> > 
> > The other alternative would be to make kernel DST-aware, I don't think
> > that's the case.
> 
> Hm, so the ruleset listing changes after DST.

Yes, but only because payload (and hence kernel data) remains the same.

> If the user specifies the time according to the userspace timezone,
> then ruleset listing should not change?
> 
> I'm still trying to understand if this is a real problem.

IMHO, it's a controversy of whether we want to make the time match
respect DST changes or not:

It should, because when I restrict internet access to my kids after 8pm,
I don't want to touch my firewall's ruleset twice a year.

It needs not, because upon the next reboot things are correct again,
anyway.

It should not because otherwise once a year there are two spooky hours in
which things run twice that should never ever run more than once per
night. Apart from that, description of --kerneltz in
iptables-extensions(8) speaks for itself.

So long story short, it is always a problem no matter how it is solved.
:(

Cheers, Phil