Arnd Bergmann arnd@arndb.de 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);
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