diff --git a/conf/options/charon.opt b/conf/options/charon.opt index dc052a89a..d57fb4ced 100644 --- a/conf/options/charon.opt +++ b/conf/options/charon.opt @@ -129,9 +129,6 @@ charon.dns2 charon.dos_protection = yes Enable Denial of Service protection using cookies and aggressiveness checks. -charon.ecp_x_coordinate_only = yes - Compliance with the errata for RFC 4753. - charon.flush_auth_cfg = no Free objects during authentication (might conflict with plugins). diff --git a/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c b/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c index 19de540b6..180eb0ab2 100644 --- a/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c +++ b/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2008-2013 Tobias Brunner + * Copyright (C) 2008-2021 Tobias Brunner * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it @@ -17,10 +17,13 @@ #ifndef OPENSSL_NO_EC -#include +#include #include #include + +#if OPENSSL_VERSION_NUMBER < 0x1010000fL || defined(OPENSSL_IS_BORINGSSL) #include +#endif #include "openssl_ec_diffie_hellman.h" #include "openssl_util.h" @@ -46,17 +49,12 @@ struct private_openssl_ec_diffie_hellman_t { /** * EC private (public) key */ - EC_KEY *key; + EVP_PKEY *key; /** * EC group */ - const EC_GROUP *ec_group; - - /** - * Other public key - */ - EC_POINT *pub_key; + EC_GROUP *ec_group; /** * Shared secret @@ -69,12 +67,15 @@ struct private_openssl_ec_diffie_hellman_t { bool computed; }; +#if OPENSSL_VERSION_NUMBER < 0x1010000fL || defined(OPENSSL_IS_BORINGSSL) /** - * Convert a chunk to an EC_POINT (which must already exist). The x and y + * Convert a chunk to an EC_POINT and set it on the given key. The x and y * coordinates of the point have to be concatenated in the chunk. */ -static bool chunk2ecp(const EC_GROUP *group, chunk_t chunk, EC_POINT *point) +static bool chunk2ecp(const EC_GROUP *group, chunk_t chunk, EVP_PKEY *key) { + EC_POINT *point = NULL; + EC_KEY *pub = NULL; BN_CTX *ctx; BIGNUM *x, *y; bool ret = FALSE; @@ -98,7 +99,8 @@ static bool chunk2ecp(const EC_GROUP *group, chunk_t chunk, EC_POINT *point) goto error; } - if (!EC_POINT_set_affine_coordinates_GFp(group, point, x, y, ctx)) + point = EC_POINT_new(group); + if (!point || !EC_POINT_set_affine_coordinates_GFp(group, point, x, y, ctx)) { goto error; } @@ -108,20 +110,40 @@ static bool chunk2ecp(const EC_GROUP *group, chunk_t chunk, EC_POINT *point) goto error; } + pub = EC_KEY_new(); + if (!pub || !EC_KEY_set_group(pub, group)) + { + goto error; + } + + if (EC_KEY_set_public_key(pub, point) != 1) + { + goto error; + } + + if (EVP_PKEY_set1_EC_KEY(key, pub) != 1) + { + goto error; + } + ret = TRUE; + error: + EC_POINT_clear_free(point); + EC_KEY_free(pub); BN_CTX_end(ctx); BN_CTX_free(ctx); return ret; } /** - * Convert an EC_POINT to a chunk by concatenating the x and y coordinates of - * the point. This function allocates memory for the chunk. + * Convert a key to a chunk by concatenating the x and y coordinates of + * the underlying EC point. This function allocates memory for the chunk. */ -static bool ecp2chunk(const EC_GROUP *group, const EC_POINT *point, - chunk_t *chunk, bool x_coordinate_only) +static bool ecp2chunk(const EC_GROUP *group, EVP_PKEY *key, chunk_t *chunk) { + EC_KEY *ec_key = NULL; + const EC_POINT *point; BN_CTX *ctx; BIGNUM *x, *y; bool ret = FALSE; @@ -140,118 +162,101 @@ static bool ecp2chunk(const EC_GROUP *group, const EC_POINT *point, goto error; } - if (!EC_POINT_get_affine_coordinates_GFp(group, point, x, y, ctx)) + ec_key = EVP_PKEY_get1_EC_KEY(key); + point = EC_KEY_get0_public_key(ec_key); + if (!point || !EC_POINT_get_affine_coordinates_GFp(group, point, x, y, ctx)) { goto error; } - if (x_coordinate_only) - { - y = NULL; - } if (!openssl_bn_cat(EC_FIELD_ELEMENT_LEN(group), x, y, chunk)) { goto error; } - ret = TRUE; + ret = chunk->len != 0; error: + EC_KEY_free(ec_key); BN_CTX_end(ctx); BN_CTX_free(ctx); return ret; } - -/** - * Compute the shared secret. - * - * We cannot use the function ECDH_compute_key() because that returns only the - * x coordinate of the shared secret point (which is defined, for instance, in - * 'NIST SP 800-56A'). - * However, we need both coordinates as RFC 4753 says: "The Diffie-Hellman - * public value is obtained by concatenating the x and y values. The format - * of the Diffie-Hellman shared secret value is the same as that of the - * Diffie-Hellman public value." - */ -static bool compute_shared_key(private_openssl_ec_diffie_hellman_t *this, - chunk_t *shared_secret) -{ - const BIGNUM *priv_key; - EC_POINT *secret = NULL; - bool x_coordinate_only, ret = FALSE; - - priv_key = EC_KEY_get0_private_key(this->key); - if (!priv_key) - { - goto error; - } - - secret = EC_POINT_new(this->ec_group); - if (!secret) - { - goto error; - } - - if (!EC_POINT_mul(this->ec_group, secret, NULL, this->pub_key, priv_key, NULL)) - { - goto error; - } - - /* - * The default setting ecp_x_coordinate_only = TRUE - * applies the following errata for RFC 4753: - * http://www.rfc-editor.org/errata_search.php?eid=9 - */ - x_coordinate_only = lib->settings->get_bool(lib->settings, - "%s.ecp_x_coordinate_only", TRUE, lib->ns); - if (!ecp2chunk(this->ec_group, secret, shared_secret, x_coordinate_only)) - { - goto error; - } - - ret = TRUE; -error: - if (secret) - { - EC_POINT_clear_free(secret); - } - return ret; -} +#endif /* OPENSSL_VERSION_NUMBER < ... */ METHOD(diffie_hellman_t, set_other_public_value, bool, private_openssl_ec_diffie_hellman_t *this, chunk_t value) { + EVP_PKEY *pub = NULL; + + chunk_clear(&this->shared_secret); + this->computed = FALSE; + if (!diffie_hellman_verify_value(this->group, value)) { return FALSE; } - if (!chunk2ecp(this->ec_group, value, this->pub_key)) + pub = EVP_PKEY_new(); + if (!pub) + { + goto error; + } + +#if OPENSSL_VERSION_NUMBER < 0x1010000fL || defined(OPENSSL_IS_BORINGSSL) + if (!chunk2ecp(this->ec_group, value, pub)) { DBG1(DBG_LIB, "ECDH public value is malformed"); - return FALSE; + goto error; } +#else + /* OpenSSL expects the pubkey in the format specified in section 2.3.4 of + * SECG SEC 1, i.e. prefixed with 0x04 to indicate an uncompressed point */ + value = chunk_cata("cc", chunk_from_chars(0x04), value); + if (EVP_PKEY_copy_parameters(pub, this->key) <= 0 || + EVP_PKEY_set1_tls_encodedpoint(pub, value.ptr, value.len) <= 0) + { + DBG1(DBG_LIB, "ECDH public value is malformed"); + goto error; + } +#endif - chunk_clear(&this->shared_secret); - - if (!compute_shared_key(this, &this->shared_secret)) { + if (!openssl_compute_shared_key(this->key, pub, &this->shared_secret)) + { DBG1(DBG_LIB, "ECDH shared secret computation failed"); - return FALSE; + goto error; } - this->computed = TRUE; - return TRUE; + +error: + EVP_PKEY_free(pub); + return this->computed; } METHOD(diffie_hellman_t, get_my_public_value, bool, - private_openssl_ec_diffie_hellman_t *this,chunk_t *value) + private_openssl_ec_diffie_hellman_t *this, chunk_t *value) { - ecp2chunk(this->ec_group, EC_KEY_get0_public_key(this->key), value, FALSE); - return TRUE; +#if OPENSSL_VERSION_NUMBER < 0x1010000fL || defined(OPENSSL_IS_BORINGSSL) + return ecp2chunk(this->ec_group, this->key, value); +#else + chunk_t pub; + + /* OpenSSL returns the pubkey in the format specified in section 2.3.4 of + * SECG SEC 1, i.e. prefixed with 0x04 to indicate an uncompressed point */ + pub.len = EVP_PKEY_get1_tls_encodedpoint(this->key, &pub.ptr); + if (pub.len != 0) + { + *value = chunk_clone(chunk_skip(pub, 1)); + chunk_free(&pub); + return value->len != 0; + } + return FALSE; +#endif } METHOD(diffie_hellman_t, set_private_value, bool, private_openssl_ec_diffie_hellman_t *this, chunk_t value) { + EC_KEY *key = NULL; EC_POINT *pub = NULL; BIGNUM *priv = NULL; bool ret = FALSE; @@ -261,7 +266,7 @@ METHOD(diffie_hellman_t, set_private_value, bool, { goto error; } - pub = EC_POINT_new(EC_KEY_get0_group(this->key)); + pub = EC_POINT_new(this->ec_group); if (!pub) { goto error; @@ -270,25 +275,29 @@ METHOD(diffie_hellman_t, set_private_value, bool, { goto error; } - if (EC_KEY_set_private_key(this->key, priv) != 1) + key = EC_KEY_new(); + if (!key || !EC_KEY_set_group(key, this->ec_group)) { goto error; } - if (EC_KEY_set_public_key(this->key, pub) != 1) + if (EC_KEY_set_private_key(key, priv) != 1) + { + goto error; + } + if (EC_KEY_set_public_key(key, pub) != 1) + { + goto error; + } + if (EVP_PKEY_set1_EC_KEY(this->key, key) != 1) { goto error; } ret = TRUE; error: - if (pub) - { - EC_POINT_free(pub); - } - if (priv) - { - BN_free(priv); - } + EC_POINT_free(pub); + BN_free(priv); + EC_KEY_free(key); return ret; } @@ -312,14 +321,8 @@ METHOD(diffie_hellman_t, get_dh_group, diffie_hellman_group_t, METHOD(diffie_hellman_t, destroy, void, private_openssl_ec_diffie_hellman_t *this) { - if (this->pub_key) - { - EC_POINT_clear_free(this->pub_key); - } - if (this->key) - { - EC_KEY_free(this->key); - } + EC_GROUP_free(this->ec_group); + EVP_PKEY_free(this->key); chunk_clear(&this->shared_secret); free(this); } @@ -598,6 +601,39 @@ static EC_KEY *ec_key_new_brainpool(diffie_hellman_group_t group) openssl_ec_diffie_hellman_t *openssl_ec_diffie_hellman_create(diffie_hellman_group_t group) { private_openssl_ec_diffie_hellman_t *this; + EC_KEY *key = NULL; + + switch (group) + { + case ECP_192_BIT: + key = EC_KEY_new_by_curve_name(NID_X9_62_prime192v1); + break; + case ECP_224_BIT: + key = EC_KEY_new_by_curve_name(NID_secp224r1); + break; + case ECP_256_BIT: + key = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1); + break; + case ECP_384_BIT: + key = EC_KEY_new_by_curve_name(NID_secp384r1); + break; + case ECP_521_BIT: + key = EC_KEY_new_by_curve_name(NID_secp521r1); + break; + case ECP_224_BP: + case ECP_256_BP: + case ECP_384_BP: + case ECP_512_BP: + key = ec_key_new_brainpool(group); + break; + default: + break; + } + + if (!key) + { + return NULL; + } INIT(this, .public = { @@ -611,59 +647,24 @@ openssl_ec_diffie_hellman_t *openssl_ec_diffie_hellman_create(diffie_hellman_gro }, }, .group = group, + .ec_group = EC_GROUP_dup(EC_KEY_get0_group(key)), ); - switch (group) - { - case ECP_192_BIT: - this->key = EC_KEY_new_by_curve_name(NID_X9_62_prime192v1); - break; - case ECP_224_BIT: - this->key = EC_KEY_new_by_curve_name(NID_secp224r1); - break; - case ECP_256_BIT: - this->key = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1); - break; - case ECP_384_BIT: - this->key = EC_KEY_new_by_curve_name(NID_secp384r1); - break; - case ECP_521_BIT: - this->key = EC_KEY_new_by_curve_name(NID_secp521r1); - break; - case ECP_224_BP: - case ECP_256_BP: - case ECP_384_BP: - case ECP_512_BP: - this->key = ec_key_new_brainpool(group); - break; - default: - this->key = NULL; - break; - } - - if (!this->key) - { - free(this); - return NULL; - } - - /* caching the EC group */ - this->ec_group = EC_KEY_get0_group(this->key); - - this->pub_key = EC_POINT_new(this->ec_group); - if (!this->pub_key) - { - destroy(this); - return NULL; - } - /* generate an EC private (public) key */ - if (!EC_KEY_generate_key(this->key)) + if (!EC_KEY_generate_key(key)) { + EC_KEY_free(key); destroy(this); return NULL; } + this->key = EVP_PKEY_new(); + if (!this->key || !EVP_PKEY_assign_EC_KEY(this->key, key)) + { + EC_KEY_free(key); + destroy(this); + return NULL; + } return &this->public; } #endif /* OPENSSL_NO_EC */ diff --git a/src/libstrongswan/plugins/openssl/openssl_util.c b/src/libstrongswan/plugins/openssl/openssl_util.c index ac6784a23..55fe80ae8 100644 --- a/src/libstrongswan/plugins/openssl/openssl_util.c +++ b/src/libstrongswan/plugins/openssl/openssl_util.c @@ -29,6 +29,49 @@ #define ASN1_STRING_get0_data(a) ASN1_STRING_data((ASN1_STRING*)a) #endif +/* + * Described in header + */ +bool openssl_compute_shared_key(EVP_PKEY *priv, EVP_PKEY *pub, chunk_t *shared) +{ + EVP_PKEY_CTX *ctx; + bool success = FALSE; + + ctx = EVP_PKEY_CTX_new(priv, NULL); + if (!ctx) + { + return FALSE; + } + + if (EVP_PKEY_derive_init(ctx) <= 0) + { + goto error; + } + + if (EVP_PKEY_derive_set_peer(ctx, pub) <= 0) + { + goto error; + } + + if (EVP_PKEY_derive(ctx, NULL, &shared->len) <= 0) + { + goto error; + } + + *shared = chunk_alloc(shared->len); + + if (EVP_PKEY_derive(ctx, shared->ptr, &shared->len) <= 0) + { + goto error; + } + + success = TRUE; + +error: + EVP_PKEY_CTX_free(ctx); + return success; +} + /** * Described in header. */ diff --git a/src/libstrongswan/plugins/openssl/openssl_util.h b/src/libstrongswan/plugins/openssl/openssl_util.h index c610433b1..eb2a3788b 100644 --- a/src/libstrongswan/plugins/openssl/openssl_util.h +++ b/src/libstrongswan/plugins/openssl/openssl_util.h @@ -36,6 +36,16 @@ */ #define EC_FIELD_ELEMENT_LEN(group) ((EC_GROUP_get_degree(group) + 7) / 8) +/** + * Derives a shared DH secret from the given keys. + * + * @param priv private key + * @param pub public key + * @param shared shared secret + * @return TRUE on success, FALSE otherwise + */ +bool openssl_compute_shared_key(EVP_PKEY *priv, EVP_PKEY *pub, chunk_t *shared); + /** * Creates a hash of a given type of a chunk of data. * diff --git a/src/libstrongswan/plugins/openssl/openssl_x_diffie_hellman.c b/src/libstrongswan/plugins/openssl/openssl_x_diffie_hellman.c index 37943f5bf..3d41eb95a 100644 --- a/src/libstrongswan/plugins/openssl/openssl_x_diffie_hellman.c +++ b/src/libstrongswan/plugins/openssl/openssl_x_diffie_hellman.c @@ -20,6 +20,7 @@ #if OPENSSL_VERSION_NUMBER >= 0x1010100fL && !defined(OPENSSL_NO_ECDH) #include "openssl_x_diffie_hellman.h" +#include "openssl_util.h" #include @@ -71,50 +72,6 @@ static int map_key_type(diffie_hellman_group_t group) } } -/** - * Compute the shared secret - */ -static bool compute_shared_key(private_diffie_hellman_t *this, EVP_PKEY *pub, - chunk_t *shared_secret) -{ - EVP_PKEY_CTX *ctx; - bool success = FALSE; - - ctx = EVP_PKEY_CTX_new(this->key, NULL); - if (!ctx) - { - return FALSE; - } - - if (EVP_PKEY_derive_init(ctx) <= 0) - { - goto error; - } - - if (EVP_PKEY_derive_set_peer(ctx, pub) <= 0) - { - goto error; - } - - if (EVP_PKEY_derive(ctx, NULL, &shared_secret->len) <= 0) - { - goto error; - } - - *shared_secret = chunk_alloc(shared_secret->len); - - if (EVP_PKEY_derive(ctx, shared_secret->ptr, &shared_secret->len) <= 0) - { - goto error; - } - - success = TRUE; - -error: - EVP_PKEY_CTX_free(ctx); - return success; -} - METHOD(diffie_hellman_t, set_other_public_value, bool, private_diffie_hellman_t *this, chunk_t value) { @@ -136,7 +93,7 @@ METHOD(diffie_hellman_t, set_other_public_value, bool, chunk_clear(&this->shared_secret); - if (!compute_shared_key(this, pub, &this->shared_secret)) + if (!openssl_compute_shared_key(this->key, pub, &this->shared_secret)) { DBG1(DBG_LIB, "%N shared secret computation failed", diffie_hellman_group_names, this->group); diff --git a/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c b/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c index 565a57f9a..747dc62d0 100644 --- a/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c +++ b/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c @@ -139,12 +139,6 @@ METHOD(diffie_hellman_t, set_other_public_value, bool, pubkey.len, pubkey.ptr, }; - - if (!lib->settings->get_bool(lib->settings, - "%s.ecp_x_coordinate_only", TRUE, lib->ns)) - { /* we only get the x coordinate back */ - return FALSE; - } value = chunk_from_thing(params); break; } diff --git a/src/libstrongswan/plugins/wolfssl/wolfssl_ec_diffie_hellman.c b/src/libstrongswan/plugins/wolfssl/wolfssl_ec_diffie_hellman.c index ba6500694..4d3e8e21c 100644 --- a/src/libstrongswan/plugins/wolfssl/wolfssl_ec_diffie_hellman.c +++ b/src/libstrongswan/plugins/wolfssl/wolfssl_ec_diffie_hellman.c @@ -153,7 +153,6 @@ static bool compute_shared_key(private_wolfssl_ec_diffie_hellman_t *this, ecc_point *pub_key, chunk_t *shared_secret) { ecc_point* secret; - bool x_coordinate_only; bool success = FALSE; if ((secret = wc_ecc_new_point()) == NULL) @@ -163,15 +162,7 @@ static bool compute_shared_key(private_wolfssl_ec_diffie_hellman_t *this, if (wolfssl_ecc_multiply(this->key.dp, &this->key.k, pub_key, secret)) { - /* - * The default setting ecp_x_coordinate_only = TRUE - * applies the following errata for RFC 4753: - * http://www.rfc-editor.org/errata_search.php?eid=9 - */ - x_coordinate_only = lib->settings->get_bool(lib->settings, - "%s.ecp_x_coordinate_only", TRUE, lib->ns); - success = ecp2chunk(this->keysize, secret, shared_secret, - x_coordinate_only); + success = ecp2chunk(this->keysize, secret, shared_secret, TRUE); } wc_ecc_del_point(secret);