Enforce that ENSURE and GUARD_OSSL use valid error codes (#3873)

codebuild/bin/grep_simple_mistakes.sh

@@ -196,10 +196,24 @@ S2N_UNNECESSARY_EXTERNS=$(find "$PWD" -type f -name "s2n*.[h]" \! -path "*/api/*
   -exec grep -RE "extern (.*?) (.*?)\(" {} +)
 if [[ -n $S2N_UNNECESSARY_EXTERNS ]]; then
   FAILED=1
-  echo "Found unnecessary 'extern' in function declaration"
-  echo "$S2N_UNNECESSARY_EXTERNS"
+  printf "\e[1;34mFound unnecessary 'extern' in function declaration:\e[0m\n"
+  printf "$S2N_UNNECESSARY_EXTERNS\n\n"
 fi
 
+#############################################
+# Assert ENSURE/GUARD have a valid error code
+#############################################
+S2N_ENSURE_WITH_INVALID_ERROR_CODE=$(find "$PWD" -type f -name "s2n*.c" -path "*" \! -path "*/tests/*" \
+    -exec grep -RE "(ENSURE|GUARD_OSSL)\(.*?, .*?);" {} + | grep -v "S2N_ERR_")
+if [[ -n $S2N_ENSURE_WITH_INVALID_ERROR_CODE ]]; then
+  FAILED=1
+  printf "\e[1;34mENSURE and GUARD_OSSL require a valid error code from errors/s2n_errno.h:\e[0m\n"
+  printf "$S2N_ENSURE_WITH_INVALID_ERROR_CODE\n\n"
+fi
+
+#############################################
+# REPORT FINAL RESULTS
+#############################################
 if [ $FAILED == 1 ]; then
   printf "\\033[31;1mFAILED Grep For Simple Mistakes check\\033[0m\\n"
   exit -1

crypto/s2n_rsa_pss.c

@@ -239,17 +239,17 @@ int s2n_rsa_pss_pkey_init(struct s2n_pkey *pkey)
 
 int s2n_evp_pkey_to_rsa_pss_public_key(struct s2n_rsa_key *rsa_pss_key, EVP_PKEY *pkey)
 {
-    POSIX_BAIL(S2N_RSA_PSS_NOT_SUPPORTED);
+    POSIX_BAIL(S2N_ERR_RSA_PSS_NOT_SUPPORTED);
 }
 
 int s2n_evp_pkey_to_rsa_pss_private_key(struct s2n_rsa_key *rsa_pss_key, EVP_PKEY *pkey)
 {
-    POSIX_BAIL(S2N_RSA_PSS_NOT_SUPPORTED);
+    POSIX_BAIL(S2N_ERR_RSA_PSS_NOT_SUPPORTED);
 }
 
 int s2n_rsa_pss_pkey_init(struct s2n_pkey *pkey)
 {
-    POSIX_BAIL(S2N_RSA_PSS_NOT_SUPPORTED);
+    POSIX_BAIL(S2N_ERR_RSA_PSS_NOT_SUPPORTED);
 }
 
 #endif

crypto/s2n_rsa_signing.c

@@ -213,17 +213,17 @@ int s2n_rsa_pss_verify(const struct s2n_pkey *pub, struct s2n_hash_state *digest
 int s2n_rsa_pss_sign_digest(const struct s2n_pkey *priv, s2n_hash_algorithm hash_alg,
         struct s2n_blob *digest_in, struct s2n_blob *signature_out)
 {
-    POSIX_BAIL(S2N_RSA_PSS_NOT_SUPPORTED);
+    POSIX_BAIL(S2N_ERR_RSA_PSS_NOT_SUPPORTED);
 }
 
 int s2n_rsa_pss_sign(const struct s2n_pkey *priv, struct s2n_hash_state *digest, struct s2n_blob *signature_out)
 {
-    POSIX_BAIL(S2N_RSA_PSS_NOT_SUPPORTED);
+    POSIX_BAIL(S2N_ERR_RSA_PSS_NOT_SUPPORTED);
 }
 
 int s2n_rsa_pss_verify(const struct s2n_pkey *pub, struct s2n_hash_state *digest, struct s2n_blob *signature_in)
 {
-    POSIX_BAIL(S2N_RSA_PSS_NOT_SUPPORTED);
+    POSIX_BAIL(S2N_ERR_RSA_PSS_NOT_SUPPORTED);
 }
 
 #endif

error/s2n_errno.c

@@ -237,7 +237,7 @@ static const char *no_such_error = "Internal s2n error";
     ERR_ENTRY(S2N_ERR_SESSION_TICKET_NOT_SUPPORTED, "Session ticket not supported for this connection") \
     ERR_ENTRY(S2N_ERR_OCSP_NOT_SUPPORTED, "OCSP stapling was requested, but is not supported") \
     ERR_ENTRY(S2N_ERR_INVALID_SIGNATURE_ALGORITHMS_PREFERENCES, "Invalid signature algorithms preferences version") \
-    ERR_ENTRY(S2N_RSA_PSS_NOT_SUPPORTED, "RSA-PSS signing not supported by underlying libcrypto implementation") \
+    ERR_ENTRY(S2N_ERR_RSA_PSS_NOT_SUPPORTED, "RSA-PSS signing not supported by underlying libcrypto implementation") \
     ERR_ENTRY(S2N_ERR_MAX_INNER_PLAINTEXT_SIZE, "Inner plaintext size exceeds limit") \
     ERR_ENTRY(S2N_ERR_INVALID_ECC_PREFERENCES, "Invalid ecc curves preferences version") \
     ERR_ENTRY(S2N_ERR_RECORD_STUFFER_SIZE, "Record stuffer out of space") \

error/s2n_errno.h

@@ -278,7 +278,7 @@ typedef enum {
     S2N_ERR_SESSION_TICKET_NOT_SUPPORTED,
     S2N_ERR_OCSP_NOT_SUPPORTED,
     S2N_ERR_INVALID_SIGNATURE_ALGORITHMS_PREFERENCES,
-    S2N_RSA_PSS_NOT_SUPPORTED,
+    S2N_ERR_RSA_PSS_NOT_SUPPORTED,
     S2N_ERR_INVALID_ECC_PREFERENCES,
     S2N_ERR_INVALID_SECURITY_POLICY,
     S2N_ERR_INVALID_KEM_PREFERENCES,

pq-crypto/s2n_kyber_512_evp.c

@@ -26,15 +26,15 @@
 int s2n_kyber_512_evp_generate_keypair(uint8_t *public_key, uint8_t *private_key) {
     EVP_PKEY_CTX *kyber_pkey_ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_KYBER512, NULL);
     POSIX_GUARD_PTR(kyber_pkey_ctx);
-    POSIX_ENSURE(EVP_PKEY_keygen_init(kyber_pkey_ctx), S2N_FAILURE);
+    POSIX_GUARD_OSSL(EVP_PKEY_keygen_init(kyber_pkey_ctx), S2N_ERR_PQ_CRYPTO);
 
     EVP_PKEY *kyber_pkey = NULL;
-    POSIX_ENSURE(EVP_PKEY_keygen(kyber_pkey_ctx, &kyber_pkey), S2N_FAILURE);
+    POSIX_GUARD_OSSL(EVP_PKEY_keygen(kyber_pkey_ctx, &kyber_pkey), S2N_ERR_PQ_CRYPTO);
 
     size_t public_key_size = S2N_KYBER_512_R3_PUBLIC_KEY_BYTES;
     size_t private_key_size = S2N_KYBER_512_R3_SECRET_KEY_BYTES;
-    POSIX_ENSURE(EVP_PKEY_get_raw_public_key(kyber_pkey, public_key, &public_key_size), S2N_FAILURE);
-    POSIX_ENSURE(EVP_PKEY_get_raw_private_key(kyber_pkey, private_key, &private_key_size), S2N_FAILURE);
+    POSIX_GUARD_OSSL(EVP_PKEY_get_raw_public_key(kyber_pkey, public_key, &public_key_size), S2N_ERR_PQ_CRYPTO);
+    POSIX_GUARD_OSSL(EVP_PKEY_get_raw_private_key(kyber_pkey, private_key, &private_key_size), S2N_ERR_PQ_CRYPTO);
 
     return S2N_SUCCESS;
 }
@@ -50,8 +50,8 @@ int s2n_kyber_512_evp_encapsulate(uint8_t *ciphertext, uint8_t *shared_secret,
 
     size_t cipher_text_size = S2N_KYBER_512_R3_CIPHERTEXT_BYTES;
     size_t shared_secret_size = S2N_KYBER_512_R3_SHARED_SECRET_BYTES;
-    POSIX_ENSURE(EVP_PKEY_encapsulate(kyber_pkey_ctx, ciphertext, &cipher_text_size, shared_secret,
-                                      &shared_secret_size), S2N_FAILURE);
+    POSIX_GUARD_OSSL(EVP_PKEY_encapsulate(kyber_pkey_ctx, ciphertext, &cipher_text_size, shared_secret,
+                                      &shared_secret_size), S2N_ERR_PQ_CRYPTO);
     return S2N_SUCCESS;
 }
 
@@ -65,8 +65,8 @@ int s2n_kyber_512_evp_decapsulate(uint8_t *shared_secret, const uint8_t *ciphert
     POSIX_GUARD_PTR(kyber_pkey_ctx);
 
     size_t shared_secret_size = S2N_KYBER_512_R3_SHARED_SECRET_BYTES;
-    POSIX_ENSURE(EVP_PKEY_decapsulate(kyber_pkey_ctx, shared_secret, &shared_secret_size, (uint8_t *) ciphertext,
-                                      S2N_KYBER_512_R3_CIPHERTEXT_BYTES), S2N_FAILURE);
+    POSIX_GUARD_OSSL(EVP_PKEY_decapsulate(kyber_pkey_ctx, shared_secret, &shared_secret_size, (uint8_t *) ciphertext,
+                                      S2N_KYBER_512_R3_CIPHERTEXT_BYTES), S2N_ERR_PQ_CRYPTO);
     return S2N_SUCCESS;
 }
 #else

stuffer/s2n_stuffer.c

@@ -360,11 +360,11 @@ int s2n_stuffer_writev_bytes(struct s2n_stuffer *stuffer, const struct iovec *io
             continue;
         }
         size_t iov_len_op = iov[i].iov_len - to_skip;
-        POSIX_ENSURE(iov_len_op <= UINT32_MAX, S2N_FAILURE);
+        POSIX_ENSURE_LTE(iov_len_op, UINT32_MAX);
         uint32_t iov_len = (uint32_t) iov_len_op;
         uint32_t iov_size_to_take = MIN(size_left, iov_len);
         POSIX_ENSURE_REF(iov[i].iov_base);
-        POSIX_ENSURE(to_skip < iov[i].iov_len, S2N_FAILURE);
+        POSIX_ENSURE_LT(to_skip, iov[i].iov_len);
         POSIX_CHECKED_MEMCPY(ptr, ((uint8_t *) (iov[i].iov_base)) + to_skip, iov_size_to_take);
         size_left -= iov_size_to_take;
         if (size_left == 0) {

stuffer/s2n_stuffer_file.c

@@ -80,14 +80,14 @@ int s2n_stuffer_alloc_ro_from_fd(struct s2n_stuffer *stuffer, int rfd)
 
     POSIX_ENSURE(fstat(rfd, &st) >= 0, S2N_ERR_FSTAT);
 
-    POSIX_ENSURE(st.st_size > 0, S2N_FAILURE);
-    POSIX_ENSURE(st.st_size <= UINT32_MAX, S2N_FAILURE);
+    POSIX_ENSURE_GT(st.st_size, 0);
+    POSIX_ENSURE_LTE(st.st_size, UINT32_MAX);
 
     uint8_t *map = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, rfd, 0);
     POSIX_ENSURE(map != MAP_FAILED, S2N_ERR_MMAP);
 
     struct s2n_blob b = { 0 };
-    POSIX_ENSURE(s2n_blob_init(&b, map, (uint32_t) st.st_size), S2N_FAILURE);
+    POSIX_GUARD(s2n_blob_init(&b, map, (uint32_t) st.st_size));
     return s2n_stuffer_init(stuffer, &b);
 }
 

tests/unit/s2n_quic_support_test.c

@@ -39,10 +39,10 @@ int main(int argc, char **argv)
         struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT);
         EXPECT_NOT_NULL(conn);
 
