Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1339418 - Reject ALPN values that weren't advertised, r=ttaubert
--HG--
extra : rebase_source : 281449f20c3dae18ae9bd2b2068bb80c16aafc6e
extra : amend_source : 68c6ed7de4242e56d718c3eb71fd851b269ac099
  • Loading branch information
martinthomson committed Feb 24, 2017
1 parent 9d4da13 commit eb6681e
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 23 deletions.
2 changes: 1 addition & 1 deletion gtests/nss_bogo_shim/config.json
Expand Up @@ -55,7 +55,7 @@
"UnsolicitedServerNameAck-TLS1*":"Boring wants us to fail with an unexpected_extension alert, we simply ignore ssl_server_name_xtn.",
"RequireAnyClientCertificate-TLS1*":"Bug 1339387",
"SendExtensionOnClientCertificate-TLS13":"Bug 1339392",
"ALPNClient-Mismatch-TLS1*":"Bug 1339418"
"ALPNClient-Mismatch-TLS13": "NSS sends alerts in response to errors in protected handshake messages in the clear"
},
"ErrorMap" : {
":HANDSHAKE_FAILURE_ON_CLIENT_HELLO:":"SSL_ERROR_NO_CYPHER_OVERLAP",
Expand Down
8 changes: 8 additions & 0 deletions gtests/ssl_gtest/ssl_extension_unittest.cc
Expand Up @@ -476,6 +476,14 @@ TEST_P(TlsExtensionTestPre13, AlpnReturnedBadNameLength) {
ssl_app_layer_protocol_xtn, extension));
}

TEST_P(TlsExtensionTestPre13, AlpnReturnedUnknownName) {
EnableAlpn();
const uint8_t val[] = {0x00, 0x02, 0x01, 0x67};
DataBuffer extension(val, sizeof(val));
ServerHelloErrorTest(std::make_shared<TlsExtensionReplacer>(
ssl_app_layer_protocol_xtn, extension), kTlsAlertIllegalParameter);
}

TEST_P(TlsExtensionTestDtls, SrtpShort) {
EnableSrtp();
ClientHelloErrorTest(
Expand Down
27 changes: 27 additions & 0 deletions lib/ssl/ssl3exthandle.c
Expand Up @@ -354,6 +354,27 @@ ssl3_ParseEncryptedSessionTicket(sslSocket *ss, SECItem *data,
return SECSuccess;
}

PRBool
ssl_AlpnTagAllowed(const sslSocket *ss, const SECItem *tag)
{
const unsigned char *data = ss->opt.nextProtoNego.data;
unsigned int length = ss->opt.nextProtoNego.len;
unsigned int offset = 0;

if (!tag->len)
return PR_TRUE;

while (offset < length) {
unsigned int taglen = (unsigned int)data[offset];
if ((taglen == tag->len) &&
!PORT_Memcmp(data + offset + 1, tag->data, tag->len))
return PR_TRUE;
offset += 1 + taglen;
}

return PR_FALSE;
}

/* handle an incoming Next Protocol Negotiation extension. */
SECStatus
ssl3_ServerHandleNextProtoNegoXtn(const sslSocket *ss, TLSExtensionData *xtnData, PRUint16 ex_type,
Expand Down Expand Up @@ -568,6 +589,12 @@ ssl3_ClientHandleAppProtoXtn(const sslSocket *ss, TLSExtensionData *xtnData, PRU
return SECFailure;
}

if (!ssl_AlpnTagAllowed(ss, &protocol_name)) {
ssl3_ExtSendAlert(ss, alert_fatal, illegal_parameter);
PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID);
return SECFailure;
}

SECITEM_FreeItem(&xtnData->nextProto, PR_FALSE);
xtnData->nextProtoState = SSL_NEXT_PROTO_SELECTED;
xtnData->negotiated[xtnData->numNegotiated++] = ex_type;
Expand Down
2 changes: 2 additions & 0 deletions lib/ssl/sslimpl.h
Expand Up @@ -1856,6 +1856,8 @@ ssl3_TLSPRFWithMasterSecret(sslSocket *ss, ssl3CipherSpec *spec,
const unsigned char *val, unsigned int valLen,
unsigned char *out, unsigned int outLen);

PRBool ssl_AlpnTagAllowed(const sslSocket *ss, const SECItem *tag);

#ifdef TRACE
#define SSL_TRACE(msg) ssl_Trace msg
#else
Expand Down
23 changes: 1 addition & 22 deletions lib/ssl/tls13con.c
Expand Up @@ -941,27 +941,6 @@ tls13_CanResume(sslSocket *ss, const sslSessionID *sid)
return PR_TRUE;
}

static PRBool
tls13_AlpnTagAllowed(const sslSocket *ss, const SECItem *tag)
{
const unsigned char *data = ss->opt.nextProtoNego.data;
unsigned int length = ss->opt.nextProtoNego.len;
unsigned int offset = 0;

if (!tag->len)
return PR_TRUE;

while (offset < length) {
unsigned int taglen = (unsigned int)data[offset];
if ((taglen == tag->len) &&
!PORT_Memcmp(data + offset + 1, tag->data, tag->len))
return PR_TRUE;
offset += 1 + taglen;
}

return PR_FALSE;
}

static PRBool
tls13_CanNegotiateZeroRtt(sslSocket *ss, const sslSessionID *sid)
{
Expand Down Expand Up @@ -4296,7 +4275,7 @@ tls13_ClientAllow0Rtt(const sslSocket *ss, const sslSessionID *sid)
return PR_FALSE;
if ((sid->u.ssl3.locked.sessionTicket.flags & ticket_allow_early_data) == 0)
return PR_FALSE;
return tls13_AlpnTagAllowed(ss, &sid->u.ssl3.alpnSelection);
return ssl_AlpnTagAllowed(ss, &sid->u.ssl3.alpnSelection);
}

SECStatus
Expand Down

0 comments on commit eb6681e

Please sign in to comment.