fix(sidetrail): Invalid stream cipher struct in proof wrapper

[goatgoose]

Description of changes:

s2n_record_parse_stream calls the decrypt function pointer in the io union of s2n_cipher:

s2n-tls/tls/s2n_record_read_stream.c

Line 53 in 38d769c

POSIX_GUARD(cipher_suite->record_alg->cipher->io.stream.decrypt(session_key, &en, &en));

In the sidetrail proof wrapper for s2n_record_parse_stream, the s2n_cipher struct is set up with the cbc union value rather than the stream value:

s2n-tls/tests/sidetrail/working/s2n-record-read-stream/s2n_record_read_wrapper.c

Lines 100 to 103 in ba825b8

	struct s2n_cipher stream_cipher = {
	.type = S2N_STREAM,
	.io.cbc.decrypt = decrypt_stream,
	};

This causes the sidetrail proof to finish successfully with no timing differences after attempting to decrypt, since this call always fails. This PR sets the decrypt function to the correct io struct, allowing the proof to explore the rest of the s2n_record_parse_stream function.

Call-outs:

Fixing this bug caused the proof to detect a timing difference between the mac validation failing and the mac validation succeeding. Given that the length of any private fields are declassified after successfully validating the mac, this PR removes the extra logic after mac validation from the scope of SideTrail's analysis to avoid the false positive error.

Testing:

I tested the proof with the following patches to ensure that valid sidechannels are successfully detected by sidetrail:

diff --git a/tls/s2n_record_read_stream.c b/tls/s2n_record_read_stream.c
index f40621b..4aa7236 100644
--- a/tls/s2n_record_read_stream.c
+++ b/tls/s2n_record_read_stream.c
@@ -75,6 +75,10 @@ int s2n_record_parse_stream(
     POSIX_ENSURE_LTE(mac_digest_size, sizeof(check_digest));
     POSIX_GUARD(s2n_hmac_digest(mac, check_digest, mac_digest_size));
 
+    if ((en.data + payload_length)[1] != check_digest[1]) {
+        POSIX_BAIL(S2N_ERR_BAD_MESSAGE);
+    }
+
     if (s2n_hmac_digest_verify(en.data + payload_length, check_digest, mac_digest_size) < 0) {
         POSIX_GUARD(s2n_stuffer_wipe(&conn->in));
         POSIX_BAIL(S2N_ERR_BAD_MESSAGE);

diff --git a/utils/s2n_safety.c b/utils/s2n_safety.c
index 6b2457b..b04503a 100644
--- a/utils/s2n_safety.c
+++ b/utils/s2n_safety.c
@@ -45,6 +45,10 @@ bool s2n_constant_time_equals(const uint8_t *a, const uint8_t *b, const uint32_t
         return true;
     }
 
+    if (a[1] != b[1]) {
+        return false;
+    }
+
     /* check if a and b are readable - if so, allow them to increment their pointer */
     uint8_t a_inc = S2N_MEM_IS_READABLE(a, len) ? 1 : 0;
     uint8_t b_inc = S2N_MEM_IS_READABLE(b, len) ? 1 : 0;

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.