Fix C++98 compatibility in our header files (aws#2193)

We encountered this when trying to compile Xtrabackup on gcc versions
lower than 10. Apparently scoped enums are a C++11 feature, which the
compiler can't understand without additional linker flags.

We also encountered this in 42a8dba, which prompted us to move it to
`ssl.h`. Upon closer examination, I don't think the C++11 usage is a
necessity especially with the pure C fallback that was included.
Predeclaring enums reduces compilation dependencies and build times,
but our build seems to work fine without it. Removing this would let us
encounter less build issues with projects that aren't expecting C++ in our
header files.

Asides from the issue with scoped enums mentioned above, we decided
to check for C++98 compatibility in CI and fix any potential issues with
`-Wpedantic` while we're at it. 

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

include/openssl/aead.h

@@ -437,7 +437,7 @@ OPENSSL_EXPORT const EVP_AEAD *EVP_aead_aes_256_gcm_tls13(void);
 // evp_aead_direction_t denotes the direction of an AEAD operation.
 enum evp_aead_direction_t {
   evp_aead_open,
-  evp_aead_seal,
+  evp_aead_seal
 };
 
 // EVP_AEAD_CTX_init_with_direction calls |EVP_AEAD_CTX_init| for normal

include/openssl/bn.h

@@ -769,7 +769,7 @@ OPENSSL_EXPORT int BN_generate_prime_ex(BIGNUM *ret, int bits, int safe,
 enum bn_primality_result_t {
   bn_probably_prime,
   bn_composite,
-  bn_non_prime_power_composite,
+  bn_non_prime_power_composite
 };
 
 // BN_enhanced_miller_rabin_primality_test tests whether |w| is probably a prime

include/openssl/crypto.h

@@ -130,7 +130,7 @@ enum fips_counter_t {
   fips_counter_evp_aes_128_ctr = 2,
   fips_counter_evp_aes_256_ctr = 3,
 
-  fips_counter_max = 3,
+  fips_counter_max = 3
 };
 
 // FIPS_read_counter returns a counter of the number of times the specific

include/openssl/curve25519.h

@@ -178,7 +178,7 @@ OPENSSL_EXPORT void ED25519_keypair_from_seed(uint8_t out_public_key[ED25519_PUB
 // must be “Alice” and the other be “Bob”.
 enum spake2_role_t {
   spake2_role_alice,
-  spake2_role_bob,
+  spake2_role_bob
 };
 
 // SPAKE2_CTX_new creates a new |SPAKE2_CTX| (which can only be used for a

include/openssl/ec.h

@@ -92,7 +92,7 @@ typedef enum {
 
   // POINT_CONVERSION_HYBRID indicates that the point is encoded as z||x||y,
   // where z specifies which solution of the quadratic equation y is.
-  POINT_CONVERSION_HYBRID = 6,
+  POINT_CONVERSION_HYBRID = 6
 } point_conversion_form_t;
 
 

include/openssl/service_indicator.h

@@ -32,7 +32,7 @@ OPENSSL_EXPORT const char* awslc_version_string(void);
 
 enum FIPSStatus {
   AWSLC_NOT_APPROVED = 0,
-  AWSLC_APPROVED = 1,
+  AWSLC_APPROVED = 1
 };
 
 #if defined(AWSLC_FIPS)

include/openssl/ssl.h

@@ -166,24 +166,6 @@ struct timeval;
 extern "C" {
 #endif
 
-#if defined(__cplusplus)
-// enums can be predeclared, but only in C++ and only if given an explicit type.
-// C doesn't support setting an explicit type for enums thus a #define is used
-// to do this only for C++. However, the ABI type between C and C++ need to have
-// equal sizes, which is confirmed in a unittest.
-#define BORINGSSL_ENUM_INT : int
-enum ssl_early_data_reason_t BORINGSSL_ENUM_INT;
-enum ssl_encryption_level_t BORINGSSL_ENUM_INT;
-enum ssl_private_key_result_t BORINGSSL_ENUM_INT;
-enum ssl_renegotiate_mode_t BORINGSSL_ENUM_INT;
-enum ssl_select_cert_result_t BORINGSSL_ENUM_INT;
-enum ssl_select_cert_result_t BORINGSSL_ENUM_INT;
-enum ssl_ticket_aead_result_t BORINGSSL_ENUM_INT;
-enum ssl_verify_result_t BORINGSSL_ENUM_INT;
-#else
-#define BORINGSSL_ENUM_INT
-#endif
-
 
 // SSL implementation.
 
@@ -1276,8 +1258,7 @@ OPENSSL_EXPORT int SSL_set_chain_and_key(
     SSL *ssl, CRYPTO_BUFFER *const *certs, size_t num_certs, EVP_PKEY *privkey,
     const SSL_PRIVATE_KEY_METHOD *privkey_method);
 
-
-// SSL_get0_chain returns the list of |CRYPTO_BUFFER|s that were set by
+// SSL_CTX_get0_chain returns the list of |CRYPTO_BUFFER|s that were set by
 // |SSL_set_chain_and_key|, unless they have been discarded. Reference counts
 // are not incremented by this call. The return value may be |NULL| if no chain
 // has been set.
@@ -1356,7 +1337,7 @@ OPENSSL_EXPORT int SSL_use_PrivateKey_file(SSL *ssl, const char *file,
 OPENSSL_EXPORT int SSL_CTX_use_certificate_chain_file(SSL_CTX *ctx,
                                                       const char *file);
 
-// SSL_CTX_use_certificate_chain_file configures certificates for |ssl|. It
+// SSL_use_certificate_chain_file configures certificates for |ssl|. It
 // reads the contents of |file| as a PEM-encoded leaf certificate followed
 // optionally by the certificate chain to send to the peer. It returns one on
 // success and zero on failure.
@@ -1384,10 +1365,10 @@ OPENSSL_EXPORT void *SSL_CTX_get_default_passwd_cb_userdata(const SSL_CTX *ctx);
 
 // Custom private keys.
 
-enum ssl_private_key_result_t BORINGSSL_ENUM_INT {
+enum ssl_private_key_result_t {
   ssl_private_key_success,
   ssl_private_key_retry,
-  ssl_private_key_failure,
+  ssl_private_key_failure
 };
 
 // ssl_private_key_method_st (aka |SSL_PRIVATE_KEY_METHOD|) describes private
@@ -2563,7 +2544,7 @@ OPENSSL_EXPORT int SSL_CTX_set_tlsext_ticket_key_cb(
 
 // ssl_ticket_aead_result_t enumerates the possible results from decrypting a
 // ticket with an |SSL_TICKET_AEAD_METHOD|.
-enum ssl_ticket_aead_result_t BORINGSSL_ENUM_INT {
+enum ssl_ticket_aead_result_t {
   // ssl_ticket_aead_success indicates that the ticket was successfully
   // decrypted.
   ssl_ticket_aead_success,
@@ -2576,7 +2557,7 @@ enum ssl_ticket_aead_result_t BORINGSSL_ENUM_INT {
   ssl_ticket_aead_ignore_ticket,
   // ssl_ticket_aead_error indicates that a fatal error occured and the
   // handshake should be terminated.
-  ssl_ticket_aead_error,
+  ssl_ticket_aead_error
 };
 
 // ssl_ticket_aead_method_st (aka |SSL_TICKET_AEAD_METHOD|) contains methods
@@ -2689,23 +2670,30 @@ OPENSSL_EXPORT int SSL_set1_groups_list(SSL *ssl, const char *groups);
 #define SSL_GROUP_SECP521R1 25
 #define SSL_GROUP_X25519 29
 
+// SSL_GROUP_SECP256R1_KYBER768_DRAFT00 is defined at
 // https://datatracker.ietf.org/doc/html/draft-kwiatkowski-tls-ecdhe-kyber
 #define SSL_GROUP_SECP256R1_KYBER768_DRAFT00 0x639A
 
+// SSL_GROUP_X25519_KYBER768_DRAFT00 is defined at
 // https://datatracker.ietf.org/doc/html/draft-tls-westerbaan-xyber768d00
 #define SSL_GROUP_X25519_KYBER768_DRAFT00 0x6399
 
+// SSL_GROUP_SECP256R1_MLKEM768 is defined at
 // https://datatracker.ietf.org/doc/html/draft-kwiatkowski-tls-ecdhe-mlkem.html
 #define SSL_GROUP_SECP256R1_MLKEM768 0x11EB
+
+// SSL_GROUP_X25519_MLKEM768 is defined at
+// https://datatracker.ietf.org/doc/html/draft-kwiatkowski-tls-ecdhe-mlkem.html
 #define SSL_GROUP_X25519_MLKEM768    0x11EC
 
-// PQ and hybrid group IDs are not yet standardized. Current IDs are driven by
-// community consensus and are defined at
+// The following PQ and hybrid group IDs are not yet standardized. Current IDs
+// are driven by community consensus and are defined at:
 // https://github.com/open-quantum-safe/oqs-provider/blob/main/oqs-template/oqs-kem-info.md
 #define SSL_GROUP_KYBER512_R3 0x023A
 #define SSL_GROUP_KYBER768_R3 0x023C
 #define SSL_GROUP_KYBER1024_R3 0x023D
 
+// The following are defined at
 // https://datatracker.ietf.org/doc/html/draft-connolly-tls-mlkem-key-agreement.html
 #define SSL_GROUP_MLKEM768  0x0768
 #define SSL_GROUP_MLKEM1024 0x1024
@@ -2904,10 +2892,10 @@ OPENSSL_EXPORT void SSL_set_verify(SSL *ssl, int mode,
                                    int (*callback)(int ok,
                                                    X509_STORE_CTX *store_ctx));
 
-enum ssl_verify_result_t BORINGSSL_ENUM_INT {
+enum ssl_verify_result_t {
   ssl_verify_ok,
   ssl_verify_invalid,
-  ssl_verify_retry,
+  ssl_verify_retry
 };
 
 // SSL_CTX_set_custom_verify configures certificate verification. |mode| is one
@@ -3840,11 +3828,11 @@ OPENSSL_EXPORT int SSL_delegated_credential_used(const SSL *ssl);
 
 // ssl_encryption_level_t represents a specific QUIC encryption level used to
 // transmit handshake messages.
-enum ssl_encryption_level_t BORINGSSL_ENUM_INT {
+enum ssl_encryption_level_t {
   ssl_encryption_initial = 0,
   ssl_encryption_early_data,
   ssl_encryption_handshake,
-  ssl_encryption_application,
+  ssl_encryption_application
 };
 
 // ssl_quic_method_st (aka |SSL_QUIC_METHOD|) describes custom QUIC hooks.
@@ -4111,7 +4099,7 @@ OPENSSL_EXPORT int32_t SSL_get_ticket_age_skew(const SSL *ssl);
 // An ssl_early_data_reason_t describes why 0-RTT was accepted or rejected.
 // These values are persisted to logs. Entries should not be renumbered and
 // numeric values should never be reused.
-enum ssl_early_data_reason_t BORINGSSL_ENUM_INT {
+enum ssl_early_data_reason_t {
   // The handshake has not progressed far enough for the 0-RTT status to be
   // known.
   ssl_early_data_unknown = 0,
@@ -4145,7 +4133,7 @@ enum ssl_early_data_reason_t BORINGSSL_ENUM_INT {
   // The value of the largest entry.
   ssl_early_data_unsupported_with_custom_extension = 15,
   ssl_early_data_reason_max_value =
-      ssl_early_data_unsupported_with_custom_extension,
+      ssl_early_data_unsupported_with_custom_extension
 };
 
 // SSL_get_early_data_reason returns details why 0-RTT was accepted or rejected
@@ -4658,12 +4646,12 @@ OPENSSL_EXPORT void SSL_CTX_set_current_time_cb(
 // such as HTTP/1.1, and not others, such as HTTP/2.
 OPENSSL_EXPORT void SSL_set_shed_handshake_config(SSL *ssl, int enable);
 
-enum ssl_renegotiate_mode_t BORINGSSL_ENUM_INT {
+enum ssl_renegotiate_mode_t {
   ssl_renegotiate_never = 0,
   ssl_renegotiate_once,
   ssl_renegotiate_freely,
   ssl_renegotiate_ignore,
-  ssl_renegotiate_explicit,
+  ssl_renegotiate_explicit
 };
 
 // SSL_set_renegotiate_mode configures how |ssl|, a client, reacts to
@@ -4795,7 +4783,7 @@ struct ssl_early_callback_ctx {
 
 // ssl_select_cert_result_t enumerates the possible results from selecting a
 // certificate with |select_certificate_cb|.
-enum ssl_select_cert_result_t BORINGSSL_ENUM_INT {
+enum ssl_select_cert_result_t {
   // ssl_select_cert_success indicates that the certificate selection was
   // successful.
   ssl_select_cert_success = 1,
@@ -4804,7 +4792,7 @@ enum ssl_select_cert_result_t BORINGSSL_ENUM_INT {
   ssl_select_cert_retry = 0,
   // ssl_select_cert_error indicates that a fatal error occured and the
   // handshake should be terminated.
-  ssl_select_cert_error = -1,
+  ssl_select_cert_error = -1
 };
 
 // SSL_early_callback_ctx_extension_get searches the extensions in

tests/ci/cdk/cdk/codebuild/github_ci_linux_x86_omnibus.yaml

@@ -14,6 +14,16 @@ batch:
         compute-type: BUILD_GENERAL1_SMALL
         image: 620771051181.dkr.ecr.us-west-2.amazonaws.com/aws-lc-docker-images-linux-x86:ubuntu-20.04_clang-8x_latest
 
+    - identifier: c99_cplusplus98_checker # The checker script runs on gcc.
+      buildspec: ./tests/ci/codebuild/common/run_simple_target.yml
+      env:
+        type: LINUX_CONTAINER
+        privileged-mode: false
+        compute-type: BUILD_GENERAL1_SMALL
+        image: 620771051181.dkr.ecr.us-west-2.amazonaws.com/aws-lc-docker-images-linux-x86:ubuntu-22.04_gcc-12x_latest
+        variables:
+          AWS_LC_CI_TARGET: "tests/coding_guidelines/c99_cplusplus98_test.sh"
+
     - identifier: ubuntu1604_gcc5x_x86
       buildspec: ./tests/ci/codebuild/common/run_simple_target.yml
       env:

tests/ci/codebuild/linux-x86/pre-push.yml

@@ -13,5 +13,5 @@ phases:
       - ./tests/check_objects_and_errors.sh
       - go run ./tests/check_licenses.go
       - ./tests/check_generated_src.sh
-      - ./tests/coding_guidelines/coding_guidelines_test.sh
+      - ./tests/coding_guidelines/style.sh
       - (cd util && go run ./doc.go)

tests/ci/run_posix_tests.sh

@@ -38,16 +38,6 @@ build_and_test -DDISABLE_PERL=ON -DENABLE_DILITHIUM=ON
 echo "Testing building with AArch64 Data-Independent Timing (DIT) on."
 build_and_test -DENABLE_DATA_INDEPENDENT_TIMING=ON -DCMAKE_BUILD_TYPE=Release -DENABLE_DILITHIUM=ON
 
-if [[ "${AWSLC_C99_TEST}" == "1" ]]; then
-    echo "Testing the C99 compatability of AWS-LC headers."
-    ./tests/coding_guidelines/c99_gcc_test.sh
-fi
-
-if [[ "${AWSLC_CODING_GUIDELINES_TEST}" == "1" ]]; then
-  echo "Testing that AWS-LC is compliant with the coding guidelines."
-  source ./tests/coding_guidelines/coding_guidelines_test.sh
-fi
-
 # Lightly verify that uncommon build options does not break the build. Fist
 # define a list of typical build options to verify the special build option with
 build_options_to_test=("" "-DBUILD_SHARED_LIBS=1" "-DCMAKE_BUILD_TYPE=Release" "-DBUILD_SHARED_LIBS=1 -DCMAKE_BUILD_TYPE=Release" "-DDISABLE_PERL=ON -DDISABLE_GO=ON")

tests/coding_guidelines/c99_gcc_test.sh → tests/coding_guidelines/c99_cplusplus98_test.sh

@@ -24,8 +24,7 @@ INCLUDE_FILES=`ls $INCLUDE_DIR/openssl/*.h | grep -v $INCLUDE_DIR/openssl/arm_ar
 # some non-ISO practices, but not all — only those for which ISO C requires a
 # diagnostic, and some others for which diagnostics have been added."
 # https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
-
-${CC} -std=c99 -c -I${INCLUDE_DIR} -include ${INCLUDE_FILES} -Wpedantic -fsyntax-only -Werror
+${CC} -std=c99 -c -I${INCLUDE_DIR} $(echo ${INCLUDE_FILES} | sed 's/[^ ]* */-include &/g') -Wpedantic -fsyntax-only -Werror ./tests/compiler_features_tests/builtin_swap_check.c
 
 # AWS C SDKs conforms to C99. They set `C_STANDARD 99` which will set the
 # flag `-std=gnu99`
@@ -34,5 +33,12 @@ ${CC} -std=c99 -c -I${INCLUDE_DIR} -include ${INCLUDE_FILES} -Wpedantic -fsyntax
 # https://cmake.org/cmake/help/latest/prop_tgt/C_STANDARD.html
 #
 # the c99 and gnu99 modes are different, so let's test both.
-
-${CC} -std=gnu99 -c -I${INCLUDE_DIR} -include ${INCLUDE_FILES} -Wpedantic -fsyntax-only -Werror
+${CC} -std=gnu99 -c -I${INCLUDE_DIR} $(echo ${INCLUDE_FILES} | sed 's/[^ ]* */-include &/g') -Wpedantic -fsyntax-only -Werror ./tests/compiler_features_tests/builtin_swap_check.c
+
+# Our SSL headers use C++, but older compilers do not have the C++11 flag enabled by
+# default. Not all consuming applications that use older compilers have enabled the
+# C++11 feature flag. To ensure a smoother integration process for migrating
+# applications, we should ensure that the default settings of older C++ compilers
+# work with our header files.
+${CXX} -std=c++98 -c -I${INCLUDE_DIR} $(echo ${INCLUDE_FILES} | sed 's/[^ ]* */-include &/g') -Wpedantic -fsyntax-only -Werror ./tests/compiler_features_tests/builtin_swap_check.c
+${CXX} -std=gnu++98 -c -I${INCLUDE_DIR} $(echo ${INCLUDE_FILES} | sed 's/[^ ]* */-include &/g') -Wpedantic -fsyntax-only -Werror ./tests/compiler_features_tests/builtin_swap_check.c