Key expiration time doesn't accept times beyond 2106-02-07 06:28:15 UTC

[ribose-jeffreylau]

Description

Key expiration time doesn't accept times beyond 2106-02-07 06:28:15 UTC
(== Time.at(('1' * 32).to_i(2))).

A RangeError would be thrown.

Steps to reproduce

This would be fine:

key.add_userid(
  userid,
  hash: 'SHA256',
  expiration_time: ('1' * 32).to_i(2)
)

This would fail:

key.add_userid(
  userid,
  hash: 'SHA256',
  expiration_time: ('1' * 32).to_i(2) + 1
)
# => RangeError: integer 4294967296 too big to convert to `unsigned int'

Expected results

It would either happily consume it or emit an error that is more meaningful,
e.g. RangeError: expiration time beyond 4294967295 is unsupported.

[dewyatt]

See https://www.rubydoc.info/github/riboseinc/ruby-rnp/Rnp/Key#add_userid-instance_method. Although the name may be confusing, expiration_time is the lifetime/duration of the signature, in seconds. So in the above code it is being fed an invalid value.

[ribose-jeffreylau]

Thanks @dewyatt for the explanation. The valid range isn't clear from the Ruby documentation since the class is Integer. I found the relevant bit in the RFC (4-octet time field) which perhaps would supplement the rubydoc well.

Maybe it's also good to make the error message clearer by saying specifically which field is invalid?

[dewyatt]

@ribose-jeffreylau I definitely think it can be improved.

Honestly I'd really like to change all these "expiration" parameters to something else, like validity_seconds or lifetime_sec, or something more intuitive. The RFC uses "expiration time" quite a bit in ways I find misleading.

[ribose-jeffreylau]

@dewyatt I was going to suggest renaming expiration_time to expiration_duration but I think your suggestions of validity_seconds and lifetime_sec are better as they're more explicit.

So it would emit errors like RangeError: validity_seconds beyond 4294967295 is unsupported by RFC4880.