Skip to content

Commit

Permalink
Relax check instead of version bump
Browse files Browse the repository at this point in the history
  • Loading branch information
@samuel40791765
samuel40791765 committed Nov 7, 2024
1 parent 4428704 commit f3b8dec
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 33 deletions.
22 changes: 13 additions & 9 deletions ssl/ssl_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7844,7 +7844,7 @@ struct EncodeDecodeKATTestParam {
};

static const EncodeDecodeKATTestParam kEncodeDecodeKATs[] = {
// V1 input round-trips as V3 output
// V1 input round-trips as V2 output
{"308201173082011302010102020303020240003081fa0201010408000000000000000104"
"0800000000000000010420000004d29e62f41ded4bb33d0faa6ffada380e2c489dfbfb44"
"4f574e475244010420cf3926d1ec5a562a642935a8050222b0aed93ffd9d1cac682274d9"
Expand All @@ -7853,7 +7853,7 @@ static const EncodeDecodeKATTestParam kEncodeDecodeKATs[] = {
"a80400043085668dcf9f0921094ebd7f91bf2a8c60d276e4c279fd85a989402f67868232"
"4fd8098dc19d900b856d0a77e048e3ced2a104020204d2a20402021c20a4020400b10301"
"01ffb20302011da206040474657374a7030101ff020108020100a0030101ff",
"308201803082017c02010102020303020240003082016202010304080000000000000001"
"308201803082017c02010102020303020240003082016202010204080000000000000001"
"040800000000000000010420000004d29e62f41ded4bb33d0faa6ffada380e2c489dfbfb"
"444f574e475244010420cf3926d1ec5a562a642935a8050222b0aed93ffd9d1cac682274"
"d942e99e42a6040200000201000201030440b9b409f5129440622f87f84402010c040c1f"
Expand All @@ -7867,7 +7867,9 @@ static const EncodeDecodeKATTestParam kEncodeDecodeKATs[] = {
// In runner.go, the test case "Basic-Server-TLS-Sync-SSL_Transfer" is used
// to generate below bytes by adding print statement on the output of
// |SSL_to_bytes| in bssl_shim.cc.
// V2 input round-trips as V3 output.
// We've bumped the buffer size in the |previous_client/server_finished|
// fields. This verifies that the original size is parsable and reencoded
// with the new size.
{"308201173082011302010102020303020240003081fa0201020408000000000000000104"
"0800000000000000010420000004d29e62f41ded4bb33d0faa6ffada380e2c489dfbfb44"
"4f574e475244010420cf3926d1ec5a562a642935a8050222b0aed93ffd9d1cac682274d9"
Expand All @@ -7876,7 +7878,7 @@ static const EncodeDecodeKATTestParam kEncodeDecodeKATs[] = {
"a80400043085668dcf9f0921094ebd7f91bf2a8c60d276e4c279fd85a989402f67868232"
"4fd8098dc19d900b856d0a77e048e3ced2a104020204d2a20402021c20a4020400b10301"
"01ffb20302011da206040474657374a7030101ff020108020100a0030101ff",
"308201803082017c02010102020303020240003082016202010304080000000000000001"
"308201803082017c02010102020303020240003082016202010204080000000000000001"
"040800000000000000010420000004d29e62f41ded4bb33d0faa6ffada380e2c489dfbfb"
"444f574e475244010420cf3926d1ec5a562a642935a8050222b0aed93ffd9d1cac682274"
"d942e99e42a6040200000201000201030440b9b409f5129440622f87f84402010c040c1f"
Expand All @@ -7891,8 +7893,10 @@ static const EncodeDecodeKATTestParam kEncodeDecodeKATs[] = {
// "TLS-TLS13-AES_128_GCM_SHA256-server-SSL_Transfer" is used to generate
// below bytes by adding print statement on the output of |SSL_to_bytes| in
// bssl_shim.cc.
// V2 input round-trips as V3 output.
{"308203883082038402010102020304020240003082036a020102040800000000000000000"
// We've bumped the buffer size in the |previous_client/server_finished|
// fields. This verifies that the original size is parsable and reencoded
// with the new size.
{"308203883082038402010102020304020240003082036a020102040800000000000000000"
"408000000000000000004206beca5c14aff6b92757545948b883c6c175327814bedcf38a6"
"b2e4c43bc02d180420a32aee5b7705a19e4bb2b47f4918199c76cee7245f1311bc4ba3888"
"3d33f236a04020000020100020101040c000000000000000000000000020100040c000000"
Expand All @@ -7917,7 +7921,7 @@ static const EncodeDecodeKATTestParam kEncodeDecodeKATs[] = {
"41be58ecbd60d18128dfa28f4b1a00042a2a0000ba2330210201010204030013013016020"
"101020117040e300c0201010201000201000101ffbb233021020101020403001301301602"
"0101020117040e300c0201010201000201000101ff020108020100a0030101ff",
"308203f0308203ec0201010202030402024000308203d202010304080000000000000000"
"308203f0308203ec0201010202030402024000308203d202010204080000000000000000"
"0408000000000000000004206beca5c14aff6b92757545948b883c6c175327814bedcf38"
"a6b2e4c43bc02d180420a32aee5b7705a19e4bb2b47f4918199c76cee7245f1311bc4ba3"
"8883d33f236a040200000201000201010440000000000000000000000000020100040c00"
Expand Down Expand Up @@ -7950,7 +7954,7 @@ static const EncodeDecodeKATTestParam kEncodeDecodeKATs[] = {
// "TLS-ECH-Server-Cipher-HKDF-SHA256-AES-256-GCM-SSL_Transfer" is used
// to generate below bytes by adding print statement on the output of
// |SSL_to_bytes| in bssl_shim.cc.
{"308203e3308203df0201010202030402024000308203c502010304080000000000000000"
{"308203e3308203df0201010202030402024000308203c502010204080000000000000000"
"04080000000000000000042028431b914ffdb44ea92ca53d5734976c6a16f141d44f180b"
"0816a5cb2b8e79030420bdaf544fa82d833d58c92213e44e850cc0b8147699b0b410d4aa"
"2a277030f3220402000002010002010104409e155007d04cd03cf4d8a95ce244dc978a87"
Expand Down Expand Up @@ -8013,7 +8017,7 @@ TEST_P(EncodeDecodeKATTest, RoundTrips) {
encoded_ptr.reset(encoded);
// Check the encoded bytes are the same as the test input.
ASSERT_EQ(output_bytes.size(), encoded_len);
ASSERT_EQ(memcmp(output_bytes.data(), encoded, encoded_len), 0);
ASSERT_EQ(OPENSSL_memcmp(output_bytes.data(), encoded, encoded_len), 0);
}

TEST(SSLTest, ZeroSizedWriteFlushesHandshakeMessages) {
Expand Down
33 changes: 9 additions & 24 deletions ssl/ssl_transfer_asn1.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,7 @@ static bool SSL3_STATE_get_optional_octet_string(CBS *cbs, void *dst,

enum SSL3_STATE_SERDE_VERSION {
SSL3_STATE_SERDE_VERSION_ONE = 1,
SSL3_STATE_SERDE_VERSION_TWO = 2,
SSL3_STATE_SERDE_VERSION_THREE = 3
SSL3_STATE_SERDE_VERSION_TWO = 2
};

static const unsigned kS3EstablishedSessionTag =
Expand Down Expand Up @@ -192,7 +191,7 @@ static int SSL3_STATE_to_bytes(SSL3_STATE *in, uint16_t protocol_version,

CBB s3, child, child2;
if (!CBB_add_asn1(cbb, &s3, CBS_ASN1_SEQUENCE) ||
!CBB_add_asn1_uint64(&s3, SSL3_STATE_SERDE_VERSION_THREE) ||
!CBB_add_asn1_uint64(&s3, SSL3_STATE_SERDE_VERSION_TWO) ||
!CBB_add_asn1_octet_string(&s3, in->read_sequence, TLS_SEQ_NUM_SIZE) ||
!CBB_add_asn1_octet_string(&s3, in->write_sequence, TLS_SEQ_NUM_SIZE) ||
!CBB_add_asn1_octet_string(&s3, in->server_random, SSL3_RANDOM_SIZE) ||
Expand Down Expand Up @@ -445,8 +444,6 @@ static int SSL3_STATE_parse_session(CBS *cbs, UniquePtr<SSL_SESSION> *out,
}
}

#define PRE_V3_PREV_FINISHED_MAX_SIZE 12

// SSL3_STATE_from_bytes recovers SSL3_STATE from |cbs|.
// |ssl| is used because |tls1_configure_aead| is used to recover
// |aead_read_ctx| and |aead_write_ctx|.
Expand All @@ -472,15 +469,9 @@ static int SSL3_STATE_from_bytes(SSL *ssl, CBS *cbs, const SSL_CTX *ctx) {
int pending_app_data_present, read_buffer_present;
if (!CBS_get_asn1(cbs, &s3, CBS_ASN1_SEQUENCE) ||
!CBS_get_asn1_uint64(&s3, &serde_version) ||
serde_version > SSL3_STATE_SERDE_VERSION_THREE ||
(is_tls13 && serde_version < SSL3_STATE_SERDE_VERSION_TWO)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_SERIALIZATION_INVALID_SSL3_STATE);
return 0;
}

bool is_pre_v3 = (serde_version < SSL3_STATE_SERDE_VERSION_THREE);

if (!CBS_get_asn1(&s3, &read_seq, CBS_ASN1_OCTETSTRING) ||
serde_version > SSL3_STATE_SERDE_VERSION_TWO ||
(is_tls13 && serde_version < SSL3_STATE_SERDE_VERSION_TWO) ||
!CBS_get_asn1(&s3, &read_seq, CBS_ASN1_OCTETSTRING) ||
CBS_len(&read_seq) != TLS_SEQ_NUM_SIZE ||
!CBS_get_asn1(&s3, &write_seq, CBS_ASN1_OCTETSTRING) ||
CBS_len(&write_seq) != TLS_SEQ_NUM_SIZE ||
Expand All @@ -494,17 +485,11 @@ static int SSL3_STATE_from_bytes(SSL *ssl, CBS *cbs, const SSL_CTX *ctx) {
!CBS_get_asn1_uint64(&s3, &early_data_reason) ||
early_data_reason > ssl_early_data_reason_max_value ||
!CBS_get_asn1(&s3, &previous_client_finished, CBS_ASN1_OCTETSTRING) ||
(is_pre_v3 &&
CBS_len(&previous_client_finished) != PRE_V3_PREV_FINISHED_MAX_SIZE) ||
(!is_pre_v3 &&
CBS_len(&previous_client_finished) != PREV_FINISHED_MAX_SIZE) ||
CBS_len(&previous_client_finished) > PREV_FINISHED_MAX_SIZE ||
!CBS_get_asn1_uint64(&s3, &previous_client_finished_len) ||
previous_client_finished_len > PREV_FINISHED_MAX_SIZE ||
!CBS_get_asn1(&s3, &previous_server_finished, CBS_ASN1_OCTETSTRING) ||
(is_pre_v3 &&
CBS_len(&previous_server_finished) != PRE_V3_PREV_FINISHED_MAX_SIZE) ||
(!is_pre_v3 &&
CBS_len(&previous_server_finished) != PREV_FINISHED_MAX_SIZE) ||
CBS_len(&previous_server_finished) > PREV_FINISHED_MAX_SIZE ||
!CBS_get_asn1_uint64(&s3, &previous_server_finished_len) ||
previous_server_finished_len > PREV_FINISHED_MAX_SIZE ||
!CBS_get_asn1_uint64(&s3, &empty_record_count) ||
Expand Down Expand Up @@ -536,11 +521,11 @@ static int SSL3_STATE_from_bytes(SSL *ssl, CBS *cbs, const SSL_CTX *ctx) {
return 0;
}

bool is_post_v2 = (serde_version >= SSL3_STATE_SERDE_VERSION_TWO);
bool is_v2 = (serde_version == SSL3_STATE_SERDE_VERSION_TWO);

// We should have no more data at this point if we are deserializing v1
// encoding.
if (!is_post_v2 && CBS_len(&s3) > 0) {
if (!is_v2 && CBS_len(&s3) > 0) {
OPENSSL_PUT_ERROR(SSL, SSL_R_SERIALIZATION_INVALID_SSL3_STATE);
return 0;
}
Expand Down

0 comments on commit f3b8dec

Please sign in to comment.