Arnd Bergmann <[email protected]> wrote:
> > struct rxkad_key {
> > u32 vice_id;
> > u32 start; /* time at which ticket starts */
> > - u32 expiry; /* time at which ticket expires */
> > + u64 expiry; /* time at which ticket expires */
> > u32 kvno; /* key version number */
> > u8 primary_flag; /* T if key for primary cell for this
> > user */
> > u16 ticket_len; /* length of ticket[] */
>
> So the expiry is currently an unsigned 32-bit value in this structure,
> presumably representing time from 1970 to 2106.
Not only that, the start value is also a time. However, I would recommend
that you leave the times in struct rxkad_key as u32. That's the way it is in
the network protocol, so by changing this to u64 you make that less obvious.
Note that rxkad_key only applies to type-2 security which is obsolete in
favour of type-5 (see struct rxk5_key) or GSSAPI.
> > - token->kad->expiry = ntohl(xdr[5]);
> > + token->kad->expiry = (long long)ntohl(xdr[5]);
The cast is unnecessary since the result of ntohl() is unsigned.
> The put_time changes look much simpler to me than the rest, and this
> can easily be split out from the rest of the changes into a separate
> self-contained patch, making it clear that those times are never
> transferred across the wire.
Yep.
> I'd use "unsigned long" as the type here, matching what you have in
> the local variables in rxrpc_connection_reaper, and change the comment
> to reflect that this is monotonic time.
Or change the types of the variables in rxrpc_connection_reaper().
> > @@ -45,7 +45,7 @@ struct key_preparsed_payload {
> > const void *data; /* Raw data */
> > size_t datalen; /* Raw datalen */
> > size_t quotalen; /* Quota length for proposed payload */
> > - time_t expiry; /* Expiry time of key */
> > + time64_t expiry; /* Expiry time of key */
Can the core keyrings code changes be a separate patch?
> The keys/key.c code sets TIME_T_MAX as the default, and out of the
> many implementations in the kernel, only the rxrpc one overrides this,
The asymmetric key code was going to also, but public-key crypto keys may
still need to be used, even after they've expired - and there are problems if
the system clock is incorrect at the time of usage.
> @@ -101,7 +101,7 @@ struct rxrpc_key_token {
> struct rxrpc_key_data_v1 {
> u16 security_index;
> u16 ticket_length;
> - u32 expiry; /* time_t */
> + u64 expiry; /* time_t */
> u32 kvno;
> u8 session_key[8];
> u8 ticket[0];
This is UAPI so you can't change it. I should probably move it somewhere
under include/uapi/
You should leave this as it is since it supports an obsolete security protocol
in which the on-the-wire time is 32-bits (see struct rxkad_key above).
> > - ENCODE(token->kad->expiry);
> > + ENCODE32(token->kad->expiry);
I recommend leaving this unchanged. Again we're dealing with an obsolete
security protocol and UAPI. People should be using RXK5 at least instead.
> > - issue = le32_to_cpu(stamp);
> > + issue = le64_to_cpu((__le64)stamp);
> > } else {
> > __be32 stamp;
> > memcpy(&stamp, p, 4);
> > - issue = be32_to_cpu(stamp);
> > + issue = be64_to_cpu((__be64)stamp);
Firstly, the protocol element is 32 bits, therefore we should be using
be32/le32 functions; secondly, this will not work on an LE machine - and is
dodgy even on a BE machine.
> This means that here we have to change the interpretation of the
> 32-bit value. This is separate from the rxrpc_key_data_v1 structure,
> but we can do the same approach I described above and add a helper
> function that turns the 32-bit value into a time64_t, such as this:
No. There's nothing you can do to fix it. Just insert a comment in noting
the limitation of the protocol. Something like this:
/* The kerberos 4 time value is traditionally a 32-bit time_t, which
* overflows in 2038. By interpreting it as an unsigned value here, we
* gain an extra 68 years until 2106. This works because we know the
* timestamp can never be negative.
*/
woud do. You could rename the variable 'issue' to 'issue32' or something like
that and change the type to be u32 rather than time_t.
David
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038