-        EXPECT_FAILURE_WITH_ERRNO(s2n_connection_enable_quic(conn), S2N_RSA_PSS_NOT_SUPPORTED);
+        EXPECT_FAILURE_WITH_ERRNO(s2n_connection_enable_quic(conn), S2N_ERR_RSA_PSS_NOT_SUPPORTED);
         EXPECT_FALSE(conn->quic_enabled);
 
-        EXPECT_FAILURE_WITH_ERRNO(s2n_connection_set_config(conn, config), S2N_RSA_PSS_NOT_SUPPORTED);
+        EXPECT_FAILURE_WITH_ERRNO(s2n_connection_set_config(conn, config), S2N_ERR_RSA_PSS_NOT_SUPPORTED);
         EXPECT_NOT_EQUAL(config, conn->config);
 
         EXPECT_SUCCESS(s2n_connection_free(conn));

tests/unit/s2n_rsa_pss_rsae_test.c

@@ -185,9 +185,9 @@ int main(int argc, char **argv)
 
         if (!s2n_is_rsa_pss_signing_supported()) {
             EXPECT_FAILURE_WITH_ERRNO(rsa_public_key.sign(rsa_cert_chain->private_key, S2N_SIGNATURE_RSA_PSS_RSAE, &sign_hash, &result),
-                    S2N_RSA_PSS_NOT_SUPPORTED);
+                    S2N_ERR_RSA_PSS_NOT_SUPPORTED);
             EXPECT_FAILURE_WITH_ERRNO(rsa_public_key.verify(&rsa_public_key, S2N_SIGNATURE_RSA_PSS_RSAE, &verify_hash, &result),
-                    S2N_RSA_PSS_NOT_SUPPORTED);
+                    S2N_ERR_RSA_PSS_NOT_SUPPORTED);
         } else {
             EXPECT_SUCCESS(rsa_public_key.sign(rsa_cert_chain->private_key, S2N_SIGNATURE_RSA_PSS_RSAE, &sign_hash, &result));
             EXPECT_SUCCESS(rsa_public_key.verify(&rsa_public_key, S2N_SIGNATURE_RSA_PSS_RSAE, &verify_hash, &result));

tls/s2n_tls13.c

@@ -128,19 +128,19 @@ S2N_RESULT s2n_connection_validate_tls13_support(struct s2n_connection *conn)
      * So a server might choose to use RSA-PSS even if even if the client does not advertise support for RSA-PSS.
      * Therefore, only servers can perform TLS1.3 without full feature support.
      */
-    RESULT_ENSURE(conn->mode == S2N_SERVER, S2N_RSA_PSS_NOT_SUPPORTED);
+    RESULT_ENSURE(conn->mode == S2N_SERVER, S2N_ERR_RSA_PSS_NOT_SUPPORTED);
 
     /* RSA signatures must use RSA-PSS in TLS1.3.
      * So RSA-PSS is required for TLS1.3 servers if an RSA certificate is used.
      */
-    RESULT_ENSURE(!conn->config->is_rsa_cert_configured, S2N_RSA_PSS_NOT_SUPPORTED);
+    RESULT_ENSURE(!conn->config->is_rsa_cert_configured, S2N_ERR_RSA_PSS_NOT_SUPPORTED);
 
     /* RSA-PSS is also required for TLS1.3 servers if client auth is requested, because the
      * client might offer an RSA certificate.
      */
     s2n_cert_auth_type client_auth_status = S2N_CERT_AUTH_NONE;
     RESULT_GUARD_POSIX(s2n_connection_get_client_auth_type(conn, &client_auth_status));
-    RESULT_ENSURE(client_auth_status == S2N_CERT_AUTH_NONE, S2N_RSA_PSS_NOT_SUPPORTED);
+    RESULT_ENSURE(client_auth_status == S2N_CERT_AUTH_NONE, S2N_ERR_RSA_PSS_NOT_SUPPORTED);
 
     return S2N_RESULT_OK;
 }

utils/s2n_mem.c

@@ -48,10 +48,10 @@ static int s2n_mem_init_impl(void)
     long sysconf_rc = sysconf(_SC_PAGESIZE);
 
     /* sysconf must not error, and page_size cannot be 0 */
-    POSIX_ENSURE(sysconf_rc > 0, S2N_FAILURE);
+    POSIX_ENSURE_GT(sysconf_rc, 0);
 
     /* page_size must be a valid uint32 */
-    POSIX_ENSURE(sysconf_rc <= UINT32_MAX, S2N_FAILURE);
+    POSIX_ENSURE_LTE(sysconf_rc, UINT32_MAX);
 
     page_size = (uint32_t) sysconf_rc;