From 96a9380208566d960fe88262ab727615bdd883ad Mon Sep 17 00:00:00 2001 From: Moonchild Date: Wed, 14 Jan 2026 22:30:19 +0100 Subject: [PATCH] [NSS] Update NSS --- security/nss/cpputil/nss_scoped_ptrs.h | 11 ++++ security/nss/gtests/der_gtest/der_gtest.gyp | 1 + .../gtests/der_gtest/p12_import_unittest.cc | 23 +++++++ .../gtests/der_gtest/p7_import_unittest.cc | 60 +++++++++++++++++ security/nss/lib/pk11wrap/pk11mech.c | 10 +++ security/nss/lib/pkcs7/p7decode.c | 65 ++++++++++--------- security/nss/lib/util/pkcs11t.h | 6 ++ 7 files changed, 144 insertions(+), 32 deletions(-) create mode 100644 security/nss/gtests/der_gtest/p7_import_unittest.cc diff --git a/security/nss/cpputil/nss_scoped_ptrs.h b/security/nss/cpputil/nss_scoped_ptrs.h index 9cb4478bd7..90c7711d2b 100644 --- a/security/nss/cpputil/nss_scoped_ptrs.h +++ b/security/nss/cpputil/nss_scoped_ptrs.h @@ -49,6 +49,15 @@ struct ScopedDelete { void operator()(SEC_PKCS12DecoderContext* dcx) { SEC_PKCS12DecoderFinish(dcx); } + void operator()(SEC_PKCS7DecoderContext* dcx) { + SEC_PKCS7ContentInfo* cinfo = SEC_PKCS7DecoderFinish(dcx); + if (cinfo) { + SEC_PKCS7DestroyContentInfo(cinfo); + } + } + void operator()(SEC_PKCS7ContentInfo* cinfo) { + SEC_PKCS7DestroyContentInfo(cinfo); + } }; template @@ -86,6 +95,8 @@ SCOPED(SECKEYPrivateKeyList); SCOPED(SECKEYPublicKey); SCOPED(SECMODModule); SCOPED(SEC_PKCS12DecoderContext); +SCOPED(SEC_PKCS7DecoderContext); +SCOPED(SEC_PKCS7ContentInfo); #undef SCOPED diff --git a/security/nss/gtests/der_gtest/der_gtest.gyp b/security/nss/gtests/der_gtest/der_gtest.gyp index 2df9242d35..cb3ae489cb 100644 --- a/security/nss/gtests/der_gtest/der_gtest.gyp +++ b/security/nss/gtests/der_gtest/der_gtest.gyp @@ -14,6 +14,7 @@ 'der_getint_unittest.cc', 'der_quickder_unittest.cc', 'p12_import_unittest.cc', + 'p7_import_unittest.cc', '<(DEPTH)/gtests/common/gtests.cc' ], 'dependencies': [ diff --git a/security/nss/gtests/der_gtest/p12_import_unittest.cc b/security/nss/gtests/der_gtest/p12_import_unittest.cc index 1a50f99349..ab0d20da3f 100644 --- a/security/nss/gtests/der_gtest/p12_import_unittest.cc +++ b/security/nss/gtests/der_gtest/p12_import_unittest.cc @@ -230,6 +230,13 @@ static const uint8_t cert_p12[] = { 0x51, 0x04, 0x08, 0xa1, 0x52, 0xdd, 0x64, 0x46, 0xe9, 0x9e, 0x3e, 0x02, 0x02, 0x08, 0x00}; +unsigned char leak_p12[] = { + 0x30, 0x82, 0x20, 0x20, 0x02, 0x01, 0xff, 0x30, 0x82, 0x09, 0x20, 0x06, + 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x02, 0xa0, 0x50, + 0x30, 0x3f, 0x02, 0x01, 0x20, 0x31, 0x0d, 0x30, 0x0b, 0x06, 0x09, 0x60, + 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x04, 0x30, 0x20, 0x06, 0x09, + 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x01}; + class PK12ImportTest : public ::testing::Test {}; TEST_F(PK12ImportTest, ImportPK12With2P7) { @@ -247,4 +254,20 @@ TEST_F(PK12ImportTest, ImportPK12With2P7) { ASSERT_EQ(SECFailure, rv); } +TEST_F(PK12ImportTest, FailsToImportButShouldNotLeak) { + SECItem password = {siBuffer, nullptr, 0}; + ScopedPK11SlotInfo slot(PK11_GetInternalSlot()); + ScopedSEC_PKCS12DecoderContext dcx( + SEC_PKCS12DecoderStart(&password, slot.get(), nullptr, nullptr, nullptr, + nullptr, nullptr, nullptr)); + ASSERT_TRUE(dcx); + SECStatus rv = SEC_PKCS12DecoderUpdate( + dcx.get(), const_cast(leak_p12), sizeof(leak_p12)); + ASSERT_EQ(SECSuccess, rv); + rv = SEC_PKCS12DecoderVerify(dcx.get()); + // This is not a valid PKCS12 file, so a failing return value is expected. + // However, the implementation shouldn't leak memory as a result. + ASSERT_EQ(SECFailure, rv); +} + } // namespace nss_test diff --git a/security/nss/gtests/der_gtest/p7_import_unittest.cc b/security/nss/gtests/der_gtest/p7_import_unittest.cc new file mode 100644 index 0000000000..19348587f2 --- /dev/null +++ b/security/nss/gtests/der_gtest/p7_import_unittest.cc @@ -0,0 +1,60 @@ ++/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ ++/* vim: set ts=2 et sw=2 tw=80: */ ++/* This Source Code Form is subject to the terms of the Mozilla Public ++ * License, v. 2.0. If a copy of the MPL was not distributed with this file, ++ * You can obtain one at http://mozilla.org/MPL/2.0/. */ ++ ++#include "nss.h" ++#include "secpkcs7.h" ++ ++#include "gtest/gtest.h" +#include "nss_scoped_ptrs.h" + +namespace nss_test { + +// This is an invalid PKCS7 message. Among other things, it contains some +// unknown hash OIDs. This should fail to parse, but it should be safe to try. +static const uint8_t p7_with_unknown_hashes[] = { + 0x30, 0x4d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, + 0x02, 0xa0, 0x40, 0x30, 0x3e, 0x02, 0x01, 0x20, 0x31, 0x27, 0x30, 0x0b, + 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x05, 0x30, + 0x0b, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x05, + 0x30, 0x0b, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, + 0x04, 0x30, 0x10, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, + 0x07, 0x01, 0xa0, 0x03, 0x04, 0x01, 0x00}; + +// This is an invalid PKCS7 message. It contains multiple hash OIDs (that's not +// what makes it invalid). When it fails to parse, the associated digest data +// structures should be freed correctly. +static const uint8_t p7_with_multiple_hashes[] = { + 0x30, 0x4d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, + 0x02, 0xa0, 0x40, 0x30, 0x3e, 0x02, 0x01, 0x20, 0x31, 0x27, 0x30, 0x0b, + 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03, 0x30, + 0x0b, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02, + 0x30, 0x0b, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, + 0x04, 0x30, 0x10, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, + 0x07, 0x01, 0xa0, 0x03, 0x04, 0x01, 0x00}; + +class P7ImportTest : public ::testing::Test {}; + +TEST_F(P7ImportTest, FailSafeWithUnknownHashes) { + ScopedSEC_PKCS7DecoderContext dcx(SEC_PKCS7DecoderStart( + nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr)); + ASSERT_TRUE(dcx); + SECStatus rv = SEC_PKCS7DecoderUpdate( + dcx.get(), reinterpret_cast(p7_with_unknown_hashes), + sizeof(p7_with_unknown_hashes)); + ASSERT_EQ(SECFailure, rv); +} + +TEST_F(P7ImportTest, NoLeakWithMultipleHashes) { + ScopedSEC_PKCS7DecoderContext dcx(SEC_PKCS7DecoderStart( + nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr)); + ASSERT_TRUE(dcx); + SECStatus rv = SEC_PKCS7DecoderUpdate( + dcx.get(), reinterpret_cast(p7_with_multiple_hashes), + sizeof(p7_with_multiple_hashes)); + ASSERT_EQ(SECFailure, rv); +} + +} // namespace nss_test \ No newline at end of file diff --git a/security/nss/lib/pk11wrap/pk11mech.c b/security/nss/lib/pk11wrap/pk11mech.c index cfbe45b220..f1e84ca05a 100644 --- a/security/nss/lib/pk11wrap/pk11mech.c +++ b/security/nss/lib/pk11wrap/pk11mech.c @@ -766,6 +766,11 @@ PK11_GetIVLength(CK_MECHANISM_TYPE type) case CKM_CAST_ECB: case CKM_CAST3_ECB: case CKM_CAST5_ECB: + case CKM_AES_KEY_WRAP: + case CKM_AES_KEY_WRAP_PAD: + case CKM_AES_KEY_WRAP_KWP: + case CKM_NSS_AES_KEY_WRAP: + case CKM_NSS_AES_KEY_WRAP_PAD: return 0; case CKM_RC2_CBC: case CKM_DES_CBC: @@ -871,6 +876,11 @@ pk11_ParamFromIVWithLen(CK_MECHANISM_TYPE type, SECItem *iv, int keyLen) case CKM_CAST3_ECB: case CKM_CAST5_ECB: case CKM_RC4: + case CKM_AES_KEY_WRAP: + case CKM_AES_KEY_WRAP_PAD: + case CKM_AES_KEY_WRAP_KWP: + case CKM_NSS_AES_KEY_WRAP: + case CKM_NSS_AES_KEY_WRAP_PAD: break; case CKM_RC2_ECB: rc2_ecb_params = (CK_RC2_PARAMS *)PORT_Alloc(sizeof(CK_RC2_PARAMS)); diff --git a/security/nss/lib/pkcs7/p7decode.c b/security/nss/lib/pkcs7/p7decode.c index a6a98960f9..4687c239c5 100644 --- a/security/nss/lib/pkcs7/p7decode.c +++ b/security/nss/lib/pkcs7/p7decode.c @@ -230,6 +230,8 @@ sec_pkcs7_decoder_start_digests(SEC_PKCS7DecoderContext *p7dcx, int depth, { int i, digcnt; + p7dcx->worker.digcnt = 0; + if (digestalgs == NULL) return SECSuccess; @@ -257,7 +259,6 @@ sec_pkcs7_decoder_start_digests(SEC_PKCS7DecoderContext *p7dcx, int depth, } p7dcx->worker.depth = depth; - p7dcx->worker.digcnt = 0; /* * Create a digest context for each algorithm. @@ -277,7 +278,6 @@ sec_pkcs7_decoder_start_digests(SEC_PKCS7DecoderContext *p7dcx, int depth, * but we cannot know that until later. */ if (digobj == NULL) { - p7dcx->worker.digcnt--; continue; } @@ -306,25 +306,19 @@ sec_pkcs7_decoder_finish_digests(SEC_PKCS7DecoderContext *p7dcx, PLArenaPool *poolp, SECItem ***digestsp) { - struct sec_pkcs7_decoder_worker *worker; - const SECHashObject *digobj; - void *digcx; - SECItem **digests, *digest; - int i; - void *mark; - /* * XXX Handling nested contents would mean that there is a chain * of workers -- one per each level of content. The following * would want to find the last worker in the chain. */ - worker = &(p7dcx->worker); + struct sec_pkcs7_decoder_worker *worker = &(p7dcx->worker); /* * If no digests, then we have nothing to do. */ - if (worker->digcnt == 0) + if (worker->digcnt == 0) { return SECSuccess; + } /* * No matter what happens after this, we want to stop filtering. @@ -340,46 +334,46 @@ sec_pkcs7_decoder_finish_digests(SEC_PKCS7DecoderContext *p7dcx, * was digested. */ if (!worker->saw_contents) { - for (i = 0; i < worker->digcnt; i++) { - digcx = worker->digcxs[i]; - digobj = worker->digobjs[i]; + for (int i = 0; i < worker->digcnt; i++) { + void *digcx = worker->digcxs[i]; + const SECHashObject *digobj = worker->digobjs[i]; (*digobj->destroy)(digcx, PR_TRUE); } + worker->digcnt = 0; return SECSuccess; } - mark = PORT_ArenaMark(poolp); + void *mark = PORT_ArenaMark(poolp); /* * Close out each digest context, saving digest away. */ - digests = - (SECItem **)PORT_ArenaAlloc(poolp, (worker->digcnt + 1) * sizeof(SECItem *)); - digest = (SECItem *)PORT_ArenaAlloc(poolp, worker->digcnt * sizeof(SECItem)); - if (digests == NULL || digest == NULL) { + SECItem **digests = + (SECItem **)PORT_ArenaZAlloc(poolp, (worker->digcnt + 1) * sizeof(SECItem *)); + if (digests == NULL) { p7dcx->error = PORT_GetError(); PORT_ArenaRelease(poolp, mark); return SECFailure; } - for (i = 0; i < worker->digcnt; i++, digest++) { - digcx = worker->digcxs[i]; - digobj = worker->digobjs[i]; - - digest->data = (unsigned char *)PORT_ArenaAlloc(poolp, digobj->length); - if (digest->data == NULL) { + for (int i = 0; i < worker->digcnt; i++) { + const SECHashObject *digobj = worker->digobjs[i]; + digests[i] = SECITEM_AllocItem(poolp, NULL, digobj->length); + if (!digests[i]) { p7dcx->error = PORT_GetError(); PORT_ArenaRelease(poolp, mark); return SECFailure; } - - digest->len = digobj->length; - (*digobj->end)(digcx, digest->data, &(digest->len), digest->len); - (*digobj->destroy)(digcx, PR_TRUE); - - digests[i] = digest; } - digests[i] = NULL; + + for (int i = 0; i < worker->digcnt; i++) { + void *digcx = worker->digcxs[i]; + const SECHashObject *digobj = worker->digobjs[i]; + + (*digobj->end)(digcx, digests[i]->data, &(digests[i]->len), digests[i]->len); + (*digobj->destroy)(digcx, PR_TRUE); + } + worker->digcnt = 0; *digestsp = digests; PORT_ArenaUnmark(poolp, mark); @@ -1082,6 +1076,13 @@ SEC_PKCS7DecoderFinish(SEC_PKCS7DecoderContext *p7dcx) if (p7dcx->worker.decryptobj) { sec_PKCS7DestroyDecryptObject(p7dcx->worker.decryptobj); } + for (int i = 0; i < p7dcx->worker.digcnt; i++) { + void *digcx = p7dcx->worker.digcxs[i]; + const SECHashObject *digobj = p7dcx->worker.digobjs[i]; + (*digobj->destroy)(digcx, PR_TRUE); + } + p7dcx->worker.digcnt = 0; + PORT_FreeArena(p7dcx->tmp_poolp, PR_FALSE); PORT_Free(p7dcx); return cinfo; diff --git a/security/nss/lib/util/pkcs11t.h b/security/nss/lib/util/pkcs11t.h index ca00017105..65f56e36a1 100644 --- a/security/nss/lib/util/pkcs11t.h +++ b/security/nss/lib/util/pkcs11t.h @@ -940,6 +940,12 @@ typedef CK_ULONG CK_MECHANISM_TYPE; #define CKM_DH_PKCS_PARAMETER_GEN 0x00002001 #define CKM_X9_42_DH_PARAMETER_GEN 0x00002002 +/* new for v2.40 */ +#define CKM_AES_CFB1 0x00002108UL +#define CKM_AES_KEY_WRAP 0x00002109UL +#define CKM_AES_KEY_WRAP_PAD 0x0000210AUL +#define CKM_AES_KEY_WRAP_KWP 0x0000210BUL + #define CKM_VENDOR_DEFINED 0x80000000 typedef CK_MECHANISM_TYPE CK_PTR CK_MECHANISM_TYPE_PTR;