Explicitly reject self-referential ech_outer_extensions.
The ECH extension is not covered in the AAD and so should not be
referenced in ech_outer_extensions. We end up rejecting this anyway when
checking for valid ClientHelloInners, but better to reject this
explicitly, as the spec suggests.
As part of this, use the more specific error in the various tests, so we
can distinguish the two cases. (DECODE_ERROR is coming from an extra,
probably unnecessary, error in ssl_decode_client_hello_inner's caller.)
Bug: 275
Change-Id: Ibeff55e5e1b7646ce9c68c5847cd1b40a47e6480
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51185
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata
index 6879134..4205402 100644
--- a/crypto/err/ssl.errordata
+++ b/crypto/err/ssl.errordata
@@ -90,6 +90,7 @@
SSL,318,INVALID_ECH_CONFIG_LIST
SSL,317,INVALID_ECH_PUBLIC_NAME
SSL,159,INVALID_MESSAGE
+SSL,320,INVALID_OUTER_EXTENSION
SSL,251,INVALID_OUTER_RECORD_TYPE
SSL,269,INVALID_SCT_LIST
SSL,295,INVALID_SIGNATURE_ALGORITHM
@@ -133,7 +134,6 @@
SSL,187,OLD_SESSION_CIPHER_NOT_RETURNED
SSL,268,OLD_SESSION_PRF_HASH_MISMATCH
SSL,188,OLD_SESSION_VERSION_NOT_RETURNED
-SSL,320,OUTER_EXTENSION_NOT_FOUND
SSL,189,OUTPUT_ALIASES_INPUT
SSL,190,PARSE_TLSEXT
SSL,191,PATH_TOO_LONG
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 232c627..0aaabfa 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -5598,7 +5598,7 @@
#define SSL_R_INVALID_ECH_PUBLIC_NAME 317
#define SSL_R_INVALID_ECH_CONFIG_LIST 318
#define SSL_R_ECH_REJECTED 319
-#define SSL_R_OUTER_EXTENSION_NOT_FOUND 320
+#define SSL_R_INVALID_OUTER_EXTENSION 320
#define SSL_R_INCONSISTENT_ECH_NEGOTIATION 321
#define SSL_R_SSLV3_ALERT_CLOSE_NOTIFY 1000
#define SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE 1010
diff --git a/ssl/encrypted_client_hello.cc b/ssl/encrypted_client_hello.cc
index 64fee3d..84b6fa9 100644
--- a/ssl/encrypted_client_hello.cc
+++ b/ssl/encrypted_client_hello.cc
@@ -203,6 +203,12 @@
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
return false;
}
+ // The ECH extension itself is not in the AAD and may not be referenced.
+ if (want == TLSEXT_TYPE_encrypted_client_hello) {
+ *out_alert = SSL_AD_ILLEGAL_PARAMETER;
+ OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_OUTER_EXTENSION);
+ return false;
+ }
// Seek to |want| in |outer_extensions|. |ext_list| is required to match
// ClientHelloOuter in order.
uint16_t found;
@@ -210,7 +216,7 @@
do {
if (CBS_len(&outer_extensions) == 0) {
*out_alert = SSL_AD_ILLEGAL_PARAMETER;
- OPENSSL_PUT_ERROR(SSL, SSL_R_OUTER_EXTENSION_NOT_FOUND);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_OUTER_EXTENSION);
return false;
}
if (!CBS_get_u16(&outer_extensions, &found) ||
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index cfff714..acffde9 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -16776,9 +16776,7 @@
},
shouldFail: true,
expectedLocalError: "remote error: illegal parameter",
- // The decoding algorithm relies on the ordering requirement, so
- // the wrong order appears as a missing extension.
- expectedError: ":OUTER_EXTENSION_NOT_FOUND:",
+ expectedError: ":INVALID_OUTER_EXTENSION:",
})
// Test that the server rejects duplicated values in ech_outer_extensions.
@@ -16812,9 +16810,7 @@
},
shouldFail: true,
expectedLocalError: "remote error: illegal parameter",
- // The decoding algorithm relies on the ordering requirement, so
- // duplicates appear as missing extensions.
- expectedError: ":OUTER_EXTENSION_NOT_FOUND:",
+ expectedError: ":INVALID_OUTER_EXTENSION:",
})
// Test that the server rejects references to missing extensions in
@@ -16843,7 +16839,7 @@
},
shouldFail: true,
expectedLocalError: "remote error: illegal parameter",
- expectedError: ":DECODE_ERROR:",
+ expectedError: ":INVALID_OUTER_EXTENSION:",
})
// Test that the server rejects a references to the ECH extension in
@@ -16871,7 +16867,7 @@
},
shouldFail: true,
expectedLocalError: "remote error: illegal parameter",
- expectedError: ":DECODE_ERROR:",
+ expectedError: ":INVALID_OUTER_EXTENSION:",
})
}
