Skip to content

Commit 08ce96d

Browse files
committed
Remove empty-label lower-bound check in CRYPTO_tls13_hkdf_expand_label
The RFC 8446 Section 7.1 "opaque label<7..255>" lower bound on the HkdfLabel.label field does not match real caller behavior: SSL_export_keying_material permits a zero-length caller label, which the ssl/test/runner exerciser relies on for TLS 1.3 exporter interop tests. Neither the pre-refactor ssl/tls13_enc.cc hkdf_expand_label nor BoringSSL's CRYPTO_tls13_hkdf_expand_label enforce the lower bound, so rejecting label_len == 0 here regressed every TLS-TLS13-* and QUIC-TLS13-* runner test that uses exportKeyingMaterial. The upper bound on out_len is already enforced implicitly by CBB_add_u16, so the explicit check is redundant. Drop the whole block and document what CBB enforces for us.
1 parent aa53c55 commit 08ce96d

1 file changed

Lines changed: 6 additions & 13 deletions

File tree

  • crypto/fipsmodule/tls

crypto/fipsmodule/tls/kdf.c

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -150,26 +150,19 @@ int CRYPTO_tls13_hkdf_expand_label(uint8_t *out, size_t out_len,
150150
int ret = 0;
151151

152152
CBB_zero(&cbb);
153-
// RFC 8446 Section 7.1 constrains the HkdfLabel.length field to a uint16 and
154-
// the HkdfLabel.label field to |opaque label<7..255>|. Since |kProtocolLabel|
155-
// ("tls13 ") is 6 bytes, the caller-provided |label| must be at least 1 byte
156-
// for the combined label to satisfy the RFC lower bound. The upper bound
157-
// (combined label <= 255) and the |hash_len| bound are enforced implicitly by
158-
// the CBB length-prefixed calls below.
159-
if (out_len > UINT16_MAX || label_len == 0) {
160-
goto end;
161-
}
162-
163153
// Construct the HkdfLabel structure per RFC 8446 Section 7.1:
164154
// struct {
165155
// uint16 length = Length;
166156
// opaque label<7..255> = "tls13 " + Label;
167157
// opaque context<0..255> = Context;
168158
// };
169159
// The CBB_add_u16 / CBB_add_u8_length_prefixed calls implicitly enforce the
170-
// RFC-mandated length upper bounds and will fail if |out_len| does not fit
171-
// in a |uint16_t|, if the "tls13 "-prefixed label exceeds 255 bytes, or if
172-
// |hash| exceeds 255 bytes.
160+
// RFC-mandated upper bounds: CBB_add_u16 fails if |out_len| does not fit in a
161+
// uint16_t, and the u8-length-prefixed children fail if the "tls13 "-prefixed
162+
// label or |hash| exceed 255 bytes. The RFC's lower bound on the label field
163+
// (|opaque label<7..255>|, i.e. |label_len| >= 1) is intentionally not
164+
// enforced here: SSL_export_keying_material callers may pass an empty label,
165+
// and this matches the pre-existing AWS-LC and BoringSSL behavior.
173166
if (!CBB_init(&cbb, 2 + 1 + kProtocolLabelLen + label_len + 1 + hash_len) ||
174167
!CBB_add_u16(&cbb, out_len) ||
175168
!CBB_add_u8_length_prefixed(&cbb, &child) ||

0 commit comments

Comments
 (0)