[Swan-dev] now() and other topics

Paul Wouters paul at nohats.ca
Thu May 1 00:33:48 EEST 2014


On Wed, 30 Apr 2014, D. Hugh Redelmeier wrote:

The whole point of that change was to fix a dead lock:

https://github.com/xelerance/Openswan/issues/74



> Date: Wed, 30 Apr 2014 17:12:42
> From: D. Hugh Redelmeier <hugh at mimosa.com>
> To: swan-dev at lists.libreswan.org
> Subject: [Swan-dev] now() and other topics
> 
> | From: Paul Wouters <paul at vault.libreswan.fi>
>
> | commit b7f0b35aadf214c0b50140ee7a9397c0c7b4f192
> | Author: Paul Wouters <pwouters at redhat.com>
> | Date:   Wed Apr 30 12:39:03 2014 -0400
> |
> |     pluto: Ensure time going backwards does not screw up queued events
>
> now() is a tricky function.  Maybe not tricky enough.
>
> It is designed so that the time it reports never goes backwards.  If
> someone sets the clock back 3 days, it notices, and adds 3 days to
> each of its subsequent results.
>
> This can fail if there is a lot of back-and-forth.  Eventually, the
> offset becomes so large that time overflows.  One expects that this
> wouldn't happen, but there is no proof.
>
> As a result of this offset action, now() is not suitable for logging.
> Any offset will make the logged time wrong in the reader's context.
> It is probably not suitable for certificate validity checking.
>
> I don't know how many of the things you changed to use now() are
> wrong.  I started to look but got bogged down (see below).  This needs
> to be unfixed.
>
> This is hard to debug since it is rare that there is any offset.  We
> could force it to be visible by initializing the local static delta to
> something other than 0.
>
> ================
>
> programs/pluto/defs.c is certainly an odd duck.  It has two functions:
> all_zero and check_expiry.  They seem to have no relation.
>
> programs/pluto/defs.h declares things that are not defined in defs.h.
> Not good.
>
> I saw this in there:
>
> typedef u_int32_t msgid_t; /* Network order for ikev1, host order for ikev2 */
>
> Why the heck would you use the same type for different things.
>
> ================
>
> I love this test in list_public_keys:
>
>                        if (!check_pub_keys ||
> 			    (check_pub_keys &&
>                 	     strncmp(check_expiry_msg, "ok", 2))) {
>
> and the fact that it calls check_expiry a second time when it has the
> result in a variable.
>
> ================
>
> constants like 172800 and 86400 are inscrutable.  So I've define
> time_t_minute, time_t_hour, and time_t_day to make these clearer.
> _______________________________________________
> Swan-dev mailing list
> Swan-dev at lists.libreswan.org
> https://lists.libreswan.org/mailman/listinfo/swan-dev
>


More information about the Swan-dev mailing list