[Bug 3845] New: parse_absolute_time: Initialize tm_isdst=-1 so mktime uses tzdata to determine whether DST is in effect

[bugzilla-daemon at mindrot.org]

https://bugzilla.mindrot.org/show_bug.cgi?id=3845

            Bug ID: 3845
           Summary: parse_absolute_time: Initialize tm_isdst=-1 so mktime
                    uses tzdata to determine whether DST is in effect
           Product: Portable OpenSSH
           Version: 10.0p2
          Hardware: amd64
                OS: Linux
            Status: NEW
          Severity: security
          Priority: P5
         Component: ssh-keygen
          Assignee: unassigned-bugs at mindrot.org
          Reporter: aim at orbit.online

Created attachment 3887
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3887&action=edit
Initializes tm.tm_isdst to 0 or 1, depending on whether we are dealing
with a UTC or local timestamp

Git signature verification via SSH certificates (1h validity) stopped
working after we switched to DST in Europe.

The following script consistently fails with "No principal matched.":
```
verify_time=$(date +%Y%m%d%H%M%S)
ssh-keygen -Y sign -n file -f id_ecdsa-cert.pub test
ssh-keygen -v -Y find-principals -f allowedSigners -s test.sig
-Overify-time=$verify_time
```

Verbose output:
```
debug1: allowedSigners:10: principal "*@<DOMAIN>" not authorized:
Certificate invalid: expired
allowedSigners:10: no valid principals found
debug1: allowedSigners:10: cert_filter_principals: invalid certificate
```

Adjusting `$verify_time` one hour back makes everything work as
expected, signatures before DST went into effect also work currently.

As far as I can tell this happens because `tm_isdst` remains
uninitialized in `parse_absolute_time` when calling `mktime`
(`strptime` does not account for DST since it only deals with offsets).

According to the docs[1]:
> The value specified in the tm_isdst field informs mktime() whether or not daylight saving time (DST) is in effect for the time supplied in the tm structure: a positive value means DST is in effect; zero means that DST is not in effect; and a negative value means that mktime() should (use timezone information and system databases to) attempt to determine whether DST is in effect at the specified time.

Meaning `tm.tm_isdst` should be set to `-1` when dealing with local
timestamps and `0` when dealing with UTC timestamps (patch attached).

Note that I have not written any tests for this. In fact, Damien even
complains about how tricky it is to write timezone/DST tests in
`test_convtime.c`. Experience shows if he hadn't done that the existing
code would have magically worked somehow, he provoked the bug demons
with that comment  ^^


[1] https://manpages.debian.org/bookworm/manpages-dev/mktime.3.en.html

-- 
You are receiving this mail because:
You are watching the assignee of the bug.