(xmlsec-openssl) Enabled password entry for PKCS12 files import; fixed error / cleanup paths; added comments

include/xmlsec/openssl/crypto.h

@@ -61,12 +61,21 @@
  *
 *****************************************************************************/
 #if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC)
+/*
+ * BoringSSL and AWS-LC do not provide a usable opensslconf.h, so the
+ * OPENSSL_NO_* capability mappings below are not available for them and the
+ * missing features have to be listed here explicitly (their opensslconf.h
+ * defines OPENSSL_NO_CAMELLIA, OPENSSL_NO_GOST and OPENSSL_NO_RMD160).
+ */
 #define XMLSEC_NO_RSA_OAEP              1
 #define XMLSEC_NO_DH                    1
 #define XMLSEC_NO_DSA                   1
 #define XMLSEC_NO_SHA3                  1
 #define XMLSEC_NO_CAMELLIA              1
 #define XMLSEC_NO_CHACHA20              1
+#define XMLSEC_NO_GOST                  1
+#define XMLSEC_NO_GOST2012              1
+#define XMLSEC_NO_RIPEMD160             1
 #endif /* defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC) */
 
 #if defined(LIBRESSL_VERSION_NUMBER)
@@ -205,6 +214,10 @@ XMLSEC_CRYPTO_EXPORT BIO*               xmlSecOpenSSLCreateReadFileBio (const ch
 #define XMLSEC_NO_DES       1
 #endif /* OPENSSL_NO_DES */
 
+#ifdef OPENSSL_NO_DH
+#define XMLSEC_NO_DH        1
+#endif /* OPENSSL_NO_DH */
+
 #ifdef OPENSSL_NO_DSA
 #define XMLSEC_NO_DSA       1
 #endif /* OPENSSL_NO_DSA */

scripts/build_memcheck.sh

@@ -22,12 +22,11 @@ parallel_jobs=${PARALLEL_JOBS:-12}
 tmp_folder=${TMPFOLDER:-/tmp}
 
 echo "============== Starting memcheck for ${crypto} using source root '${top_dir}'"
-cd "$top_dir"
 rm -rf "${tmp_folder}"/xmlsec-test*
 if [ -f Makefile ]; then
     make distclean
 fi
-autoreconf -i -f
-./configure --enable-development --enable-legacy-features --with-default-crypto=${crypto} "$@"
+autoreconf -i -f "${top_dir}"
+${top_dir}/configure --enable-development --enable-legacy-features --with-default-crypto=${crypto} "$@"
 make -j${parallel_jobs}
 make memcheck-crypto-${crypto}

src/openssl/app.c

@@ -1377,31 +1377,29 @@ xmlSecOpenSSLAppPkcs12LoadMemory(
  * in format=xmlSecKeyDataFormatPkcs12.
  *
  * @param bio the PKCS12 key bio.
- * @param pwd the PKCS12 file password.
- * @param pwdCallback the password callback.
+ * @param pwd the PKCS12 file password. If NULL, the password is obtained
+ *        from the password callback.
+ * @param pwdCallback the password callback, used when pwd is NULL.
  * @param pwdCallbackCtx the user context for password callback.
  * @return pointer to the key or NULL if an error occurs.
  */
 xmlSecKeyPtr
 xmlSecOpenSSLAppPkcs12LoadBIO(BIO* bio, const char *pwd,
-                           void* pwdCallback XMLSEC_ATTRIBUTE_UNUSED,
-                           void* pwdCallbackCtx XMLSEC_ATTRIBUTE_UNUSED) {
+                            void* pwdCallback,
+                            void* pwdCallbackCtx) {
 
     PKCS12 *p12 = NULL;
     EVP_PKEY * pKey = NULL;
     X509 * keyCert = NULL;
     STACK_OF(X509) * chain = NULL;
     xmlSecKeyPtr res = NULL;
+    pem_password_cb* pwdCb = NULL;
+    char pwdBuf[2048];
     size_t pwdSize;
     int pwdLen;
     int ret;
 
     xmlSecAssert2(bio != NULL, NULL);
-    XMLSEC_UNREFERENCED(pwdCallback);
-    XMLSEC_UNREFERENCED(pwdCallbackCtx);
-
-    pwdSize = (pwd != NULL) ? strlen(pwd) : 0;
-    XMLSEC_SAFE_CAST_SIZE_T_TO_INT(pwdSize, pwdLen, return(NULL), NULL);
 
     XMLSEC_OPENSSL_PUSH_LIB_CTX(goto done);
     p12 = d2i_PKCS12_bio(bio, NULL);
@@ -1411,6 +1409,24 @@ xmlSecOpenSSLAppPkcs12LoadBIO(BIO* bio, const char *pwd,
         goto done;
     }
 
+    /*
+     * If the password is not given explicitly, try to obtain it
+     * from the password callback.
+     */
+    if((pwd == NULL) && (pwdCallback != NULL)) {
+        pwdCb = XMLSEC_PTR_TO_FUNC(pem_password_cb, pwdCallback);
+        ret = pwdCb(pwdBuf, (int)sizeof(pwdBuf), 0, pwdCallbackCtx);
+        if((ret < 0) || (ret >= (int)sizeof(pwdBuf))) {
+            xmlSecOpenSSLError("pwdCallback", NULL);
+            goto done;
+        }
+        pwdBuf[ret] = '\0';
+        pwd = pwdBuf;
+    }
+
+    pwdSize = (pwd != NULL) ? strlen(pwd) : 0;
+    XMLSEC_SAFE_CAST_SIZE_T_TO_INT(pwdSize, pwdLen, goto done, NULL);
+
     XMLSEC_OPENSSL_PUSH_LIB_CTX(goto done);
     ret = PKCS12_verify_mac(p12, pwd, ((pwd != NULL) ? pwdLen : 0));
     XMLSEC_OPENSSL_POP_LIB_CTX();
@@ -1450,6 +1466,7 @@ xmlSecOpenSSLAppPkcs12LoadBIO(BIO* bio, const char *pwd,
     /* success! */
 
 done:
+    OPENSSL_cleanse(pwdBuf, sizeof(pwdBuf));
     if(chain != NULL) {
         sk_X509_pop_free(chain, X509_free);
     }

src/openssl/crypto.c

@@ -767,18 +767,26 @@ xmlSecOpenSSLErrorsShutdown(void) {
  */
 int
 xmlSecOpenSSLSetDefaultTrustedCertsFolder(const xmlChar* path) {
-    if(gXmlSecOpenSSLTrustedCertsFolder != NULL) {
-        xmlFree(gXmlSecOpenSSLTrustedCertsFolder);
-        gXmlSecOpenSSLTrustedCertsFolder = NULL;
-    }
+    xmlChar* newPath;
 
+    /*
+     * Duplicate the new path first so that the previously set folder
+     * is preserved if the allocation fails.
+     */
     if(path != NULL) {
-        gXmlSecOpenSSLTrustedCertsFolder = xmlStrdup(BAD_CAST path);
-        if(gXmlSecOpenSSLTrustedCertsFolder == NULL) {
+        newPath = xmlStrdup(BAD_CAST path);
+        if(newPath == NULL) {
             xmlSecStrdupError(BAD_CAST path, NULL);
             return(-1);
         }
+    } else {
+        newPath = NULL;
+    }
+
+    if(gXmlSecOpenSSLTrustedCertsFolder != NULL) {
+        xmlFree(gXmlSecOpenSSLTrustedCertsFolder);
     }
+    gXmlSecOpenSSLTrustedCertsFolder = newPath;
 
     return(0);
 }

src/openssl/evp.c

@@ -336,6 +336,7 @@ xmlSecOpenSSLEvpKeyDataGetKeySize(xmlSecKeyDataPtr data) {
 #endif /* XMLSEC_NO_DSA */
 
 #ifndef XMLSEC_NO_DH
+    case EVP_PKEY_DH:
     case EVP_PKEY_DHX:
         {
             DH* dh = NULL;
@@ -502,6 +503,7 @@ xmlSecOpenSSLEvpKeyDataGetType(xmlSecKeyDataPtr data) {
 #endif /* XMLSEC_NO_DSA */
 
 #ifndef XMLSEC_NO_DH
+    case EVP_PKEY_DH:
     case EVP_PKEY_DHX:
         {
             DH* dh = NULL;
@@ -700,6 +702,7 @@ xmlSecOpenSSLEvpKeyGetKeyDataId(EVP_PKEY *pKey) {
 
     switch(xmlSecOpenSSLEvpKeyGetId(pKey)) {
 #ifndef XMLSEC_NO_DH
+    case EVP_PKEY_DH:
     case EVP_PKEY_DHX:
         return (xmlSecOpenSSLKeyDataDhId);
 #endif /* XMLSEC_NO_DH */
@@ -1710,6 +1713,8 @@ typedef struct _xmlSecOpenSSLKeyValueDh {
  *
  * The DH key type uses PKCS#3 format which saves p and g, but not the 'q' value. The DHX key
  * type uses X9.42 format which saves the value of 'q' and this must be used for FIPS186-4.
+ * Keys in both formats are accepted; the 'DHX' name is used to create new keys because it
+ * supports the 'q' value.
  */
 #define XMLSEC_OPENSSL_DH_EVP_NAME              "DHX"
 
@@ -1795,7 +1800,11 @@ int
 xmlSecOpenSSLKeyDataDhAdoptEvp(xmlSecKeyDataPtr data, EVP_PKEY* pKey) {
     xmlSecAssert2(xmlSecKeyDataCheckId(data, xmlSecOpenSSLKeyDataDhId), -1);
     xmlSecAssert2(pKey != NULL, -1);
-    xmlSecAssert2(EVP_PKEY_base_id(pKey) == EVP_PKEY_DHX, -1);
+    /*
+     * DH keys can have two types: EVP_PKEY_DH (PKCS#3 form, no 'q' value) and
+     * EVP_PKEY_DHX (X9.42 form, with 'q' value).
+     */
+    xmlSecAssert2((EVP_PKEY_base_id(pKey) == EVP_PKEY_DH) || (EVP_PKEY_base_id(pKey) == EVP_PKEY_DHX), -1);
 
     return(xmlSecOpenSSLEvpKeyDataAdoptEvp(data, pKey));
 }
@@ -1879,7 +1888,7 @@ xmlSecOpenSSLKeyDataDhGetDh(xmlSecKeyDataPtr data) {
     xmlSecAssert2(xmlSecKeyDataCheckId(data, xmlSecOpenSSLKeyDataDhId), NULL);
 
     pKey = xmlSecOpenSSLKeyDataDhGetEvp(data);
-    xmlSecAssert2((pKey == NULL) || (EVP_PKEY_base_id(pKey) == EVP_PKEY_DHX), NULL);
+    xmlSecAssert2((pKey == NULL) || (EVP_PKEY_base_id(pKey) == EVP_PKEY_DH) || (EVP_PKEY_base_id(pKey) == EVP_PKEY_DHX), NULL);
 
     return((pKey != NULL) ? EVP_PKEY_get0_DH(pKey) : NULL);
 }
@@ -2046,8 +2055,7 @@ xmlSecOpenSSLKeyDataDhGetValue(xmlSecKeyDataPtr data, xmlSecOpenSSLKeyValueDhPtr
     }
     ret = EVP_PKEY_get_bn_param(pKey, OSSL_PKEY_PARAM_FFC_Q, &(dhKeyValue->q));
     if((ret != 1) || (dhKeyValue->q == NULL)) {
-        xmlSecOpenSSLError("EVP_PKEY_get_bn_param(q)", xmlSecKeyDataGetName(data));
-        return(-1);
+        /* ignore the error since q is optional for DH keys in the PKCS#3 form */
     }
     ret = EVP_PKEY_get_bn_param(pKey, OSSL_PKEY_PARAM_FFC_G, &(dhKeyValue->generator));
     if((ret != 1) || (dhKeyValue->generator == NULL)) {
@@ -2839,6 +2847,7 @@ xmlSecOpenSSLKeyDataEcSetValue(xmlSecKeyDataPtr data, const xmlChar* curveOid, x
         xmlSecOpenSSLError("EC_POINT_new",  xmlSecKeyDataGetName(data));
         goto done;
     }
+    /* EC_POINT_oct2point() validates that the point is on the curve */
     ret = EC_POINT_oct2point(group, point, pubkeyData, pubkeySize, NULL);
     if(ret != 1) {
         xmlSecOpenSSLError("EC_POINT_oct2point",  xmlSecKeyDataGetName(data));

src/openssl/globals.h

@@ -21,6 +21,7 @@
 #include <stdint.h>
 
 #include <openssl/crypto.h>
+#include <openssl/err.h>
 
 #ifndef IN_XMLSEC_CRYPTO
 #define IN_XMLSEC_CRYPTO

src/openssl/hmac.c

@@ -458,6 +458,14 @@ xmlSecOpenSSLHmacVerify(xmlSecTransformPtr transform,
         return(-1);
     }
 
+    /* the digest to verify must not be empty */
+    if(dataSize <= 0) {
+        xmlSecInvalidSizeError("HMAC digest", dataSize,
+                               XMLSEC_TRANSFORM_HMAC_BITS_TO_BYTES(ctx->dgstSizeInBits),
+                               xmlSecTransformGetName(transform));
+        return(-1);
+    }
+
     /* Returns 1 for match, 0 for no match, <0 for errors. */
     ret = xmlSecTransformHmacVerify(data, dataSize, ctx->dgst, ctx->dgstSizeInBits, EVP_MAX_MD_SIZE);
     if(ret < 0) {

src/openssl/kdf.c

@@ -67,6 +67,7 @@ struct _xmlSecOpenSSLKdfCtx {
 
     xmlSecBuffer buffer;
     xmlSecBuffer buffer2;
+    xmlSecBuffer keyBuffer;
     unsigned int param1;
 };
 XMLSEC_TRANSFORM_DECLARE(OpenSSLKdf, xmlSecOpenSSLKdfCtx)
@@ -190,6 +191,14 @@ xmlSecOpenSSLKdfInitialize(xmlSecTransformPtr transform) {
     }
     ctx->buffer2.flags |= XMLSEC_BUFFER_FLAG_SECURE;
 
+    ret = xmlSecBufferInitialize(&(ctx->keyBuffer), 0);
+    if(ret < 0) {
+        xmlSecInternalError("xmlSecBufferInitialize(keyBuffer)", NULL);
+        xmlSecOpenSSLKdfFinalize(transform);
+        return(-1);
+    }
+    ctx->keyBuffer.flags |= XMLSEC_BUFFER_FLAG_SECURE;
+
     /* done */
     return(0);
 }
@@ -218,6 +227,7 @@ xmlSecOpenSSLKdfFinalize(xmlSecTransformPtr transform) {
 
     xmlSecBufferFinalize(&(ctx->buffer));
     xmlSecBufferFinalize(&(ctx->buffer2));
+    xmlSecBufferFinalize(&(ctx->keyBuffer));
 
     OPENSSL_cleanse(ctx, sizeof(xmlSecOpenSSLKdfCtx));
 }
@@ -247,6 +257,7 @@ xmlSecOpenSSLKdfSetKey(xmlSecTransformPtr transform, xmlSecKeyPtr key) {
     xmlSecBufferPtr buffer;
     xmlSecByte * keyData;
     xmlSecSize keySize;
+    int ret;
 
     xmlSecAssert2(xmlSecOpenSSLKdfCheckId(transform), -1);
     xmlSecAssert2(((transform->operation == xmlSecTransformOperationEncrypt) || (transform->operation == xmlSecTransformOperationDecrypt)), -1);
@@ -273,6 +284,16 @@ xmlSecOpenSSLKdfSetKey(xmlSecTransformPtr transform, xmlSecKeyPtr key) {
         return(-1);
     }
 
+    /* copy the key into a ctx-owned buffer so the OSSL_PARAM pointer remains
+     * valid until EVP_KDF_derive() is called */
+    ret = xmlSecBufferSetData(&(ctx->keyBuffer), keyData, keySize);
+    if(ret < 0) {
+        xmlSecInternalError("xmlSecBufferSetData(key)", xmlSecTransformGetName(transform));
+        return(-1);
+    }
+    keyData = xmlSecBufferGetData(&(ctx->keyBuffer));
+    xmlSecAssert2(keyData != NULL, -1);
+
     /* set key */
     if(ctx->paramsPos >= XMLSEC_OPENSSL_KDF_MAX_PARAMS) {
         xmlSecInvalidSizeDataError("Kdf Params Number", ctx->paramsPos, "too big", xmlSecTransformGetName(transform));

src/openssl/key_agrmnt.c

@@ -385,6 +385,11 @@ xmlSecOpenSSLKeyAgreementGenerateSecret(xmlSecOpenSSLKeyAgreementCtxPtr ctx, xml
         xmlSecOpenSSLError("EVP_PKEY_derive_init", NULL);
         goto done;
     }
+    /*
+     * EVP_PKEY_derive_set_peer() validates the peer key: it checks that the peer
+     * has the same key type and matching parameters (OpenSSL < 3.0), and performs
+     * a public key check (OpenSSL 3.0+).
+     */
     ret = EVP_PKEY_derive_set_peer(pKeyCtx, otherPubKey);
     if(ret != 1) {
         xmlSecOpenSSLError("EVP_PKEY_derive_set_peer", NULL);

src/openssl/kt_rsa.c

@@ -749,6 +749,7 @@ xmlSecOpenSSLRsaOaepProcessImpl(xmlSecOpenSSLRsaOaepCtxPtr ctx, const xmlSecByte
     } else {
         /* decrypt */
         BIGNUM * bn;
+        xmlSecSize outResSize;
         int outLen;
 
         ret = RSA_private_decrypt(inLen, inBuf, outBuf, rsa, RSA_NO_PADDING);
@@ -797,9 +798,19 @@ xmlSecOpenSSLRsaOaepProcessImpl(xmlSecOpenSSLRsaOaepCtxPtr ctx, const xmlSecByte
             OPENSSL_cleanse(outBuf, (*outSize));
             return(-1);
         }
+        XMLSEC_SAFE_CAST_INT_TO_SIZE(ret, outResSize, return(-1), NULL);
+
+        /*
+         * cleanse the tail of the output buffer that held the raw
+         * OAEP-encoded message; only the first 'ret' bytes contain the
+         * recovered plaintext
+         */
+        if((*outSize) > outResSize) {
+            OPENSSL_cleanse(outBuf + outResSize, ((*outSize) - outResSize));
+        }
 
         /* success */
-        XMLSEC_SAFE_CAST_INT_TO_SIZE(ret, (*outSize), return(-1), NULL);
+        (*outSize) = outResSize;
     }
 
     /* success */

src/openssl/symkeys.c

@@ -140,7 +140,7 @@ xmlSecOpenSSLSymKeyDataGenerate(xmlSecKeyDataPtr data, xmlSecSize sizeBits, xmlS
     buffer = xmlSecKeyDataBinaryValueGetBuffer(data);
     xmlSecAssert2(buffer != NULL, -1);
 
-    size = (sizeBits + 7) / 8;
+    size = (sizeBits - 1) / 8 + 1;
     ret = xmlSecBufferSetSize(buffer, size);
     if(ret < 0) {
         xmlSecInternalError2("xmlSecBufferSetSize", NULL, "size=" XMLSEC_SIZE_FMT, size);