Fixes to low severity security issues as detailed in Security_Advisor…

…ies (https://github.com/awslabs/s2n/security/advisories):

 - Server denial-of-service via crafted handshake message
 - Online Certificate Stapling Protocol (OCSP) Revocation check bypass
 - Predictable IV in CBC-mode composite cipher suites

crypto/s2n_openssl_x509.h

@@ -20,4 +20,3 @@
 #include "utils/s2n_safety.h"
 
 DEFINE_POINTER_CLEANUP_FUNC(X509*, X509_free);
-DEFINE_POINTER_CLEANUP_FUNC(X509_STORE_CTX*, X509_STORE_CTX_free);

error/s2n_errno.c

@@ -139,6 +139,7 @@ static const char *no_such_error = "Internal s2n error";
     ERR_ENTRY(S2N_ERR_INITIAL_HMAC, "error calling EVP_CIPHER_CTX_ctrl for composite cbc cipher") \
     ERR_ENTRY(S2N_ERR_INVALID_NONCE_TYPE, "Invalid AEAD nonce type") \
     ERR_ENTRY(S2N_ERR_UNIMPLEMENTED, "Unimplemented feature") \
+    ERR_ENTRY(S2N_ERR_HANDSHAKE_UNREACHABLE, "Unreachable handshake state machine handler invoked") \
     ERR_ENTRY(S2N_ERR_READ, "error calling read") \
     ERR_ENTRY(S2N_ERR_WRITE, "error calling write") \
     ERR_ENTRY(S2N_ERR_BAD_FD, "Invalid file descriptor") \
@@ -234,6 +235,7 @@ static const char *no_such_error = "Internal s2n error";
     ERR_ENTRY(S2N_ERR_ASYNC_APPLY_WHILE_INVOKING, "Async private key operation cannot consumed inside async pkey callback") \
     ERR_ENTRY(S2N_ERR_ASYNC_ALREADY_APPLIED, "Async operation was already applied to connection, cannot apply it again") \
     ERR_ENTRY(S2N_ERR_INVALID_HELLO_RETRY, "Invalid hello retry request") \
+    ERR_ENTRY(S2N_ERR_INVALID_STATE, "Invalid state, this is the result of invalid use of an API. Check the API documentation for the function that raised this error for more info.") \
 
 /* clang-format on */
 

error/s2n_errno.h

@@ -169,6 +169,7 @@ typedef enum {
     S2N_ERR_INITIAL_HMAC,
     S2N_ERR_INVALID_NONCE_TYPE,
     S2N_ERR_UNIMPLEMENTED,
+    S2N_ERR_HANDSHAKE_UNREACHABLE,
     S2N_ERR_READ,
     S2N_ERR_WRITE,
     S2N_ERR_BAD_FD,
@@ -197,6 +198,7 @@ typedef enum {
     S2N_ERR_INVALID_PARSED_EXTENSIONS,
     S2N_ERR_ASYNC_CALLBACK_FAILED,
     S2N_ERR_ASYNC_MORE_THAN_ONE,
+    S2N_ERR_INVALID_STATE,
     S2N_ERR_T_INTERNAL_END,
 
     /* S2N_ERR_T_USAGE */

tests/pems/ocsp/certs_revoked.txt

@@ -0,0 +1,3 @@
+V	21170812053434Z		7777	unknown	/C=US/ST=WA/O=s2n/OU=s2n Test Cert/CN=www.s2ntest.com
+R	21170812053925Z	180812053925Z	7778	unknown	/C=US/ST=WA/O=s2n/CN=s2n Test Cert
+V	21170812054322Z		7779	unknown	/C=US/ST=WA/O=s2n/OU=s2n Test OCSP/CN=ocsp.s2ntest.com

tests/testlib/s2n_testlib.h

@@ -113,11 +113,15 @@ int s2n_unsafe_set_drbg_seed(const struct s2n_blob *seed);
 
 /* OCSP Stapled Response Testing files */
 #define S2N_OCSP_SERVER_CERT                   "../pems/ocsp/server_cert.pem"
+#define S2N_OCSP_SERVER_ECDSA_CERT             "../pems/ocsp/server_ecdsa_cert.pem"
+
 #define S2N_OCSP_SERVER_KEY                    "../pems/ocsp/server_key.pem"
 #define S2N_OCSP_CA_CERT                       "../pems/ocsp/ca_cert.pem"
 #define S2N_OCSP_CA_KEY                        "../pems/ocsp/ca_key.pem"
 #define S2N_OCSP_RESPONSE_DER                  "../pems/ocsp/ocsp_response.der"
 #define S2N_OCSP_RESPONSE_NO_NEXT_UPDATE_DER   "../pems/ocsp/ocsp_response_no_next_update.der"
+#define S2N_OCSP_RESPONSE_REVOKED_DER          "../pems/ocsp/ocsp_response_revoked.der"
+#define S2N_OCSP_RESPONSE_WRONG_SIGNER_DER     "../pems/ocsp/ocsp_response_wrong_signer.der"
 #define S2N_OCSP_RESPONSE_CERT                 "../pems/ocsp/ocsp_cert.pem"
 
 #define S2N_ALLIGATOR_SAN_CERT                 "../pems/sni/alligator_cert.pem"

tests/unit/s2n_aes_sha_composite_test.c

@@ -32,6 +32,25 @@
 #include "crypto/s2n_hmac.h"
 #include "crypto/s2n_hash.h"
 
+/* Explicit IV starts after the TLS record header. */
+#define EXPLICIT_IV_OFFSET S2N_TLS_RECORD_HEADER_LENGTH
+
+/* IVs should never be repeated with the same session key. */
+static int ensure_explicit_iv_is_unique(uint8_t existing_explicit_ivs[S2N_DEFAULT_FRAGMENT_LENGTH][S2N_TLS_MAX_IV_LEN],
+        size_t num_existing_ivs,
+        const uint8_t *candidate_iv,
+        uint16_t iv_len)
+{
+    for (size_t i = 0; i < num_existing_ivs; i++) {
+        if (memcmp(existing_explicit_ivs[i], candidate_iv, iv_len) == 0) {
+            return S2N_FAILURE;
+        }
+    }
+
+    return S2N_SUCCESS;
+}
+
+
 int main(int argc, char **argv)
 {
     struct s2n_connection *conn;
@@ -43,6 +62,8 @@ int main(int argc, char **argv)
     struct s2n_blob aes128 = {.data = aes128_key,.size = sizeof(aes128_key) };
     struct s2n_blob aes256 = {.data = aes256_key,.size = sizeof(aes256_key) };
     struct s2n_blob r = {.data = random_data, .size = sizeof(random_data)};
+    /* Stores explicit IVs used in each test case to validate uniqueness. */
+    uint8_t existing_explicit_ivs[S2N_DEFAULT_FRAGMENT_LENGTH + 2][S2N_TLS_MAX_IV_LEN];
 
     BEGIN_TEST();
 
@@ -112,7 +133,15 @@ int main(int argc, char **argv)
 
             /* The data should be encrypted */
             if (bytes_written > 10) {
-                EXPECT_NOT_EQUAL(memcmp(conn->out.blob.data + S2N_TLS_RECORD_HEADER_LENGTH, random_data, bytes_written), 0);
+                EXPECT_NOT_EQUAL(memcmp(conn->out.blob.data + EXPLICIT_IV_OFFSET + explicit_iv_len, random_data, bytes_written), 0);
+            }
+
+            if (explicit_iv_len > 0) {
+                /* The explicit IV for every record written should be random */
+                uint8_t *explicit_iv = conn->out.blob.data + EXPLICIT_IV_OFFSET;
+                EXPECT_SUCCESS(ensure_explicit_iv_is_unique(existing_explicit_ivs, i, explicit_iv, explicit_iv_len));
+                /* Record this IV */
+                memcpy(existing_explicit_ivs[i], explicit_iv, explicit_iv_len);
             }
 
             /* Copy the encrypted out data to the in data */
@@ -178,7 +207,15 @@ int main(int argc, char **argv)
 
             /* The data should be encrypted */
             if (bytes_written > 10) {
-                EXPECT_NOT_EQUAL(memcmp(conn->out.blob.data + S2N_TLS_RECORD_HEADER_LENGTH, random_data, bytes_written), 0);
+                EXPECT_NOT_EQUAL(memcmp(conn->out.blob.data + EXPLICIT_IV_OFFSET + explicit_iv_len, random_data, bytes_written), 0);
+            }
+
+            if (explicit_iv_len > 0) {
+                /* The explicit IV for every record written should be random */
+                uint8_t *explicit_iv = conn->out.blob.data + EXPLICIT_IV_OFFSET;
+                EXPECT_SUCCESS(ensure_explicit_iv_is_unique(existing_explicit_ivs, i, explicit_iv, explicit_iv_len));
+                /* Record this IV */
+                memcpy(existing_explicit_ivs[i], explicit_iv, explicit_iv_len);
             }
 
             /* Copy the encrypted out data to the in data */
@@ -245,7 +282,15 @@ int main(int argc, char **argv)
 
             /* The data should be encrypted */
             if (bytes_written > 10) {
-                EXPECT_NOT_EQUAL(memcmp(conn->out.blob.data + S2N_TLS_RECORD_HEADER_LENGTH, random_data, bytes_written), 0);
+                EXPECT_NOT_EQUAL(memcmp(conn->out.blob.data + EXPLICIT_IV_OFFSET + explicit_iv_len, random_data, bytes_written), 0);
+            }
+
+            if (explicit_iv_len > 0) {
+                /* The explicit IV for every record written should be random */
+                uint8_t *explicit_iv = conn->out.blob.data + EXPLICIT_IV_OFFSET;
+                EXPECT_SUCCESS(ensure_explicit_iv_is_unique(existing_explicit_ivs, i, explicit_iv, explicit_iv_len));
+                /* Record this IV */
+                memcpy(existing_explicit_ivs[i], explicit_iv, explicit_iv_len);
             }
 
             /* Copy the encrypted out data to the in data */
@@ -311,7 +356,15 @@ int main(int argc, char **argv)
 
             /* The data should be encrypted */
             if (bytes_written > 10) {
-                EXPECT_NOT_EQUAL(memcmp(conn->out.blob.data + S2N_TLS_RECORD_HEADER_LENGTH, random_data, bytes_written), 0);
+                EXPECT_NOT_EQUAL(memcmp(conn->out.blob.data + EXPLICIT_IV_OFFSET + explicit_iv_len, random_data, bytes_written), 0);
+            }
+
+            if (explicit_iv_len > 0) {
+                /* The explicit IV for every record written should be random */
+                uint8_t *explicit_iv = conn->out.blob.data + EXPLICIT_IV_OFFSET;
+                EXPECT_SUCCESS(ensure_explicit_iv_is_unique(existing_explicit_ivs, i, explicit_iv, explicit_iv_len));
+                /* Record this IV */
+                memcpy(existing_explicit_ivs[i], explicit_iv, explicit_iv_len);
             }
 
             /* Copy the encrypted out data to the in data */

tests/unit/s2n_handshake_invariant_test.c

@@ -0,0 +1,93 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file is distributed
+ * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+#include "s2n_test.h"
+
+#include "testlib/s2n_testlib.h"
+
+#include <unistd.h>
+#include <stdint.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <stdlib.h>
+
+#include <s2n.h>
+
+#include "crypto/s2n_fips.h"
+#include "crypto/s2n_rsa_pss.h"
+
+#include "tls/s2n_connection.h"
+#include "tls/s2n_handshake.h"
+#include "tls/s2n_cipher_preferences.h"
+#include "tls/s2n_cipher_suites.h"
+#include "tls/s2n_tls13.h"
+#include "utils/s2n_safety.h"
+#include "tls/s2n_handshake_io.c"
+
+/* A full record that has an "APPLICATION_DATA" inside according to s2n's
+ * handshake message_types.
+ */
+uint8_t record[] = {
+        /* Record type HANDSHAKE */
+        0x16,
+        /* Protocol version TLS 1.2 */
+        0x03, 0x03,
+        /* record len */
+        0x00, 0x05,
+        /* Type(s2n has this as expected message type for APPLICATION_DATA handler.
+         * This is not a standardized value, just something s2n has hardcoded as a placeholder
+         * For the APPLICATON_DATA state in the state machine.
+         */
+        0x00,
+        /* Len */
+        0x00, 0x00, 0x01,
+        /* Data */
+        0x00
+};
+static int amt_written = 0;
+
+int s2n_app_data_in_handshake_record_recv_fn(void *io_context, uint8_t *buf, uint32_t len)
+{
+    int amt_left = sizeof(record) - amt_written;
+    int to_write = MIN(len, amt_left);
+    memcpy(buf, record + amt_written, to_write);
+    amt_written += to_write;
+    return to_write;
+}
+
+int main(int argc, char **argv)
+{
+    BEGIN_TEST();
+
+    struct s2n_connection *conn;
+    EXPECT_NOT_NULL(conn = s2n_connection_new(S2N_SERVER));
+
+    /* Initialize *some* handshake type. Not terribly relevant for this test. */
+    conn->handshake.handshake_type = NEGOTIATED | FULL_HANDSHAKE | TLS12_PERFECT_FORWARD_SECRECY;
+    /* Fast forward the handshake state machine to the end of this "handshake_type".
+     * APPLICATION_DATA is the 11th state for "NEGOTIATED | FULL_HANDSHAKE | TLS12_PERFECT_FORWARD_SECRECY".
+     */
+    conn->handshake.message_number = 10;
+    conn->actual_protocol_version = S2N_TLS12;
+    /* Provide the crafted record to s2n's I/O */
+    s2n_connection_set_recv_cb(conn, s2n_app_data_in_handshake_record_recv_fn);
+
+    EXPECT_FAILURE_WITH_ERRNO(s2n_handshake_read_io(conn), S2N_ERR_BAD_MESSAGE);
+
+    s2n_connection_free(conn);
+    END_TEST();
+    return 0;
+}
+