diff options
author | Pauli <pauli@openssl.org> | 2022-08-16 11:05:02 +1000 |
---|---|---|
committer | Pauli <pauli@openssl.org> | 2022-08-22 14:44:19 +1000 |
commit | ec47c3060bd3b7c3e75dbcfada113de9d94de747 (patch) | |
tree | 7aafe203cb5370b4c48ca65538cc47368ec05c20 /ssl | |
parent | aac139414e6f8fdfb4509f40811f1771f3ec193d (diff) |
Coverity 1508506: misuse of time_t
Fixes a bug in the cookie code which would have caused problems for ten
minutes before and after the lower 32 bits of time_t rolled over.
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19023)
Diffstat (limited to 'ssl')
-rw-r--r-- | ssl/statem/extensions_srvr.c | 14 |
1 files changed, 7 insertions, 7 deletions
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 6b1bf9a913..15c4e8af9e 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -12,16 +12,16 @@ #include "statem_local.h" #include "internal/cryptlib.h" -#define COOKIE_STATE_FORMAT_VERSION 0 +#define COOKIE_STATE_FORMAT_VERSION 1 /* * 2 bytes for packet length, 2 bytes for format version, 2 bytes for * protocol version, 2 bytes for group id, 2 bytes for cipher id, 1 byte for - * key_share present flag, 4 bytes for timestamp, 2 bytes for the hashlen, + * key_share present flag, 8 bytes for timestamp, 2 bytes for the hashlen, * EVP_MAX_MD_SIZE for transcript hash, 1 byte for app cookie length, app cookie * length bytes, SHA256_DIGEST_LENGTH bytes for the HMAC of the whole thing. */ -#define MAX_COOKIE_SIZE (2 + 2 + 2 + 2 + 2 + 1 + 4 + 2 + EVP_MAX_MD_SIZE + 1 \ +#define MAX_COOKIE_SIZE (2 + 2 + 2 + 2 + 2 + 1 + 8 + 2 + EVP_MAX_MD_SIZE + 1 \ + SSL_COOKIE_LENGTH + SHA256_DIGEST_LENGTH) /* @@ -690,7 +690,7 @@ int tls_parse_ctos_cookie(SSL *s, PACKET *pkt, unsigned int context, X509 *x, unsigned char hmac[SHA256_DIGEST_LENGTH]; unsigned char hrr[MAX_HRR_SIZE]; size_t rawlen, hmaclen, hrrlen, ciphlen; - unsigned long tm, now; + uint64_t tm, now; /* Ignore any cookie if we're not set up to verify it */ if (s->ctx->verify_stateless_cookie_cb == NULL @@ -791,7 +791,7 @@ int tls_parse_ctos_cookie(SSL *s, PACKET *pkt, unsigned int context, X509 *x, } if (!PACKET_get_1(&cookie, &key_share) - || !PACKET_get_net_4(&cookie, &tm) + || !PACKET_get_net_8(&cookie, &tm) || !PACKET_get_length_prefixed_2(&cookie, &chhash) || !PACKET_get_length_prefixed_1(&cookie, &appcookie) || PACKET_remaining(&cookie) != SHA256_DIGEST_LENGTH) { @@ -800,7 +800,7 @@ int tls_parse_ctos_cookie(SSL *s, PACKET *pkt, unsigned int context, X509 *x, } /* We tolerate a cookie age of up to 10 minutes (= 60 * 10 seconds) */ - now = (unsigned long)time(NULL); + now = time(NULL); if (tm > now || (now - tm) > 600) { /* Cookie is stale. Ignore it */ return 1; @@ -1740,7 +1740,7 @@ EXT_RETURN tls_construct_stoc_cookie(SSL *s, WPACKET *pkt, unsigned int context, &ciphlen) /* Is there a key_share extension present in this HRR? */ || !WPACKET_put_bytes_u8(pkt, s->s3.peer_tmp == NULL) - || !WPACKET_put_bytes_u32(pkt, (unsigned int)time(NULL)) + || !WPACKET_put_bytes_u64(pkt, time(NULL)) || !WPACKET_start_sub_packet_u16(pkt) || !WPACKET_reserve_bytes(pkt, EVP_MAX_MD_SIZE, &hashval1)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); |