diff options
-rw-r--r-- | lib/ssl/ssl3ext.c | 80 |
1 files changed, 48 insertions, 32 deletions
diff --git a/lib/ssl/ssl3ext.c b/lib/ssl/ssl3ext.c index e88b60a77..d4e58b2af 100644 --- a/lib/ssl/ssl3ext.c +++ b/lib/ssl/ssl3ext.c @@ -603,17 +603,11 @@ ssl3_ValidateNextProtoNego(const unsigned char* data, unsigned int length) * store protocol identifiers in null-terminated strings. */ if (newOffset > length || data[offset] == 0) { - PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID); return SECFailure; } offset = newOffset; } - if (offset > length) { - PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID); - return SECFailure; - } - return SECSuccess; } @@ -626,34 +620,41 @@ ssl3_SelectAppProtocol(sslSocket *ss, PRUint16 ex_type, SECItem *data) SECItem result = { siBuffer, resultBuffer, 0 }; rv = ssl3_ValidateNextProtoNego(data->data, data->len); - if (rv != SECSuccess) + if (rv != SECSuccess) { + PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID); + (void)SSL3_SendAlert(ss, alert_fatal, decode_error); return rv; + } PORT_Assert(ss->nextProtoCallback); rv = ss->nextProtoCallback(ss->nextProtoArg, ss->fd, data->data, data->len, - result.data, &result.len, sizeof resultBuffer); - if (rv != SECSuccess) - return rv; + result.data, &result.len, sizeof(resultBuffer)); + if (rv != SECSuccess) { + /* Expect callback to call PORT_SetError() */ + (void)SSL3_SendAlert(ss, alert_fatal, internal_error); + return SECFailure; + } + /* If the callback wrote more than allowed to |result| it has corrupted our * stack. */ - if (result.len > sizeof resultBuffer) { + if (result.len > sizeof(resultBuffer)) { PORT_SetError(SEC_ERROR_OUTPUT_LEN); + /* TODO: crash */ return SECFailure; } + SECITEM_FreeItem(&ss->ssl3.nextProto, PR_FALSE); + if (ex_type == ssl_app_layer_protocol_xtn && ss->ssl3.nextProtoState != SSL_NEXT_PROTO_NEGOTIATED) { - /* The callback might say OK, but then it's picked a default. - * That's OK for NPN, but not ALPN. */ - SECITEM_FreeItem(&ss->ssl3.nextProto, PR_FALSE); + /* The callback might say OK, but then it picks a default value - one + * that was not listed. That's OK for NPN, but not ALPN. */ PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_NO_PROTOCOL); (void)SSL3_SendAlert(ss, alert_fatal, no_application_protocol); return SECFailure; } ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ex_type; - - SECITEM_FreeItem(&ss->ssl3.nextProto, PR_FALSE); return SECITEM_CopyItem(NULL, &ss->ssl3.nextProto, &result); } @@ -669,17 +670,16 @@ ssl3_ServerHandleAppProtoXtn(sslSocket *ss, PRUint16 ex_type, SECItem *data) if (ss->firstHsDone || data->len == 0) { /* Clients MUST send a non-empty ALPN extension. */ PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID); + (void)SSL3_SendAlert(ss, alert_fatal, illegal_parameter); return SECFailure; } - /* unlike NPN, ALPN has extra redundant length information so that - * the extension is the same in both ClientHello and ServerHello */ + /* Unlike NPN, ALPN has extra redundant length information so that + * the extension is the same in both ClientHello and ServerHello. */ count = ssl3_ConsumeHandshakeNumber(ss, 2, &data->data, &data->len); - if (count < 0) { - return SECFailure; /* fatal alert was sent */ - } if (count != data->len) { - return ssl3_DecodeError(ss); + (void)ssl3_DecodeError(ss); + return SECFailure; } if (!ss->nextProtoCallback) { @@ -694,8 +694,13 @@ ssl3_ServerHandleAppProtoXtn(sslSocket *ss, PRUint16 ex_type, SECItem *data) /* prepare to send back a response, if we negotiated */ if (ss->ssl3.nextProtoState == SSL_NEXT_PROTO_NEGOTIATED) { - return ssl3_RegisterServerHelloExtensionSender( + rv = ssl3_RegisterServerHelloExtensionSender( ss, ex_type, ssl3_ServerSendAppProtoXtn); + if (rv != SECSuccess) { + PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); + (void)SSL3_SendAlert(ss, alert_fatal, internal_error); + return rv; + } } return SECSuccess; } @@ -713,7 +718,8 @@ ssl3_ClientHandleNextProtoNegoXtn(sslSocket *ss, PRUint16 ex_type, * we've negotiated NPN then we're required to send the NPN handshake * message. Thus, these two extensions cannot both be negotiated on the * same connection. */ - PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); + PORT_SetError(SSL_ERROR_BAD_SERVER); + (void)SSL3_SendAlert(ss, alert_fatal, illegal_parameter); return SECFailure; } @@ -722,7 +728,9 @@ ssl3_ClientHandleNextProtoNegoXtn(sslSocket *ss, PRUint16 ex_type, * that an application erroneously cleared the callback between the time * we sent the ClientHello and now. */ if (!ss->nextProtoCallback) { + PORT_Assert(0); PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_NO_CALLBACK); + (void)SSL3_SendAlert(ss, alert_fatal, internal_error); return SECFailure; } @@ -732,8 +740,8 @@ ssl3_ClientHandleNextProtoNegoXtn(sslSocket *ss, PRUint16 ex_type, static SECStatus ssl3_ClientHandleAppProtoXtn(sslSocket *ss, PRUint16 ex_type, SECItem *data) { - const unsigned char* d = data->data; - PRUint16 name_list_len; + SECStatus rv; + PRInt32 list_len; SECItem protocol_name; if (ssl3_ExtensionNegotiated(ss, ssl_next_proto_nego_xtn)) { @@ -743,22 +751,30 @@ ssl3_ClientHandleAppProtoXtn(sslSocket *ss, PRUint16 ex_type, SECItem *data) /* The extension data from the server has the following format: * uint16 name_list_len; - * uint8 len; + * uint8 len; // where len >= 1 * uint8 protocol_name[len]; */ if (data->len < 4 || data->len > 2 + 1 + 255) { PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID); + (void)SSL3_SendAlert(ss, alert_fatal, decode_error); return SECFailure; } - name_list_len = ((PRUint16) d[0]) << 8 | - ((PRUint16) d[1]); - if (name_list_len != data->len - 2 || d[2] != data->len - 3) { + list_len = ssl3_ConsumeHandshakeNumber(ss, 2, &data->data, &data->len); + /* The list has to be the entire extension. */ + if (list_len != data->len) { PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID); + (void)SSL3_SendAlert(ss, alert_fatal, decode_error); return SECFailure; } - protocol_name.data = data->data + 3; - protocol_name.len = data->len - 3; + rv = ssl3_ConsumeHandshakeVariable(ss, &protocol_name, 1, + &data->data, &data->len); + /* The list must have exactly one value. */ + if (rv != SECSuccess || data->len != 0) { + PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID); + (void)SSL3_SendAlert(ss, alert_fatal, decode_error); + return SECFailure; + } SECITEM_FreeItem(&ss->ssl3.nextProto, PR_FALSE); ss->ssl3.nextProtoState = SSL_NEXT_PROTO_SELECTED; |