Skip to content

Commit

Permalink
resolve memory leak in security test cases and process review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Marcel Jordense <marcel@zettascale.tech>
  • Loading branch information
MarcelJordense committed Mar 19, 2024
1 parent f2a7317 commit c014f8e
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 28 deletions.
34 changes: 20 additions & 14 deletions src/security/builtin_plugins/authentication/src/auth_utils.c
Expand Up @@ -237,12 +237,12 @@ static DDS_Security_ValidationResult_t check_certificate_type_and_size(X509 *cer
DDS_Security_ValidationResult_t check_certificate_expiry(const X509 *cert, DDS_Security_SecurityException *ex)
{
assert(cert);
if (X509_cmp_current_time(X509_get_notBefore(cert)) == 0)
if (X509_cmp_current_time(X509_get0_notBefore(cert)) == 0)
{
DDS_Security_Exception_set(ex, DDS_AUTH_PLUGIN_CONTEXT, DDS_SECURITY_ERR_CERT_STARTDATE_IN_FUTURE_CODE, DDS_SECURITY_VALIDATION_FAILED, DDS_SECURITY_ERR_CERT_STARTDATE_IN_FUTURE_MESSAGE);
return DDS_SECURITY_VALIDATION_FAILED;
}
if (X509_cmp_current_time(X509_get_notAfter(cert)) == 0)
if (X509_cmp_current_time(X509_get0_notAfter(cert)) == 0)
{
DDS_Security_Exception_set(ex, DDS_AUTH_PLUGIN_CONTEXT, DDS_SECURITY_ERR_CERT_EXPIRED_CODE, DDS_SECURITY_VALIDATION_FAILED, DDS_SECURITY_ERR_CERT_EXPIRED_MESSAGE);
return DDS_SECURITY_VALIDATION_FAILED;
Expand Down Expand Up @@ -908,6 +908,7 @@ static int dh_set_public_key(DH *dhkey, BIGNUM *pubkey)

static DDS_Security_ValidationResult_t dh_public_key_to_oct_modp(EVP_PKEY *pkey, unsigned char **buffer, uint32_t *length, DDS_Security_SecurityException *ex)
{
DDS_Security_ValidationResult_t result = DDS_SECURITY_VALIDATION_FAILED;
DH *dhkey;
ASN1_INTEGER *asn1int;
*buffer = NULL;
Expand All @@ -919,31 +920,32 @@ static DDS_Security_ValidationResult_t dh_public_key_to_oct_modp(EVP_PKEY *pkey,
if (!(asn1int = BN_to_ASN1_INTEGER (dh_get_public_key(dhkey), NULL)))
{
DDS_Security_Exception_set_with_openssl_error(ex, DDS_AUTH_PLUGIN_CONTEXT, DDS_SECURITY_ERR_UNDEFINED_CODE, DDS_SECURITY_VALIDATION_FAILED, "Failed to convert DH key to ASN1 integer: ");
DH_free(dhkey);
return DDS_SECURITY_VALIDATION_FAILED;
goto failed_asn1int;
}

int i2dlen = i2d_ASN1_INTEGER (asn1int, NULL);
if (i2dlen <= 0)
{
DDS_Security_Exception_set_with_openssl_error(ex, DDS_AUTH_PLUGIN_CONTEXT, DDS_SECURITY_ERR_UNDEFINED_CODE, DDS_SECURITY_VALIDATION_FAILED, "Failed to convert DH key to ASN1 integer: ");
DH_free(dhkey);
return DDS_SECURITY_VALIDATION_FAILED;
goto failed_i2d;
}

*length = (uint32_t) i2dlen;
if ((*buffer = ddsrt_malloc (*length)) == NULL)
{
DDS_Security_Exception_set_with_openssl_error(ex, DDS_AUTH_PLUGIN_CONTEXT, DDS_SECURITY_ERR_UNDEFINED_CODE, DDS_SECURITY_VALIDATION_FAILED, "Failed to convert DH key to ASN1 integer: ");
DH_free(dhkey);
return DDS_SECURITY_VALIDATION_FAILED;
goto failed_i2d;
}

unsigned char *buffer_arg = *buffer;
(void) i2d_ASN1_INTEGER (asn1int, &buffer_arg);
result = DDS_SECURITY_VALIDATION_OK;

failed_i2d:
ASN1_INTEGER_free (asn1int);
failed_asn1int:
DH_free (dhkey);
return DDS_SECURITY_VALIDATION_OK;
return result;
}

static DDS_Security_ValidationResult_t dh_public_key_to_oct_ecdh(EVP_PKEY *pkey, unsigned char **buffer, uint32_t *length, DDS_Security_SecurityException *ex)
Expand Down Expand Up @@ -1040,6 +1042,7 @@ static DDS_Security_ValidationResult_t dh_oct_to_public_key_ecdh(EVP_PKEY **pkey
EC_KEY *eckey;
EC_GROUP *group;
EC_POINT *point;

if (!(group = EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1)))
{
DDS_Security_Exception_set_with_openssl_error(ex, DDS_AUTH_PLUGIN_CONTEXT, DDS_SECURITY_ERR_UNDEFINED_CODE, DDS_SECURITY_VALIDATION_FAILED, "Failed to allocate EC group: ");
Expand Down Expand Up @@ -1210,6 +1213,7 @@ static DDS_Security_ValidationResult_t dh_oct_to_public_key_modp(EVP_PKEY **pkey

static DDS_Security_ValidationResult_t dh_oct_to_public_key_ecdh(EVP_PKEY **pkey, const unsigned char *keystr, uint32_t size, DDS_Security_SecurityException *ex)
{
DDS_Security_ValidationResult_t result = DDS_SECURITY_VALIDATION_OK;
EVP_PKEY_CTX *ctx = NULL;
OSSL_PARAM params[3];
params[0] = OSSL_PARAM_construct_utf8_string(OSSL_PKEY_PARAM_GROUP_NAME, SN_X9_62_prime256v1, 0);
Expand All @@ -1224,17 +1228,19 @@ static DDS_Security_ValidationResult_t dh_oct_to_public_key_ecdh(EVP_PKEY **pkey
if (EVP_PKEY_fromdata_init(ctx) != 1)
{
DDS_Security_Exception_set(ex, DDS_AUTH_PLUGIN_CONTEXT, DDS_SECURITY_ERR_UNDEFINED_CODE, DDS_SECURITY_VALIDATION_FAILED, "EVP_PKEY_fromdata_init(EC) failed: ");
EVP_PKEY_CTX_free(ctx);
return DDS_SECURITY_VALIDATION_FAILED;
result = DDS_SECURITY_VALIDATION_FAILED;
goto failed;
}
if (EVP_PKEY_fromdata(ctx, pkey, EVP_PKEY_PUBLIC_KEY, params) != 1)
{
DDS_Security_Exception_set(ex, DDS_AUTH_PLUGIN_CONTEXT, DDS_SECURITY_ERR_UNDEFINED_CODE, DDS_SECURITY_VALIDATION_FAILED, "EVP_PKEY_fromdata(EC) failed: ");
EVP_PKEY_CTX_free(ctx);
return DDS_SECURITY_VALIDATION_FAILED;
result = DDS_SECURITY_VALIDATION_FAILED;
goto failed;
}

return DDS_SECURITY_VALIDATION_OK;
failed:
EVP_PKEY_CTX_free(ctx);
return result;
}

#endif
Expand Down
16 changes: 8 additions & 8 deletions src/security/builtin_plugins/tests/common/src/handshake_helper.c
Expand Up @@ -75,14 +75,17 @@ dh_set_public_key(
ASN1_INTEGER *
get_pubkey_asn1int(EVP_PKEY *pkey)
{
ASN1_INTEGER *result;
DH *dhkey = EVP_PKEY_get1_DH(pkey);
if (!dhkey) {
char *msg = get_openssl_error_message_for_test();
printf("Failed to get DH key from PKEY: %s", msg);
ddsrt_free(msg);
return NULL;
}
return BN_to_ASN1_INTEGER(dh_get_public_key(dhkey), NULL);
result = BN_to_ASN1_INTEGER(dh_get_public_key(dhkey), NULL);
DH_free(dhkey);
return result;
}

int
Expand Down Expand Up @@ -411,7 +414,7 @@ get_pubkey_asn1int(EVP_PKEY *pkey)
return NULL;
}
asn1int = BN_to_ASN1_INTEGER(pubkey_bn, NULL);
OPENSSL_free(pubkey_bn);
BN_free(pubkey_bn);
return asn1int;
}

Expand Down Expand Up @@ -555,6 +558,7 @@ modp_data_to_pubkey(
EVP_PKEY_CTX_free(pctx);
fail_ctx:
BN_free(pubkey);
ddsrt_free(buffer);
fail_pubkey:
ASN1_INTEGER_free(asn1int);
fail_asni:
Expand Down Expand Up @@ -919,10 +923,6 @@ get_public_key(
return result;
}





int
check_shared_secret(
dds_security_authentication *auth,
Expand Down Expand Up @@ -1079,9 +1079,9 @@ create_asymmetrical_signature_for_test(
ddsrt_free(*signature);
}

err_sign:
err_sign:
EVP_MD_CTX_destroy(mdctx);
err_create_ctx:
err_create_ctx:
return result;
}

Expand Down
Expand Up @@ -279,11 +279,11 @@ get_certificate_expiry(
/*_In_*/ X509 *cert)
{
dds_time_t expiry = DDS_TIME_INVALID;
ASN1_TIME *ans1;
const ASN1_TIME *ans1;

assert(cert);

ans1 = X509_get_notAfter(cert);
ans1 = X509_get0_notAfter(cert);
if (ans1 != NULL) {
int days;
int seconds;
Expand Down Expand Up @@ -454,12 +454,12 @@ static DDS_Security_boolean create_certificate_from_csr(const char* csr, long va
/* ---------------------------------------------------------- *
* Set X509V3 start date (now) and expiration date (+365 days)*
* -----------------------------------------------------------*/
if (!(X509_gmtime_adj(X509_get_notBefore(newcert), -10))) {
if (!(X509_gmtime_adj(X509_getm_notBefore(newcert), -10))) {
BIO_printf(outbio, "Error setting start time\n");
return false;
}

if (!(X509_gmtime_adj(X509_get_notAfter(newcert), valid_secs))) {
if (!(X509_gmtime_adj(X509_getm_notAfter(newcert), valid_secs))) {
BIO_printf(outbio, "Error setting expiration time\n");
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions src/security/core/tests/common/cert_utils.c
Expand Up @@ -25,8 +25,8 @@ static X509 * get_x509(int not_valid_before, int not_valid_after, const char * c
X509 * cert = X509_new ();
CU_ASSERT_FATAL (cert != NULL);
ASN1_INTEGER_set (X509_get_serialNumber (cert), 1);
X509_gmtime_adj (X509_get_notBefore (cert), not_valid_before);
X509_gmtime_adj (X509_get_notAfter (cert), not_valid_after);
X509_gmtime_adj (X509_getm_notBefore (cert), not_valid_before);
X509_gmtime_adj (X509_getm_notAfter (cert), not_valid_after);

X509_NAME * name = X509_get_subject_name (cert);
X509_NAME_add_entry_by_txt (name, "C", MBSTRING_ASC, (unsigned char *) "NL", -1, -1, 0);
Expand Down
Expand Up @@ -38,6 +38,7 @@
#include <WinSock2.h>
#endif

/* Setting this macro to 30000 specifies that the code will be compatible with openssl version 3 and lower like version 1.1 */
#define OPENSSL_API_COMPAT 30000

#include <openssl/opensslv.h>
Expand Down

0 comments on commit c014f8e

Please sign in to comment.