diff --git a/src/gcrypt/app.c b/src/gcrypt/app.c index 139a796e9..efe6312f7 100644 --- a/src/gcrypt/app.c +++ b/src/gcrypt/app.c @@ -27,6 +27,11 @@ #include "asn1.h" #include "../cast_helpers.h" +/* the flag that indicates whether the secure memory was initialized + (GCRYCTL_INIT_SECMEM can fail; the error is ignored because of + a known libgcrypt bug, see https://github.com/lsh123/xmlsec/issues/415) */ +static int g_xmlSecGCryptSecureMemoryInitialized = 0; + /** * @brief Initializes the GCrypt crypto engine. * @details General crypto engine initialization. This function is used @@ -95,6 +100,9 @@ Noteworthy changes in version 1.4.3 (2008-09-18) xmlSecGCryptError("gcry_control(GCRYCTL_INIT_SECMEM)", err, NULL); /* ignore this error because of libgcrypt bug in allocating memory, see https://github.com/lsh123/xmlsec/issues/415 for more details */ + g_xmlSecGCryptSecureMemoryInitialized = 0; + } else { + g_xmlSecGCryptSecureMemoryInitialized = 1; } /* It is now okay to let Libgcrypt complain when there was/is @@ -130,10 +138,16 @@ int xmlSecGCryptAppShutdown(void) { gcry_error_t err; - err = gcry_control(GCRYCTL_TERM_SECMEM); - if(err != GPG_ERR_NO_ERROR) { - xmlSecGCryptError("gcry_control(GCRYCTL_TERM_SECMEM)", err, NULL); - return(-1); + /* only terminate the secure memory if it was actually initialized; + GCRYCTL_TERM_SECMEM can fail (e.g. GPG_ERR_CONFLICT) when the + secure memory was never allocated (e.g. because the ignored + GCRYCTL_INIT_SECMEM failure happened during initialization) */ + if(g_xmlSecGCryptSecureMemoryInitialized) { + err = gcry_control(GCRYCTL_TERM_SECMEM); + if(err != GPG_ERR_NO_ERROR) { + xmlSecGCryptError("gcry_control(GCRYCTL_TERM_SECMEM)", err, NULL); + return(-1); + } } /* done */ diff --git a/src/gcrypt/asn1.c b/src/gcrypt/asn1.c index 9344ed463..ce74fc0de 100644 --- a/src/gcrypt/asn1.c +++ b/src/gcrypt/asn1.c @@ -328,7 +328,15 @@ xmlSecGCryptAsn1ParseIntegerSequence(int level, xmlSecByte const **buffer, xmlSe /* expect an INTEGER tag */ if((remaining < 1) || (*p != TAG_INTEGER)) { - xmlSecInternalError2("xmlSecGCryptAsn1ParseIntegerSequence", NULL, "INTEGER expected inside BIT STRING, remaining=%lu", remaining); + if((remaining >= 1) && (*p == 0x30)) { + /* a SEQUENCE inside the BIT STRING indicates an + unsupported key format, e.g. an RSA key in + SPKI/SubjectPublicKeyInfo form; only the + traditional (PKCS#1) RSA formats are supported */ + xmlSecInvalidDataError("unsupported key format: a SEQUENCE was found inside a BIT STRING (e.g. an RSA key in SPKI/SubjectPublicKeyInfo form); use the traditional PKCS#1 format instead", NULL); + } else { + xmlSecInternalError2("xmlSecGCryptAsn1ParseIntegerSequence", NULL, "INTEGER expected inside BIT STRING, remaining=%lu", remaining); + } return(-1); } p++; remaining--; @@ -436,13 +444,12 @@ xmlSecGCryptAsn1GuessKeyType(gcry_mpi_t * integers, xmlSecSize integers_num, xml } /* try other keys */ - /* Note: this guessing is inherently heuristic. A malformed key (for example, - * an EC private key missing both its curve OID and its public key, which - * flattens to exactly two integers with no OIDs) can be misidentified as a - * different type (here, an RSA public key with a garbage modulus of 0 or 1). - * Such a misidentified key is invalid and is rejected by libgcrypt on use, so - * this cannot lead to a verification bypass; it only affects how malformed - * input is reported. This is therefore not a security defect. */ + /* Note: this guessing is inherently heuristic. An RSA public key is only + * recognized when the first integer (the modulus) is at least 512 bits long; + * a 2-integer key with a small first integer (for example, an EC private key + * missing both its curve OID and its optional public point, which flattens to + * version + d) is not treated as an RSA key and is rejected with a + * "number of parameters" error below instead of a garbage key being built. */ switch(integers_num) { case XMLSEC_GCRYPT_ASN1_DSA_PUB_NUM: return(xmlSecGCryptDerKeyTypePublicDsa); @@ -450,7 +457,14 @@ xmlSecGCryptAsn1GuessKeyType(gcry_mpi_t * integers, xmlSecSize integers_num, xml return(xmlSecGCryptDerKeyTypePrivateDsa); case XMLSEC_GCRYPT_ASN1_RSA_PUB_NUM: - return(xmlSecGCryptDerKeyTypePublicRsa); + /* a real RSA modulus is at least 512 bits long; 512 is a heuristic + threshold (libgcrypt itself accepts smaller moduli), used to tell a + valid RSAPublicKey apart from other 2-integer shapes, e.g. an EC + private key (version + d) missing its curve OID */ + if(gcry_mpi_get_nbits(integers[0]) >= 512) { + return(xmlSecGCryptDerKeyTypePublicRsa); + } + return(xmlSecGCryptDerKeyTypeAuto); case XMLSEC_GCRYPT_ASN1_RSA_PRIV_NUM: return(xmlSecGCryptDerKeyTypePrivateRsa); default: @@ -516,10 +530,16 @@ xmlSecGCryptParseDer(const xmlSecByte * der, xmlSecSize derlen, } /* PKCS#8-wrapped private keys (PrivateKeyInfo) are not supported and would be - * misparsed into a garbage key: they flatten to exactly two integers - * [version(0), ] with an algorithm object id present. Detect this - * shape and fail instead of building a wrong key. */ - if((integers_num == 2) && (objectids_num >= 1) && (gcry_mpi_get_nbits(integers[0]) == 0)) { + * misparsed into a garbage key: they always flatten to at least two integers + * starting with version(0) and carry an algorithm object id. For a PKCS#8 RSA + * key the shape is exactly two integers [version(0), ], but a + * PKCS#8 DSA key flattens to more (the DSA parameters p, q, g are collected + * from the algorithm-params SEQUENCE), so detect any such shape (version(0) + + * >=1 more integer + an OID present) and fail instead of building a wrong key. + * No supported traditional format matches this: PKCS#1/traditional DSA/EC keys + * carry no OIDs, a SPKI DSA public key has integers[0]=p (non-zero), and a SPKI + * EC public key has exactly one integer. */ + if((integers_num >= 2) && (objectids_num >= 1) && (gcry_mpi_get_nbits(integers[0]) == 0)) { xmlSecInvalidDataError("PKCS#8 private keys are not supported; use a traditional format (PKCS#1 RSAPrivateKey, DSAPrivateKey, or ECPrivateKey)", NULL); goto done; } diff --git a/src/gcrypt/asymkeys.c b/src/gcrypt/asymkeys.c index 530e752eb..eed111be2 100644 --- a/src/gcrypt/asymkeys.c +++ b/src/gcrypt/asymkeys.c @@ -1611,6 +1611,37 @@ xmlSecGCryptKeyDataEcGetKlass(void) { return(&xmlSecGCryptKeyDataEcKlass); } +/** + * @brief Checks that a GCrypt key S-expression is an EC key. + * @details Accepts both the "ecdsa" and the "ecc" algorithm token: "ecdsa" is + * the token used in the S-expressions constructed by this back-end, while "ecc" + * is the token that libgcrypt itself uses in the EC keys it creates + * (e.g. with gcry_pk_genkey). + * @param key the pointer to the GCrypt key S-expression (public key, private key, or key pair). + * @return 0 if the key is an EC key or -1 otherwise. + */ +static int +xmlSecGCryptKeyDataEcCheckKeyAlg(gcry_sexp_t key) { + gcry_sexp_t tok; + + xmlSecAssert2(key != NULL, -1); + + /* the algorithm token (ecdsa/ecc) is present in the public-key, + private-key and key-pair S-expressions alike, so its presence + unambiguously identifies the key type */ + tok = gcry_sexp_find_token(key, "ecdsa", 0); + if(tok == NULL) { + tok = gcry_sexp_find_token(key, "ecc", 0); + } + if(tok == NULL) { + return(-1); + } + gcry_sexp_release(tok); + + /* success */ + return(0); +} + /** * @brief Sets the value of EC key data. * @details On success, @p ec_key will be owned by the @p data; on failure the @@ -1624,7 +1655,7 @@ xmlSecGCryptKeyDataEcAdoptKey(xmlSecKeyDataPtr data, gcry_sexp_t ec_key) { xmlSecAssert2(xmlSecKeyDataCheckId(data, xmlSecGCryptKeyDataEcId), -1); xmlSecAssert2(ec_key != NULL, -1); - if(xmlSecGCryptAsymKeyDataCheckKeyAlg(ec_key, "ecdsa") < 0) { + if(xmlSecGCryptKeyDataEcCheckKeyAlg(ec_key) < 0) { xmlSecInvalidDataError("the provided key is not an EC key", xmlSecKeyDataGetName(data)); return(-1); } @@ -1646,11 +1677,11 @@ xmlSecGCryptKeyDataEcAdoptKeyPair(xmlSecKeyDataPtr data, gcry_sexp_t pub_key, gc xmlSecAssert2(xmlSecKeyDataCheckId(data, xmlSecGCryptKeyDataEcId), -1); xmlSecAssert2(pub_key != NULL, -1); - if(xmlSecGCryptAsymKeyDataCheckKeyAlg(pub_key, "ecdsa") < 0) { + if(xmlSecGCryptKeyDataEcCheckKeyAlg(pub_key) < 0) { xmlSecInvalidDataError("the provided key is not an EC key", xmlSecKeyDataGetName(data)); return(-1); } - if((priv_key != NULL) && (xmlSecGCryptAsymKeyDataCheckKeyAlg(priv_key, "ecdsa") < 0)) { + if((priv_key != NULL) && (xmlSecGCryptKeyDataEcCheckKeyAlg(priv_key) < 0)) { xmlSecInvalidDataError("the provided key is not an EC key", xmlSecKeyDataGetName(data)); return(-1); } @@ -1778,12 +1809,33 @@ typedef struct _xmlSecGCryptKeyDataEcCurveOidToName { xmlChar curveOid[64]; } xmlSecGCryptKeyDataEcCurveOidToName; +/* The table contains both the curve names accepted by libgcrypt in the + S-expressions (secpNnnr1, primeNnnv1, secp256k1, brainpoolPNNNr1) and the + canonical names that libgcrypt itself stores in the S-expressions of the + keys it creates (e.g. "NIST P-256" for secp256r1/prime256v1), so that both + the keys constructed by this back-end and the libgcrypt-created keys can be + written to XML. */ static xmlSecGCryptKeyDataEcCurveOidToName g_xmlSecGCryptKeyDataEcCurveOidToName[] = { - { "prime192v1", "1.2.840.10045.3.1.1" }, - { "prime256v1", "1.2.840.10045.3.1.7" }, - { "secp224r1", "1.3.132.0.33" }, - { "secp384r1", "1.3.132.0.34" }, - { "secp521r1", "1.3.132.0.35" } + { "prime192v1", "1.2.840.10045.3.1.1" }, + { "prime256v1", "1.2.840.10045.3.1.7" }, + { "secp192r1", "1.3.132.0.32" }, + { "secp224r1", "1.3.132.0.33" }, + { "secp256r1", "1.2.840.10045.3.1.7" }, + { "secp384r1", "1.3.132.0.34" }, + { "secp521r1", "1.3.132.0.35" }, + { "secp256k1", "1.3.132.0.10" }, + { "brainpoolP160r1", "1.3.36.3.3.2.8.1.1.1" }, + { "brainpoolP192r1", "1.3.36.3.3.2.8.1.1.3" }, + { "brainpoolP224r1", "1.3.36.3.3.2.8.1.1.5" }, + { "brainpoolP256r1", "1.3.36.3.3.2.8.1.1.7" }, + { "brainpoolP320r1", "1.3.36.3.3.2.8.1.1.9" }, + { "brainpoolP384r1", "1.3.36.3.3.2.8.1.1.11" }, + { "brainpoolP512r1", "1.3.36.3.3.2.8.1.1.13" }, + { "NIST P-192", "1.2.840.10045.3.1.1" }, + { "NIST P-224", "1.3.132.0.33" }, + { "NIST P-256", "1.2.840.10045.3.1.7" }, + { "NIST P-384", "1.3.132.0.34" }, + { "NIST P-521", "1.3.132.0.35" } }; static const char* @@ -1834,11 +1886,17 @@ xmlSecGCryptKeyDataEcRead(xmlSecKeyDataId id, xmlSecKeyValueEcPtr ecValue) { goto done; } - /* pubkey */ - if(xmlSecBufferGetSize(&(ecValue->pubkey)) == 0) { - xmlSecInvalidZeroKeyDataSizeError(xmlSecKeyDataKlassGetName(id)); + /* check that the public key point is well-formed (odd size > 1 and the + leading uncompressed point magic byte 0x04); note that the point cannot + be validated to be on the curve, because libgcrypt does not expose the + curve parameters for that */ + ret = xmlSecKeyDataEcPublicKeySplitComponents(ecValue); + if(ret < 0) { + xmlSecInternalError("xmlSecKeyDataEcPublicKeySplitComponents", xmlSecKeyDataKlassGetName(id)); goto done; } + + /* pubkey */ err = gcry_mpi_scan(&pubkey, GCRYMPI_FMT_USG, xmlSecBufferGetData(&(ecValue->pubkey)), xmlSecBufferGetSize(&(ecValue->pubkey)), NULL); @@ -1946,9 +2004,14 @@ xmlSecGCryptKeyDataEcWrite(xmlSecKeyDataId id, xmlSecKeyDataPtr data, xmlSecKeyV goto done; } + /* the algorithm token is "ecdsa" for the S-expressions constructed by this + back-end and "ecc" for the keys created by libgcrypt itself */ s_ecdsa = gcry_sexp_find_token(s_pub_key, "ecdsa", 0); if(s_ecdsa == NULL) { - xmlSecGCryptError("gcry_sexp_find_token(ecdsa)", (gcry_error_t)GPG_ERR_NO_ERROR, + s_ecdsa = gcry_sexp_find_token(s_pub_key, "ecc", 0); + } + if(s_ecdsa == NULL) { + xmlSecGCryptError("gcry_sexp_find_token(ecdsa/ecc)", (gcry_error_t)GPG_ERR_NO_ERROR, xmlSecKeyDataKlassGetName(id)); goto done; } diff --git a/src/gcrypt/ciphers.c b/src/gcrypt/ciphers.c index 01145cfeb..2e05391ef 100644 --- a/src/gcrypt/ciphers.c +++ b/src/gcrypt/ciphers.c @@ -172,11 +172,12 @@ xmlSecGCryptBlockCipherCtxUpdate(xmlSecGCryptBlockCipherCtxPtr ctx, } inSize = inBlocks * blockSize; - /* we write out the input size plus maybe one block. - * - * The size_t sum (outSize + inSize + blockSize) could in principle wrap on a - * 32-bit build, but only for multi-gigabyte buffers, which is not a realistic - * input size for XML Security processing; this matches the other backends. */ + /* we write out the input size plus maybe one block */ + if((inSize > XMLSEC_SIZE_MAX - blockSize) || (outSize > XMLSEC_SIZE_MAX - inSize - blockSize)) { + xmlSecInternalError4("xmlSecBufferSetMaxSize", cipherName, + "outSize=" XMLSEC_SIZE_FMT "; inSize=" XMLSEC_SIZE_FMT "; blockSize=" XMLSEC_SIZE_FMT, outSize, inSize, blockSize); + return(-1); + } ret = xmlSecBufferSetMaxSize(out, outSize + inSize + blockSize); if(ret < 0) { xmlSecInternalError2("xmlSecBufferSetMaxSize", cipherName, @@ -273,9 +274,12 @@ xmlSecGCryptBlockCipherCtxFinal(xmlSecGCryptBlockCipherCtxPtr ctx, } } - /* process last block. The size_t sum (outSize + 2 * blockSize) could in - * principle wrap on a 32-bit build, but only for multi-gigabyte buffers, - * which is not a realistic input size. */ + /* process last block */ + if((blockSize > (XMLSEC_SIZE_MAX / 2)) || (outSize > XMLSEC_SIZE_MAX - 2 * blockSize)) { + xmlSecInternalError3("xmlSecBufferSetMaxSize", cipherName, + "outSize=" XMLSEC_SIZE_FMT "; blockSize=" XMLSEC_SIZE_FMT, outSize, blockSize); + return(-1); + } ret = xmlSecBufferSetMaxSize(out, outSize + 2 * blockSize); if(ret < 0) { xmlSecInternalError2("xmlSecBufferSetMaxSize", cipherName, diff --git a/src/gcrypt/crypto.c b/src/gcrypt/crypto.c index 98ecc7d18..eef09c87b 100644 --- a/src/gcrypt/crypto.c +++ b/src/gcrypt/crypto.c @@ -309,12 +309,19 @@ xmlSecCryptoGetFunctions_gcrypt(void) { */ int xmlSecGCryptInit (void) { - /* Note: this function does not perform explicit libgcrypt initialization - * (no gcry_check_version/gcry_control/gcry_init). The full libgcrypt - * initialization is performed by xmlSecGCryptAppInit (src/gcrypt/app.c), - * which the application invokes through xmlSecCryptoAppInit before calling - * xmlSecInit. Applications that skip the app-init step rely on libgcrypt's - * self-initialization, which was added in libgcrypt 1.4.3. */ + /* Note: the full libgcrypt initialization (secure memory, etc.) is + * performed by xmlSecGCryptAppInit (src/gcrypt/app.c), which the + * application invokes through xmlSecCryptoAppInit before calling + * xmlSecInit. Applications that skip the app-init step rely on + * libgcrypt's self-initialization, which was added in libgcrypt 1.4.3; + * the version check below still makes sure that the linked libgcrypt is + * at least GCRYPT_MIN_VERSION (configure.ac defines GCRYPT_MIN_VERSION) + * and triggers the basic libgcrypt initialization. */ + if(gcry_check_version(GCRYPT_MIN_VERSION) == NULL) { + xmlSecOtherError2(XMLSEC_ERRORS_R_CRYPTO_FAILED, NULL, + "gcry_check_version failed; min_version=%s", GCRYPT_MIN_VERSION); + return(-1); + } /* Check loaded xmlsec library version */ if(xmlSecCheckVersionExact() != 1) { diff --git a/src/gcrypt/digests.c b/src/gcrypt/digests.c index daa07d080..f8c553989 100644 --- a/src/gcrypt/digests.c +++ b/src/gcrypt/digests.c @@ -281,9 +281,8 @@ xmlSecGCryptDigestExecute(xmlSecTransformPtr transform, int last, xmlSecTransfor inSize = xmlSecBufferGetSize(in); if(inSize > 0) { - /* The gcry_md_write() return value is not checked: given the validated - * context handle it effectively cannot fail. This is a codebase-wide - * pattern in the gcrypt backend. */ + /* gcry_md_write() returns void (no error code), so there is + * nothing to check. */ gcry_md_write(ctx->digestCtx, xmlSecBufferGetData(in), inSize); ret = xmlSecBufferRemoveHead(in, inSize); diff --git a/src/gcrypt/hmac.c b/src/gcrypt/hmac.c index 39ab434e1..7171ef2e4 100644 --- a/src/gcrypt/hmac.c +++ b/src/gcrypt/hmac.c @@ -310,6 +310,14 @@ xmlSecGCryptHmacVerify(xmlSecTransformPtr transform, xmlSecAssert2(ctx->digestCtx != NULL, -1); xmlSecAssert2(ctx->dgstSizeInBits > 0, -1); + /* the digest to verify must not be empty */ + if(dataSize <= 0) { + xmlSecInvalidSizeError("HMAC digest", dataSize, + XMLSEC_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, sizeof(ctx->dgst)); if(ret < 0) { @@ -354,10 +362,8 @@ xmlSecGCryptHmacExecute(xmlSecTransformPtr transform, int last, xmlSecTransformC inSize = xmlSecBufferGetSize(in); if(inSize > 0) { - /* The gcry_md_write() return value is not checked: given the validated - * context handle it effectively cannot fail (if it ever did, the input - * chunk would be silently omitted from the HMAC). This is a codebase-wide - * pattern in the gcrypt backend. */ + /* gcry_md_write() returns void (no error code), so there is + * nothing to check. */ gcry_md_write(ctx->digestCtx, xmlSecBufferGetData(in), inSize); ret = xmlSecBufferRemoveHead(in, inSize); diff --git a/src/gcrypt/kt_rsa.c b/src/gcrypt/kt_rsa.c index 673092a00..31ee3b8af 100644 --- a/src/gcrypt/kt_rsa.c +++ b/src/gcrypt/kt_rsa.c @@ -321,6 +321,7 @@ static int xmlSecGCryptRsaPkcs1Encrypt(xmlSecGCryptRsaPkcs1CtxPtr ctx, xmlSecBufferPtr in, xmlSecBufferPtr out) { xmlSecSize inSize; int inLen; + gcry_sexp_t s_pub_key; gcry_sexp_t s_plaintext_data = NULL; gpg_error_t err; int ret; @@ -344,10 +345,16 @@ xmlSecGCryptRsaPkcs1Encrypt(xmlSecGCryptRsaPkcs1CtxPtr ctx, xmlSecBufferPtr in, goto done; } + s_pub_key = xmlSecGCryptKeyDataRsaGetPublicKey(ctx->keyData); + if(s_pub_key == NULL) { + xmlSecInternalError("xmlSecGCryptKeyDataRsaGetPublicKey", NULL); + goto done; + } + /* encrypt */ ret = xmlSecGCryptRsaKtEncrypt( s_plaintext_data, - xmlSecGCryptKeyDataRsaGetPublicKey(ctx->keyData), + s_pub_key, out); if(ret != 0) { xmlSecInternalError("xmlSecGCryptRsaKtEncrypt", NULL); @@ -901,6 +908,7 @@ static int xmlSecGCryptRsaOaepEncrypt(xmlSecGCryptRsaOaepCtxPtr ctx, xmlSecBufferPtr in, xmlSecBufferPtr out) { xmlSecSize inSize, oaepParamSize; int inLen, oaepParamLen; + gcry_sexp_t s_pub_key; gcry_sexp_t s_plaintext_data = NULL; gpg_error_t err; int ret; @@ -948,10 +956,16 @@ xmlSecGCryptRsaOaepEncrypt(xmlSecGCryptRsaOaepCtxPtr ctx, xmlSecBufferPtr in, xm goto done; } + s_pub_key = xmlSecGCryptKeyDataRsaGetPublicKey(ctx->keyData); + if(s_pub_key == NULL) { + xmlSecInternalError("xmlSecGCryptKeyDataRsaGetPublicKey", NULL); + goto done; + } + /* encrypt */ ret = xmlSecGCryptRsaKtEncrypt( s_plaintext_data, - xmlSecGCryptKeyDataRsaGetPublicKey(ctx->keyData), + s_pub_key, out); if(ret != 0) { xmlSecInternalError("xmlSecGCryptRsaKtEncrypt", NULL); @@ -982,7 +996,11 @@ xmlSecGCryptRsaOaepEncrypt(xmlSecGCryptRsaOaepCtxPtr ctx, xmlSecBufferPtr in, xm static int xmlSecGCryptRsaOaepDecrypt(xmlSecGCryptRsaOaepCtxPtr ctx, xmlSecBufferPtr in, xmlSecBufferPtr out) { xmlSecSize inSize, oaepParamSize; + xmlSecSize modulusSize = 0; + const void *modulusData; int inLen, oaepParamLen; + gcry_sexp_t s_priv_key; + gcry_sexp_t s_modulus = NULL; gcry_sexp_t s_encrypted_data = NULL; gpg_error_t err; int ret; @@ -1007,6 +1025,36 @@ xmlSecGCryptRsaOaepDecrypt(xmlSecGCryptRsaOaepCtxPtr ctx, xmlSecBufferPtr in, xm inSize = xmlSecBufferGetSize(in); XMLSEC_SAFE_CAST_SIZE_TO_INT(inSize, inLen, return(-1), NULL); + /* verify the input size: an RSA OAEP ciphertext must be exactly + * the size of the RSA modulus */ + s_priv_key = xmlSecGCryptKeyDataRsaGetPrivateKey(ctx->keyData); + if(s_priv_key == NULL) { + xmlSecInternalError("xmlSecGCryptKeyDataRsaGetPrivateKey", NULL); + return(-1); + } + s_modulus = gcry_sexp_find_token(s_priv_key, "n", 0); + if(s_modulus == NULL) { + xmlSecGCryptError2("gcry_sexp_find_token()", (gcry_error_t)GPG_ERR_NO_ERROR, NULL, + "name=%s", "n"); + return(-1); + } + modulusData = gcry_sexp_nth_data(s_modulus, 1, &modulusSize); + if(modulusData == NULL) { + xmlSecGCryptError("gcry_sexp_nth_data()", (gcry_error_t)GPG_ERR_NO_ERROR, NULL); + goto done; + } + /* libgcrypt may prepend a leading 0x00 byte to positive integers; strip + * it so the size matches the actual modulus size */ + if((modulusSize > 0) && (((const xmlSecByte*)modulusData)[0] == 0x00)) { + modulusSize--; + } + gcry_sexp_release(s_modulus); + s_modulus = NULL; + if(inSize != modulusSize) { + xmlSecInvalidSizeError("Input data", inSize, modulusSize, NULL); + goto done; + } + oaepParamSize = xmlSecBufferGetSize(&(ctx->oaepParams)); XMLSEC_SAFE_CAST_SIZE_TO_INT(oaepParamSize, oaepParamLen, return(-1), NULL); @@ -1033,7 +1081,7 @@ xmlSecGCryptRsaOaepDecrypt(xmlSecGCryptRsaOaepCtxPtr ctx, xmlSecBufferPtr in, xm /* decrypt */ ret = xmlSecGCryptRsaKtDecrypt( s_encrypted_data, - xmlSecGCryptKeyDataRsaGetPrivateKey(ctx->keyData), + s_priv_key, out); if(ret != 0) { xmlSecInternalError("xmlSecGCryptRsaKtDecrypt", NULL); @@ -1053,6 +1101,9 @@ xmlSecGCryptRsaOaepDecrypt(xmlSecGCryptRsaOaepCtxPtr ctx, xmlSecBufferPtr in, xm done: /* cleanup */ + if(s_modulus != NULL) { + gcry_sexp_release(s_modulus); + } if(s_encrypted_data != NULL) { gcry_sexp_release(s_encrypted_data); } diff --git a/src/gcrypt/signatures.c b/src/gcrypt/signatures.c index b46400996..764cf28ea 100644 --- a/src/gcrypt/signatures.c +++ b/src/gcrypt/signatures.c @@ -977,7 +977,14 @@ xmlSecGCryptDsaVerify(int digest XMLSEC_ATTRIBUTE_UNUSED, xmlSecKeyDataPtr key_d xmlSecAssert2(dgst != NULL, -1); xmlSecAssert2(dgstSize > 0, -1); xmlSecAssert2(data != NULL, -1); - xmlSecAssert2(dataSize == (XMLSEC_GCRYPT_DSA_SIG_SIZE + XMLSEC_GCRYPT_DSA_SIG_SIZE), -1); + + /* check signature size: a DSA signature is two fixed-size components (r and s) */ + if(dataSize != (XMLSEC_GCRYPT_DSA_SIG_SIZE + XMLSEC_GCRYPT_DSA_SIG_SIZE)) { + xmlSecInternalError3("Invalid signature size", NULL, + "actual=" XMLSEC_SIZE_FMT "; expected=" XMLSEC_SIZE_FMT, dataSize, + (xmlSecSize)(XMLSEC_GCRYPT_DSA_SIG_SIZE + XMLSEC_GCRYPT_DSA_SIG_SIZE)); + goto done; + } s_key = xmlSecGCryptKeyDataDsaGetPublicKey(key_data); xmlSecAssert2(s_key != NULL, -1); diff --git a/src/gcrypt/symkeys.c b/src/gcrypt/symkeys.c index ec74af3e7..c34813cf2 100644 --- a/src/gcrypt/symkeys.c +++ b/src/gcrypt/symkeys.c @@ -136,6 +136,10 @@ xmlSecGCryptSymKeyDataGenerate(xmlSecKeyDataPtr data, xmlSecSize sizeBits, xmlSe xmlSecAssert2(xmlSecGCryptSymKeyDataCheckId(data), -1); xmlSecAssert2(sizeBits > 0, -1); + if(sizeBits > (XMLSEC_SIZE_MAX - 7)) { + xmlSecInvalidSizeMoreThanError("sizeBits", sizeBits, (XMLSEC_SIZE_MAX - 7), NULL); + return(-1); + } buffer = xmlSecKeyDataBinaryValueGetBuffer(data); xmlSecAssert2(buffer != NULL, -1);