Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix C++98 compatibility in our header files #2193

Merged
merged 3 commits into from
Feb 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/openssl/aead.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion include/openssl/bn.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion include/openssl/crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion include/openssl/curve25519.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion include/openssl/ec.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;


Expand Down
2 changes: 1 addition & 1 deletion include/openssl/service_indicator.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
62 changes: 25 additions & 37 deletions include/openssl/ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
10 changes: 10 additions & 0 deletions tests/ci/cdk/cdk/codebuild/github_ci_linux_x86_omnibus.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion tests/ci/codebuild/linux-x86/pre-push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
10 changes: 0 additions & 10 deletions tests/ci/run_posix_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this sed removing from the list? And why add the builtin_swap_check to all of these targets?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sed's mainly in charge of prepending an -include to each file in $INCLUDE_FILES. What we had previously didn't actually work :(. This also needs a basic "c" file to compile, so I chose builtin_swap_check since that was a good existing example that works with older compilers


# AWS C SDKs conforms to C99. They set `C_STANDARD 99` which will set the
# flag `-std=gnu99`
Expand All @@ -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
8 changes: 0 additions & 8 deletions tests/coding_guidelines/coding_guidelines_test.sh

This file was deleted.

Loading