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

Upstream support for AWS-LC #672

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
23 changes: 23 additions & 0 deletions README.awslc
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
This version of OpenVPN supports AWS-LC (AWS Libcrypto), AWS's open-source cryptographic library.

If you encounter bugs in OpenVPN while using AWS-LC:
1. Try compiling OpenVPN with OpenSSL to determine if the issue is specific to AWS-LC
2. For AWS-LC-specific issues, please report them at: https://github.com/aws/aws-lc

To build and install OpenVPN with AWS-LC:

OPENSSL_CFLAGS="-I/${AWS_LC_INSTALL_FOLDER}/include" \
OPENSSL_LIBS="-L/${AWS_LC_INSTALL_FOLDER}/lib -lssl -lcrypto" \
./configure --with-crypto-library=openssl
make
make install

export LD_LIBRARY_PATH="${AWS_LC_INSTALL_FOLDER}/lib"

When running tests, LD_LIBRARY_PATH must be passed in again:

LD_LIBRARY_PATH="${AWS_LC_INSTALL_FOLDER}/lib" make check

*************************************************************************
Due to limitations in AWS-LC, the following features are missing
* Windows CryptoAPI support
7 changes: 7 additions & 0 deletions src/openvpn/crypto_openssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1398,6 +1398,13 @@ ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret,

return ret;
}
#elif defined(OPENSSL_IS_AWSLC)
bool
ssl_tls1_PRF(const uint8_t *label, int label_len, const uint8_t *sec,
int slen, uint8_t *out1, int olen)
{
CRYPTO_tls1_prf(EVP_md5_sha1(), out1, olen, sec, slen, label, label_len, NULL, 0, NULL, 0);
}
#elif !defined(LIBRESSL_VERSION_NUMBER) && !defined(ENABLE_CRYPTO_WOLFSSL)
bool
ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret,
Expand Down
2 changes: 1 addition & 1 deletion src/openvpn/openssl_compat.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ X509_OBJECT_free(X509_OBJECT *obj)
#define RSA_F_RSA_OSSL_PRIVATE_ENCRYPT RSA_F_RSA_EAY_PRIVATE_ENCRYPT
#endif

#if defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x3050400fL
#if defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x3050400fL || defined(OPENSSL_IS_AWSLC)
#define SSL_get_peer_tmp_key SSL_get_server_tmp_key
#endif

Expand Down
11 changes: 8 additions & 3 deletions src/openvpn/ssl_openssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1669,7 +1669,11 @@ tls_ctx_use_external_ec_key(struct tls_root_ctx *ctx, EVP_PKEY *pkey)

/* Among init methods, we only need the finish method */
EC_KEY_METHOD_set_init(ec_method, NULL, openvpn_extkey_ec_finish, NULL, NULL, NULL, NULL);
#ifdef OPENSSL_IS_AWSLC
EC_KEY_METHOD_set_sign(ec_method, ecdsa_sign, NULL, ecdsa_sign_sig);
#else
EC_KEY_METHOD_set_sign(ec_method, ecdsa_sign, ecdsa_sign_setup, ecdsa_sign_sig);
#endif

ec = EC_KEY_dup(EVP_PKEY_get0_EC_KEY(pkey));
if (!ec)
Expand Down Expand Up @@ -1895,9 +1899,11 @@ tls_ctx_load_ca(struct tls_root_ctx *ctx, const char *ca_file,
}
sk_X509_INFO_pop_free(info_stack, X509_INFO_free);
}


int cnum;
if (tls_server)
{
cnum = sk_X509_NAME_num(cert_names);
SSL_CTX_set_client_CA_list(ctx->ctx, cert_names);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This change seems weird. With the old code we add the client CA list and then use sk_X509_NAME_num get the number, while with this patch we change the order. I haven't look exactly into what sk_X509_NAME_num does right now since it is one of the annoying function where finding the man page is hard but doesn't this change the logic?

Choose a reason for hiding this comment

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

Good catch!This change addresses a subtle behavioral difference between AWS-LC and OpenSSL regarding object ownership semantics in SSL_CTX_set_client_CA_list(ctx->ctx, cert_names).

  1. OpenSSL Behavior:

    • Stores a reference to the provided cert_names stack
    • cert_names remains valid after SSL_CTX_set_client_CA_list
  2. AWS-LC Behavior:

    • Creates a copy of the parameter cert_names (which is a stack of type X509_NAME) and converts it to a stack of CRYPTO_BUFFER (how we internally represent X509_NAME, it's an opaque byte string).
    • Then frees the original passed in cert_names
    • After SSL_CTX_set_client_CA_list, cert_names no longer points to valid memory

The proposed changes reorder operations to getting the size of the stack before the set operation as opposed to after the set operation. No operations between the setter and stack size check modify cert_names. Therefore, the logical outcome should remain the same - and this would also handle the subtle behavioral difference in AWS-LC.


Expand All @@ -1910,7 +1916,6 @@ tls_ctx_load_ca(struct tls_root_ctx *ctx, const char *ca_file,

if (tls_server)
{
int cnum = sk_X509_NAME_num(cert_names);
if (cnum != added)
{
crypto_msg(M_FATAL, "Cannot load CA certificate file %s (only %d "
Expand Down Expand Up @@ -2558,7 +2563,7 @@ show_available_tls_ciphers_list(const char *cipher_list,
crypto_msg(M_FATAL, "Cannot create SSL object");
}

#if OPENSSL_VERSION_NUMBER < 0x1010000fL
#if OPENSSL_VERSION_NUMBER < 0x1010000fL || defined(OPENSSL_IS_AWSLC)
STACK_OF(SSL_CIPHER) *sk = SSL_get_ciphers(ssl);
#else
STACK_OF(SSL_CIPHER) *sk = SSL_get1_supported_ciphers(ssl);
Expand Down