Skip to content

autotls: expires timestamps do not follow RFC 3339 #2975

Description

@rlve

Description

parse3339DateTime accepts exactly one timestamp layout, yyyy-MM-ddTHH:mm:ss.<digits>Z. Inside that layout it is wrong twice, and outside it the failure is silent or fatal.

1. The offset is discarded and the components are read as local time. times.parse defaults to zone = local() and the Z is split off before parsing ever sees it. East of UTC a live bearer reads as expired, so AutotlsBroker drops it and re-runs the full handshake on every registration. West of UTC an expired bearer reads as live, the client keeps sending it, and the broker rejects every registration until real time catches up with the skew.

2. The fractional second is read as milliseconds whatever its length. parseInt("621940726") becomes initDuration(milliseconds = 621940726), which is about 7 days. The broker sends a 9-digit fraction, see below. Worst case is 11.5 days of an expired bearer reading as live.

3. A timestamp with no fractional second is silently dropped. RFC 3339 makes the fraction optional, so 2026-08-21T12:00:00Z is legal. split('.') leaves the whole string in parts[0], parse chokes on the trailing Z, and the catch turns that into Opt.none. The bearer is then treated as never expiring, which runs into #2972 once the broker starts rejecting it. The same happens to any non-Z offset, so 2026-08-21T12:00:00+02:00 is dropped too.

4. A timestamp with no offset raises IndexDefect. parts[1] is out of bounds. IndexDefect is not a CatchableError, so the catch misses it and it escapes requestAuthorization, which is declared raises: [PeerIDAuthError, CancelledError] inside a module under {.push raises: [].}. No caller can handle it. This input is not valid RFC 3339, so it takes a non-conforming server to trigger, but nothing in the type or the raises list says so.

The ACME side has symptom 1 in a different function, and quotes 'Z' as a literal, so any expires carrying a fraction or a numeric offset raises TimeParseError. handleError catches that as a ValueError and reports downloadCertificate: Failed to decode JSON, which points at the wrong thing.

Symptom 1 is a no-op on a UTC host, which is why CI has never caught it.

Evidence from the live broker

Probed registration.libp2p.direct:

  • It sends no expires field today, only sig and bearer. So the peeridauth path is latent. The ACME path is not.
  • Its bearer token embeds "created-time":"2026-08-21T11:36:41.621940726Z", which is Go's time.RFC3339Nano. That format strips trailing zeros, so a fix must accept a fraction of any length from 1 to 9 digits rather than a fixed set.

Spec Recommendation

expires is RFC 3339, which always carries an offset and makes the fractional second optional and unbounded in length.

peer-id-auth L175-L183

RFC 3339 section 5.6

Implementation

The parser, where the same split('.') drops the offset, misreads the fraction, requires a fraction, and indexes out of bounds without one:

proc parse3339DateTime(timeStr: string): DateTime {.raises: [ValueError].} =
let parts = timeStr.split('.')
let base = parse(parts[0], "yyyy-MM-dd'T'HH:mm:ss")
let millis = parseInt(parts[1].strip(chars = {'Z'}))
base + initDuration(milliseconds = millis)

The catch that is meant to make a bad expires non-fatal, and does so for everything except a Defect:

let bearerExpires =
try:
Opt.some(parse3339DateTime(extractField(authenticationInfo, "expires")))
except ValueError, PeerIDAuthError, TimeParseError:
Opt.none(DateTime)

The two consumers of the parsed value, both comparing against now():

if bearer.expires.isSome() and bearer.expires.get <= now():
raise newException(PeerIDAuthError, "Bearer expired")

if self.bearer.isSome():
let cached = self.bearer.get()
if cached.expires.isSome() and cached.expires.get() <= now():
self.bearer = Opt.none(BearerToken)

The ACME side, which quotes 'Z' as a literal and passes no zone:

certificateExpiry: parse(orderResponse.expires, "yyyy-MM-dd'T'HH:mm:ss'Z'"),

Notes for the fix

An attempt at this fix hit four separate traps, all of them in times.

  • times.parse defaults to zone = local(). Pass utc() explicitly. Two call sites in this repo already do, transports/tls/certificate.nim:192 and protocols/kademlia/types.nim:364, so this is the established convention rather than a new one.
  • times.parse range-checks the hour but reads the minute and second straight into dateTime(), where an out-of-range value becomes a RangeDefect. Same escaping-Defect class as symptom 4, in a different field. Guard the fields before parsing.
  • times.parse's zzz pattern reads the : of a numeric offset with no bounds check, so a truncated offset such as +12 raises IndexDefect. Guard the offset shape too. RFC 3339 allows only Z or exactly six characters.
  • parseutils.parseInt can raise ValueError on overflow, so it does not compile under {.push raises: [].}. parseSaturatedNatural is the non-raising equivalent.

There is no ready-made helper to reach for. Nim's times has no RFC 3339 entry point. Of the two Nimble candidates, Skrylar/rfc3339 has 9 stars, no license on the repo and no commit since 2020, and treeform/chrono is MIT and maintained but would be a new third-party dependency in a base library. npeg ships an RFC 3339 grammar, but it validates shape only and produces no DateTime.

Two test-side consequences.

PeerIDAuthClientStub builds its authentication-info header from a local DateTime and labels it Z, so the two errors cancel and the bearer tests in tests/libp2p/autotls/test_broker.nim pass in every timezone. Fixing the parser alone turns them red east of UTC by more than an hour. The stub needs .utc() in the same change.

Four characterization tests in tests/libp2p/autotls/test_peer_id_auth.nim pin all four symptoms and carry a TODO pointing here. They assert the broken behaviour deliberately and go red when this is fixed.

Related

#2972, where a dropped expiry means the client keeps sending a bearer the broker has already stopped accepting.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions