diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata index f0f450e8..e9b20664 100644 --- a/crypto/err/ssl.errordata +++ b/crypto/err/ssl.errordata @@ -109,6 +109,7 @@ SSL,192,PEER_DID_NOT_RETURN_A_CERTIFICATE SSL,193,PEER_ERROR_UNSUPPORTED_CERTIFICATE_TYPE SSL,267,PRE_SHARED_KEY_MUST_BE_LAST SSL,194,PROTOCOL_IS_SHUTDOWN +SSL,271,PSK_IDENTITY_BINDER_COUNT_MISMATCH SSL,195,PSK_IDENTITY_NOT_FOUND SSL,196,PSK_NO_CLIENT_CB SSL,197,PSK_NO_SERVER_CB diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index f588f46b..796949d8 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -4566,6 +4566,7 @@ BORINGSSL_MAKE_DELETER(SSL_SESSION, SSL_SESSION_free) #define SSL_R_OLD_SESSION_PRF_HASH_MISMATCH 268 #define SSL_R_INVALID_SCT_LIST 269 #define SSL_R_TOO_MUCH_SKIPPED_EARLY_DATA 270 +#define SSL_R_PSK_IDENTITY_BINDER_COUNT_MISMATCH 271 #define SSL_R_SSLV3_ALERT_CLOSE_NOTIFY 1000 #define SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE 1010 #define SSL_R_SSLV3_ALERT_BAD_RECORD_MAC 1020 diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 21005f00..19c36d76 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1977,11 +1977,12 @@ int ssl_ext_pre_shared_key_parse_clienthello(SSL *ssl, CBS *contents) { /* We only process the first PSK identity since we don't support pure PSK. */ uint32_t obfuscated_ticket_age; - CBS identity, ticket, binders; - if (!CBS_get_u16_length_prefixed(contents, &identity) || - !CBS_get_u16_length_prefixed(&identity, &ticket) || - !CBS_get_u32(&identity, &obfuscated_ticket_age) || + CBS identities, ticket, binders; + if (!CBS_get_u16_length_prefixed(contents, &identities) || + !CBS_get_u16_length_prefixed(&identities, &ticket) || + !CBS_get_u32(&identities, &obfuscated_ticket_age) || !CBS_get_u16_length_prefixed(contents, &binders) || + CBS_len(&binders) == 0 || CBS_len(contents) != 0) { OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); *out_alert = SSL_AD_DECODE_ERROR; @@ -1990,11 +1991,38 @@ int ssl_ext_pre_shared_key_parse_clienthello(SSL *ssl, *out_binders = binders; - /* The PSK identity must have a corresponding binder. */ - CBS binder; - if (!CBS_get_u8_length_prefixed(&binders, &binder)) { - OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); - *out_alert = SSL_AD_DECODE_ERROR; + /* Check the syntax of the remaining identities, but do not process them. */ + size_t num_identities = 1; + while (CBS_len(&identities) != 0) { + CBS unused_ticket; + uint32_t unused_obfuscated_ticket_age; + if (!CBS_get_u16_length_prefixed(&identities, &unused_ticket) || + !CBS_get_u32(&identities, &unused_obfuscated_ticket_age)) { + OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); + *out_alert = SSL_AD_DECODE_ERROR; + return 0; + } + + num_identities++; + } + + /* Check the syntax of the binders. The value will be checked later if + * resuming. */ + size_t num_binders = 0; + while (CBS_len(&binders) != 0) { + CBS binder; + if (!CBS_get_u8_length_prefixed(&binders, &binder)) { + OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); + *out_alert = SSL_AD_DECODE_ERROR; + return 0; + } + + num_binders++; + } + + if (num_identities != num_binders) { + OPENSSL_PUT_ERROR(SSL, SSL_R_PSK_IDENTITY_BINDER_COUNT_MISMATCH); + *out_alert = SSL_AD_ILLEGAL_PARAMETER; return 0; } diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index caa9cde8..9fbf5dc9 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go @@ -1217,6 +1217,10 @@ type ProtocolBugs struct { // SendNoPSKBinder, if true, causes the client to send no PSK binders. SendNoPSKBinder bool + // SendExtraPSKBinder, if true, causes the client to send an extra PSK + // binder. + SendExtraPSKBinder bool + // PSKBinderFirst, if true, causes the client to send the PSK Binder // extension as the first extension instead of the last extension. PSKBinderFirst bool diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index 2ebd0dcd..e138f228 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go @@ -1676,11 +1676,16 @@ func generatePSKBinders(hello *clientHelloMsg, pskCipherSuite *cipherSuite, psk, binderLen-- } + numBinders := 1 + if config.Bugs.SendExtraPSKBinder { + numBinders++ + } + // Fill hello.pskBinders with appropriate length arrays of zeros so the // length prefixes are correct when computing the binder over the truncated // ClientHello message. - hello.pskBinders = make([][]byte, len(hello.pskIdentities)) - for i := range hello.pskIdentities { + hello.pskBinders = make([][]byte, numBinders) + for i := range hello.pskBinders { hello.pskBinders[i] = make([]byte, binderLen) } diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 35742cb1..578d7355 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -5977,6 +5977,36 @@ func addResumptionVersionTests() { expectedError: ":DECODE_ERROR:", }) + testCases = append(testCases, testCase{ + testType: serverTest, + name: "Resume-Server-ExtraPSKBinder", + resumeSession: true, + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + SendExtraPSKBinder: true, + }, + }, + shouldFail: true, + expectedLocalError: "remote error: illegal parameter", + expectedError: ":PSK_IDENTITY_BINDER_COUNT_MISMATCH:", + }) + + testCases = append(testCases, testCase{ + testType: serverTest, + name: "Resume-Server-ExtraIdentityNoBinder", + resumeSession: true, + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + ExtraPSKIdentity: true, + }, + }, + shouldFail: true, + expectedLocalError: "remote error: illegal parameter", + expectedError: ":PSK_IDENTITY_BINDER_COUNT_MISMATCH:", + }) + testCases = append(testCases, testCase{ testType: serverTest, name: "Resume-Server-InvalidPSKBinder", @@ -9502,7 +9532,8 @@ func addTLS13HandshakeTests() { config: Config{ MaxVersion: VersionTLS13, Bugs: ProtocolBugs{ - ExtraPSKIdentity: true, + ExtraPSKIdentity: true, + SendExtraPSKBinder: true, }, }, resumeSession: true,