resolve memory leak in security test cases and process review comments

Signed-off-by: Marcel Jordense <[email protected]>

src/security/builtin_plugins/authentication/src/auth_utils.c

@@ -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;
@@ -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;
@@ -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)
@@ -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: ");
@@ -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);
@@ -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

src/security/builtin_plugins/tests/common/src/handshake_helper.c

@@ -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
@@ -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;
 }
 
@@ -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:
@@ -919,10 +923,6 @@ get_public_key(
     return result;
 }
 
-
-
-
-
 int
 check_shared_secret(
     dds_security_authentication *auth,
@@ -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;
 }
 

src/security/builtin_plugins/tests/listeners_authentication/src/listeners_authentication_utests.c

@@ -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;
@@ -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;
     }

src/security/core/tests/common/cert_utils.c

@@ -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);

src/security/openssl/include/dds/security/openssl_support.h

@@ -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>