[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