Merge branch 'main' into fix-valgrind-errors

CMakeLists.txt

@@ -197,7 +197,12 @@ if(ASAN)
     target_link_options(${PROJECT_NAME} PUBLIC -fsanitize=address)
 endif()
 
-if(TSAN OR ASAN)
+if (UBSAN)
+    target_compile_options(${PROJECT_NAME} PUBLIC -fsanitize=undefined -fno-sanitize-recover=all)
+    target_link_options(${PROJECT_NAME} PUBLIC -fsanitize=undefined -fno-sanitize-recover=all)
+endif()
+
+if(TSAN OR ASAN OR UBSAN)
     # no-omit-frame-pointer and no-optimize-sibling-calls provide better stack traces
     target_compile_options(${PROJECT_NAME} PUBLIC -fno-omit-frame-pointer -fno-optimize-sibling-calls)
 endif()

bin/common.c

@@ -345,8 +345,8 @@ int s2n_set_common_server_config(int max_early_data, struct s2n_config *config,
 
     if (conn_settings.session_ticket || conn_settings.session_cache) {
         /* Key initialization */
-        uint8_t *st_key;
-        uint32_t st_key_length;
+        uint8_t *st_key = NULL;
+        uint32_t st_key_length = 0;
 
         if (session_ticket_key_file_path) {
             int fd = open(session_ticket_key_file_path, O_RDONLY);

bin/echo.c

@@ -139,10 +139,10 @@ int early_data_send(struct s2n_connection *conn, uint8_t *data, uint32_t len)
 
 int print_connection_info(struct s2n_connection *conn)
 {
-    int client_hello_version;
-    int client_protocol_version;
-    int server_protocol_version;
-    int actual_protocol_version;
+    int client_hello_version = 0;
+    int client_protocol_version = 0;
+    int server_protocol_version = 0;
+    int actual_protocol_version = 0;
 
     if ((client_hello_version = s2n_connection_get_client_hello_version(conn)) < 0) {
         fprintf(stderr, "Could not get client hello version\n");
@@ -179,7 +179,7 @@ int print_connection_info(struct s2n_connection *conn)
     printf("KEM: %s\n", s2n_connection_get_kem_name(conn));
     printf("KEM Group: %s\n", s2n_connection_get_kem_group_name(conn));
 
-    uint32_t length;
+    uint32_t length = 0;
     const uint8_t *status = s2n_connection_get_ocsp_response(conn, &length);
     if (status && length > 0) {
         printf("OCSP response received, length %u\n", length);

bin/s2nc.c

@@ -278,8 +278,8 @@ static void setup_s2n_config(struct s2n_config *config, const char *cipher_prefs
 
 int main(int argc, char *const *argv)
 {
-    struct addrinfo hints, *ai_list, *ai;
-    int r, sockfd = 0;
+    struct addrinfo hints, *ai_list = NULL, *ai = NULL;
+    int r = 0, sockfd = 0;
     bool session_ticket_recv = 0;
     /* Optional args */
     const char *alpn_protocols = NULL;

bin/s2nd.c

@@ -254,8 +254,8 @@ int handle_connection(int fd, struct s2n_config *config, struct conn_settings se
 
 int main(int argc, char *const *argv)
 {
-    struct addrinfo hints, *ai;
-    int r, sockfd = 0;
+    struct addrinfo hints, *ai = NULL;
+    int r = 0, sockfd = 0;
 
     /* required args */
     const char *host = NULL;
@@ -629,7 +629,7 @@ int main(int argc, char *const *argv)
                 "Failed to set key log callback");
     }
 
-    int fd;
+    int fd = 0;
     while ((fd = accept(sockfd, ai->ai_addr, &ai->ai_addrlen)) > 0) {
         if (non_blocking) {
             int flags = fcntl(sockfd, F_GETFL, 0);

crypto/s2n_dhe.c

@@ -34,7 +34,7 @@
  */
 static const BIGNUM *s2n_get_Ys_dh_param(struct s2n_dh_params *dh_params)
 {
-    const BIGNUM *Ys;
+    const BIGNUM *Ys = NULL;
 
 /* DH made opaque in Openssl 1.1.0 */
 #if S2N_OPENSSL_VERSION_AT_LEAST(1, 1, 0)
@@ -48,7 +48,7 @@ static const BIGNUM *s2n_get_Ys_dh_param(struct s2n_dh_params *dh_params)
 
 static const BIGNUM *s2n_get_p_dh_param(struct s2n_dh_params *dh_params)
 {
-    const BIGNUM *p;
+    const BIGNUM *p = NULL;
 #if S2N_OPENSSL_VERSION_AT_LEAST(1, 1, 0)
     DH_get0_pqg(dh_params->dh, &p, NULL, NULL);
 #else
@@ -60,7 +60,7 @@ static const BIGNUM *s2n_get_p_dh_param(struct s2n_dh_params *dh_params)
 
 static const BIGNUM *s2n_get_g_dh_param(struct s2n_dh_params *dh_params)
 {
-    const BIGNUM *g;
+    const BIGNUM *g = NULL;
 #if S2N_OPENSSL_VERSION_AT_LEAST(1, 1, 0)
     DH_get0_pqg(dh_params->dh, NULL, NULL, &g);
 #else

crypto/s2n_ecc_evp.c

@@ -23,6 +23,8 @@
 
 #include <stdint.h>
 
+#include "crypto/s2n_fips.h"
+#include "crypto/s2n_libcrypto.h"
 #include "tls/s2n_connection.h"
 #include "tls/s2n_ecc_preferences.h"
 #include "tls/s2n_tls_parameters.h"
@@ -118,6 +120,15 @@ int s2n_is_evp_apis_supported()
     return EVP_APIS_SUPPORTED;
 }
 
+bool s2n_ecc_evp_supports_fips_check()
+{
+#ifdef S2N_LIBCRYPTO_SUPPORTS_EC_KEY_CHECK_FIPS
+    return true;
+#else
+    return false;
+#endif
+}
+
 #if EVP_APIS_SUPPORTED
 static int s2n_ecc_evp_generate_key_x25519(const struct s2n_ecc_named_curve *named_curve, EVP_PKEY **evp_pkey)
 {
@@ -163,27 +174,50 @@ static int s2n_ecc_evp_generate_own_key(const struct s2n_ecc_named_curve *named_
     return named_curve->generate_key(named_curve, evp_pkey);
 }
 
+static S2N_RESULT s2n_ecc_check_key(EC_KEY *ec_key)
+{
+    RESULT_ENSURE_REF(ec_key);
+
+#ifdef S2N_LIBCRYPTO_SUPPORTS_EC_KEY_CHECK_FIPS
+    if (s2n_is_in_fips_mode()) {
+        RESULT_GUARD_OSSL(EC_KEY_check_fips(ec_key), S2N_ERR_ECDHE_INVALID_PUBLIC_KEY_FIPS);
+        return S2N_RESULT_OK;
+    }
+#endif
+
+    RESULT_GUARD_OSSL(EC_KEY_check_key(ec_key), S2N_ERR_ECDHE_INVALID_PUBLIC_KEY);
+
+    return S2N_RESULT_OK;
+}
+
 static int s2n_ecc_evp_compute_shared_secret(EVP_PKEY *own_key, EVP_PKEY *peer_public, uint16_t iana_id, struct s2n_blob *shared_secret)
 {
     POSIX_ENSURE_REF(peer_public);
     POSIX_ENSURE_REF(own_key);
 
-    /* From RFC 8446(TLS1.3) Section 4.2.8.2: For the curves secp256r1, secp384r1, and secp521r1, peers MUST validate 
-     * each other's public value Q by ensuring that the point is a valid point on the elliptic curve.
-     * For the curve x25519 and x448 the peer public-key validation check doesn't apply.
-     * From RFC 8422(TLS1.2) Section 5.11: With the NIST curves, each party MUST validate the public key sent by its peer
-     * in the ClientKeyExchange and ServerKeyExchange messages. A receiving party MUST check that the x and y parameters from 
-     * the peer's public value satisfy the curve equation, y^2 = x^3 + ax + b mod p.
-     * Note that the `EC_KEY_check_key` validation is a MUST for only NIST curves, if a non-NIST curve is added to s2n-tls 
-     * this is an additional validation step that increases security but decreases performance.
+    /**
+     *= https://tools.ietf.org/rfc/rfc8446#section-4.2.8.2
+     *# For the curves secp256r1, secp384r1, and secp521r1, peers MUST
+     *# validate each other's public value Q by ensuring that the point is a
+     *# valid point on the elliptic curve.
+     *
+     *= https://tools.ietf.org/rfc/rfc8422#section-5.11
+     *# With the NIST curves, each party MUST validate the public key sent by
+     *# its peer in the ClientKeyExchange and ServerKeyExchange messages.  A
+     *# receiving party MUST check that the x and y parameters from the
+     *# peer's public value satisfy the curve equation, y^2 = x^3 + ax + b
+     *# mod p.
+     *
+     * The validation requirement for the public key value only applies to NIST curves. The
+     * validation is skipped with non-NIST curves for increased performance.
      */
     if (iana_id != TLS_EC_CURVE_ECDH_X25519 && iana_id != TLS_EC_CURVE_ECDH_X448) {
         DEFER_CLEANUP(EC_KEY *ec_key = EVP_PKEY_get1_EC_KEY(peer_public), EC_KEY_free_pointer);
-        S2N_ERROR_IF(ec_key == NULL, S2N_ERR_ECDHE_UNSUPPORTED_CURVE);
-        POSIX_GUARD_OSSL(EC_KEY_check_key(ec_key), S2N_ERR_ECDHE_SHARED_SECRET);
+        POSIX_ENSURE(ec_key, S2N_ERR_ECDHE_UNSUPPORTED_CURVE);
+        POSIX_GUARD_RESULT(s2n_ecc_check_key(ec_key));
     }
 
-    size_t shared_secret_size;
+    size_t shared_secret_size = 0;
 
     DEFER_CLEANUP(EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new(own_key, NULL), EVP_PKEY_CTX_free_pointer);
     S2N_ERROR_IF(ctx == NULL, S2N_ERR_ECDHE_SHARED_SECRET);
@@ -233,7 +267,7 @@ int s2n_ecc_evp_compute_shared_secret_as_server(struct s2n_ecc_evp_params *ecc_e
     POSIX_ENSURE_REF(ecc_evp_params->evp_pkey);
     POSIX_ENSURE_REF(Yc_in);
 
-    uint8_t client_public_len;
+    uint8_t client_public_len = 0;
     struct s2n_blob client_public_blob = { 0 };
 
     DEFER_CLEANUP(EVP_PKEY *peer_key = EVP_PKEY_new(), EVP_PKEY_free_pointer);
@@ -345,8 +379,8 @@ int s2n_ecc_evp_read_params(struct s2n_stuffer *in, struct s2n_blob *data_to_ver
         struct s2n_ecdhe_raw_server_params *raw_server_ecc_params)
 {
     POSIX_ENSURE_REF(in);
-    uint8_t curve_type;
-    uint8_t point_length;
+    uint8_t curve_type = 0;
+    uint8_t point_length = 0;
 
     /* Remember where we started reading the data */
     data_to_verify->data = s2n_stuffer_raw_read(in, 0);
@@ -507,7 +541,7 @@ int s2n_ecc_evp_find_supported_curve(struct s2n_connection *conn, struct s2n_blo
     for (size_t i = 0; i < ecc_prefs->count; i++) {
         const struct s2n_ecc_named_curve *supported_curve = ecc_prefs->ecc_curves[i];
         for (uint32_t j = 0; j < iana_ids->size / 2; j++) {
-            uint16_t iana_id;
+            uint16_t iana_id = 0;
             POSIX_GUARD(s2n_stuffer_read_uint16(&iana_ids_in, &iana_id));
             if (supported_curve->iana_id == iana_id) {
                 *found = supported_curve;

crypto/s2n_ecc_evp.h

@@ -85,3 +85,4 @@ int s2n_ecc_evp_parse_params(struct s2n_connection *conn,
 int s2n_ecc_evp_find_supported_curve(struct s2n_connection *conn, struct s2n_blob *iana_ids, const struct s2n_ecc_named_curve **found);
 int s2n_ecc_evp_params_free(struct s2n_ecc_evp_params *ecc_evp_params);
 int s2n_is_evp_apis_supported();
+bool s2n_ecc_evp_supports_fips_check();

crypto/s2n_ecdsa.c

@@ -118,7 +118,7 @@ static int s2n_ecdsa_verify(const struct s2n_pkey *pub, s2n_signature_algorithm
     const s2n_ecdsa_public_key *key = &pub->key.ecdsa_key;
     POSIX_ENSURE_REF(key->ec_key);
 
-    uint8_t digest_length;
+    uint8_t digest_length = 0;
     POSIX_GUARD(s2n_hash_digest_size(digest->alg, &digest_length));
     POSIX_ENSURE_LTE(digest_length, S2N_MAX_DIGEST_LEN);
 

crypto/s2n_hash.c

@@ -621,7 +621,7 @@ int s2n_hash_const_time_get_currently_in_hash_block(struct s2n_hash_state *state
     POSIX_PRECONDITION(s2n_hash_state_validate(state));
     POSIX_ENSURE(S2N_MEM_IS_WRITABLE_CHECK(out, sizeof(*out)), S2N_ERR_PRECONDITION_VIOLATION);
     POSIX_ENSURE(state->is_ready_for_input, S2N_ERR_HASH_NOT_READY);
-    uint64_t hash_block_size;
+    uint64_t hash_block_size = 0;
     POSIX_GUARD(s2n_hash_block_size(state->alg, &hash_block_size));
 
     /* Requires that hash_block_size is a power of 2. This is true for all hashes we currently support

crypto/s2n_hkdf.c

@@ -76,7 +76,7 @@ static int s2n_custom_hkdf_expand(struct s2n_hmac_state *hmac, s2n_hmac_algorith
     POSIX_ENSURE(total_rounds <= MAX_HKDF_ROUNDS, S2N_ERR_HKDF_OUTPUT_SIZE);
 
     for (uint32_t curr_round = 1; curr_round <= total_rounds; curr_round++) {
-        uint32_t cat_len;
+        uint32_t cat_len = 0;
         POSIX_GUARD(s2n_hmac_init(hmac, alg, pseudo_rand_key->data, pseudo_rand_key->size));
         if (curr_round != 1) {
             POSIX_GUARD(s2n_hmac_update(hmac, prev, hash_len));

crypto/s2n_hmac.c

@@ -332,7 +332,7 @@ int s2n_hmac_reset(struct s2n_hmac_state *state)
     POSIX_ENSURE(state->hash_block_size != 0, S2N_ERR_PRECONDITION_VIOLATION);
     POSIX_GUARD(s2n_hash_copy(&state->inner, &state->inner_just_key));
 
-    uint64_t bytes_in_hash;
+    uint64_t bytes_in_hash = 0;
     POSIX_GUARD(s2n_hash_get_currently_in_hash_total(&state->inner, &bytes_in_hash));
     bytes_in_hash %= state->hash_block_size;
     POSIX_ENSURE(bytes_in_hash <= UINT32_MAX, S2N_ERR_INTEGER_OVERFLOW);

crypto/s2n_rsa_signing.c

@@ -96,8 +96,8 @@ int s2n_rsa_pkcs1v15_sign(const struct s2n_pkey *priv, struct s2n_hash_state *di
 
 int s2n_rsa_pkcs1v15_verify(const struct s2n_pkey *pub, struct s2n_hash_state *digest, struct s2n_blob *signature)
 {
-    uint8_t digest_length;
-    int digest_NID_type;
+    uint8_t digest_length = 0;
+    int digest_NID_type = 0;
     POSIX_GUARD(s2n_hash_digest_size(digest->alg, &digest_length));
     POSIX_GUARD(s2n_hash_NID_type(digest->alg, &digest_NID_type));
     POSIX_ENSURE_LTE(digest_length, S2N_MAX_DIGEST_LEN);
@@ -186,7 +186,7 @@ int s2n_rsa_pss_verify(const struct s2n_pkey *pub, struct s2n_hash_state *digest
 {
     POSIX_ENSURE_REF(pub);
 
-    uint8_t digest_length;
+    uint8_t digest_length = 0;
     uint8_t digest_data[S2N_MAX_DIGEST_LEN];
     POSIX_GUARD(s2n_hash_digest_size(digest->alg, &digest_length));
     POSIX_GUARD(s2n_hash_digest(digest, digest_data, digest_length));

error/s2n_errno.c

@@ -88,6 +88,8 @@ static const char *no_such_error = "Internal s2n error";
     ERR_ENTRY(S2N_ERR_ECDHE_GEN_KEY, "Failed to generate an ECDHE key") \
     ERR_ENTRY(S2N_ERR_ECDHE_SHARED_SECRET, "Error computing ECDHE shared secret") \
     ERR_ENTRY(S2N_ERR_ECDHE_UNSUPPORTED_CURVE, "Unsupported EC curve was presented during an ECDHE handshake") \
+    ERR_ENTRY(S2N_ERR_ECDHE_INVALID_PUBLIC_KEY, "Failed to validate the peer's point on the elliptic curve") \
+    ERR_ENTRY(S2N_ERR_ECDHE_INVALID_PUBLIC_KEY_FIPS, "Failed to validate the peer's point on the elliptic curve, per FIPS requirements") \
     ERR_ENTRY(S2N_ERR_ECDSA_UNSUPPORTED_CURVE, "Unsupported EC curve was presented during an ECDSA SignatureScheme handshake") \
     ERR_ENTRY(S2N_ERR_ECDHE_SERIALIZING, "Error serializing ECDHE public") \
     ERR_ENTRY(S2N_ERR_KEM_UNSUPPORTED_PARAMS, "Unsupported KEM params was presented during a handshake that uses a KEM") \

error/s2n_errno.h

@@ -103,6 +103,8 @@ typedef enum {
     S2N_ERR_ECDHE_GEN_KEY,
     S2N_ERR_ECDHE_SHARED_SECRET,
     S2N_ERR_ECDHE_UNSUPPORTED_CURVE,
+    S2N_ERR_ECDHE_INVALID_PUBLIC_KEY,
+    S2N_ERR_ECDHE_INVALID_PUBLIC_KEY_FIPS,
     S2N_ERR_ECDSA_UNSUPPORTED_CURVE,
     S2N_ERR_ECDHE_SERIALIZING,
     S2N_ERR_KEM_UNSUPPORTED_PARAMS,

stuffer/s2n_stuffer.c

@@ -432,3 +432,15 @@ int s2n_stuffer_extract_blob(struct s2n_stuffer *stuffer, struct s2n_blob *out)
     POSIX_POSTCONDITION(s2n_blob_validate(out));
     return S2N_SUCCESS;
 }
+
+int s2n_stuffer_shift(struct s2n_stuffer *stuffer)
+{
+    POSIX_ENSURE_REF(stuffer);
+    struct s2n_stuffer copy = *stuffer;
+    POSIX_GUARD(s2n_stuffer_rewrite(&copy));
+    uint8_t *data = stuffer->blob.data + stuffer->read_cursor;
+    uint32_t data_size = s2n_stuffer_data_available(stuffer);
+    POSIX_GUARD(s2n_stuffer_write_bytes(&copy, data, data_size));
+    *stuffer = copy;
+    return S2N_SUCCESS;
+}

stuffer/s2n_stuffer.h

@@ -82,6 +82,7 @@ int S2N_RESULT_MUST_USE s2n_stuffer_resize_if_empty(struct s2n_stuffer *stuffer,
 int S2N_RESULT_MUST_USE s2n_stuffer_rewind_read(struct s2n_stuffer *stuffer, const uint32_t size);
 int S2N_RESULT_MUST_USE s2n_stuffer_reread(struct s2n_stuffer *stuffer);
 int S2N_RESULT_MUST_USE s2n_stuffer_rewrite(struct s2n_stuffer *stuffer);
+int S2N_RESULT_MUST_USE s2n_stuffer_shift(struct s2n_stuffer *stuffer);
 int s2n_stuffer_wipe(struct s2n_stuffer *stuffer);
 int s2n_stuffer_wipe_n(struct s2n_stuffer *stuffer, const uint32_t n);
 bool s2n_stuffer_is_consumed(struct s2n_stuffer *stuffer);

tests/cbmc/proofs/s2n_stuffer_shift/Makefile

@@ -0,0 +1,41 @@
+#
+#
+# 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.
+
+# Expected runtime is 10 seconds.
+
+MAX_BLOB_SIZE = 20
+DEFINES += -DMAX_BLOB_SIZE=$(MAX_BLOB_SIZE)
+
+CBMCFLAGS +=
+
+PROOF_UID = s2n_stuffer_shift
+HARNESS_ENTRY = $(PROOF_UID)_harness
+HARNESS_FILE = $(HARNESS_ENTRY).c
+
+PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE)
+PROOF_SOURCES += $(PROOF_SOURCE)/cbmc_utils.c
+PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c
+PROOF_SOURCES += $(PROOF_STUB)/memmove_simple.c
+
+PROJECT_SOURCES += $(SRCDIR)/error/s2n_errno.c
+PROJECT_SOURCES += $(SRCDIR)/stuffer/s2n_stuffer.c
+PROJECT_SOURCES += $(SRCDIR)/utils/s2n_blob.c
+PROJECT_SOURCES += $(SRCDIR)/utils/s2n_ensure.c
+PROJECT_SOURCES += $(SRCDIR)/utils/s2n_mem.c
+PROJECT_SOURCES += $(SRCDIR)/utils/s2n_result.c
+PROJECT_SOURCES += $(SRCDIR)/utils/s2n_safety.c
+
+UNWINDSET += memmove_impl.0:$(call addone,$(MAX_BLOB_SIZE))
+UNWINDSET += memmove_impl.1:$(call addone,$(MAX_BLOB_SIZE))
+
+include ../Makefile.common

tests/cbmc/proofs/s2n_stuffer_shift/cbmc-proof.txt

@@ -0,0 +1 @@
+# This file marks this directory as containing a CBMC proof.