Bug 1591742 - check des iv length and add test for it, r=jcj,kjacobs
authorFranziskus Kiefer <franziskuskiefer@gmail.com>
Fri, 01 Nov 2019 10:00:04 +0100
changeset 15371 35857ae98190c590ae00a01cb1a2ed48def3915f
parent 15370 27a29997f59819d712e2ccdd9d529f6dad99ca2d
child 15372 dc9552c2aa7779601bfa232ab0764a9c4ad17535
push id3564
push userfranziskuskiefer@gmail.com
push dateMon, 04 Nov 2019 08:28:11 +0000
reviewersjcj, kjacobs
bugs1591742
Bug 1591742 - check des iv length and add test for it, r=jcj,kjacobs Summary: Let's make sure the DES IV has the length we expect it to have. Bug #: 1591742 Differential Revision: https://phabricator.services.mozilla.com/D51073
gtests/pk11_gtest/manifest.mn
gtests/pk11_gtest/pk11_des_unittest.cc
gtests/pk11_gtest/pk11_gtest.gyp
lib/softoken/pkcs11c.c
--- a/gtests/pk11_gtest/manifest.mn
+++ b/gtests/pk11_gtest/manifest.mn
@@ -9,16 +9,17 @@ MODULE = nss
 CPPSRCS = \
       pk11_aes_gcm_unittest.cc \
       pk11_aeskeywrap_unittest.cc \
       pk11_aeskeywrappad_unittest.cc \
       pk11_cbc_unittest.cc \
       pk11_chacha20poly1305_unittest.cc \
       pk11_curve25519_unittest.cc \
       pk11_der_private_key_import_unittest.cc \
+      pk11_des_unittest.cc \
       pk11_ecdsa_unittest.cc \
       pk11_encrypt_derive_unittest.cc \
       pk11_export_unittest.cc \
       pk11_find_certs_unittest.cc \
       pk11_import_unittest.cc \
       pk11_keygen.cc \
       pk11_key_unittest.cc \
       pk11_pbkdf2_unittest.cc \
new file mode 100644
--- /dev/null
+++ b/gtests/pk11_gtest/pk11_des_unittest.cc
@@ -0,0 +1,65 @@
+/* -*- 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 <memory>
+#include "nss.h"
+#include "pk11pub.h"
+
+#include "nss_scoped_ptrs.h"
+
+#include "gtest/gtest.h"
+
+namespace nss_test {
+
+class Pkcs11DesTest : public ::testing::Test {
+ protected:
+  SECStatus EncryptWithIV(std::vector<uint8_t>& iv,
+                          const CK_MECHANISM_TYPE mech) {
+    // Generate a random key.
+    ScopedPK11SlotInfo slot(PK11_GetInternalSlot());
+    ScopedPK11SymKey sym_key(
+        PK11_KeyGen(slot.get(), mech, nullptr, 8, nullptr));
+    EXPECT_TRUE(!!sym_key);
+
+    std::vector<uint8_t> data(16);
+    std::vector<uint8_t> output(16);
+
+    SECItem params = {siBuffer, iv.data(),
+                      static_cast<unsigned int>(iv.size())};
+
+    // Try to encrypt.
+    unsigned int output_len = 0;
+    return PK11_Encrypt(sym_key.get(), mech, &params, output.data(),
+                        &output_len, output.size(), data.data(), data.size());
+  }
+};
+
+TEST_F(Pkcs11DesTest, ZeroLengthIV) {
+  std::vector<uint8_t> iv(0);
+  EXPECT_EQ(SECFailure, EncryptWithIV(iv, CKM_DES_CBC));
+  EXPECT_EQ(SECFailure, EncryptWithIV(iv, CKM_DES3_CBC));
+}
+
+TEST_F(Pkcs11DesTest, IVTooShort) {
+  std::vector<uint8_t> iv(7);
+  EXPECT_EQ(SECFailure, EncryptWithIV(iv, CKM_DES_CBC));
+  EXPECT_EQ(SECFailure, EncryptWithIV(iv, CKM_DES3_CBC));
+}
+
+TEST_F(Pkcs11DesTest, WrongLengthIV) {
+  // We tolerate IVs > 8
+  std::vector<uint8_t> iv(15, 0);
+  EXPECT_EQ(SECSuccess, EncryptWithIV(iv, CKM_DES_CBC));
+  EXPECT_EQ(SECSuccess, EncryptWithIV(iv, CKM_DES3_CBC));
+}
+
+TEST_F(Pkcs11DesTest, AllGood) {
+  std::vector<uint8_t> iv(8, 0);
+  EXPECT_EQ(SECSuccess, EncryptWithIV(iv, CKM_DES_CBC));
+  EXPECT_EQ(SECSuccess, EncryptWithIV(iv, CKM_DES3_CBC));
+}
+
+}  // namespace nss_test
--- a/gtests/pk11_gtest/pk11_gtest.gyp
+++ b/gtests/pk11_gtest/pk11_gtest.gyp
@@ -15,16 +15,17 @@
         'pk11_aes_gcm_unittest.cc',
         'pk11_aeskeywrap_unittest.cc',
         'pk11_aeskeywrappad_unittest.cc',
         'pk11_cbc_unittest.cc',
         'pk11_chacha20poly1305_unittest.cc',
         'pk11_cipherop_unittest.cc',
         'pk11_curve25519_unittest.cc',
         'pk11_der_private_key_import_unittest.cc',
+        'pk11_des_unittest.cc',
         'pk11_ecdsa_unittest.cc',
         'pk11_encrypt_derive_unittest.cc',
         'pk11_find_certs_unittest.cc',
         'pk11_import_unittest.cc',
         'pk11_keygen.cc',
         'pk11_key_unittest.cc',
         'pk11_pbkdf2_unittest.cc',
         'pk11_prf_unittest.cc',
--- a/lib/softoken/pkcs11c.c
+++ b/lib/softoken/pkcs11c.c
@@ -997,16 +997,20 @@ sftk_CryptInit(CK_SESSION_HANDLE hSessio
         case CKM_DES_CBC_PAD:
             context->doPad = PR_TRUE;
         /* fall thru */
         case CKM_DES_CBC:
             if (key_type != CKK_DES) {
                 crv = CKR_KEY_TYPE_INCONSISTENT;
                 break;
             }
+            if (pMechanism->ulParameterLen < 8) {
+                crv = CKR_DOMAIN_PARAMS_INVALID;
+                break;
+            }
             t = NSS_DES_CBC;
             goto finish_des;
         case CKM_DES3_ECB:
             if ((key_type != CKK_DES2) && (key_type != CKK_DES3)) {
                 crv = CKR_KEY_TYPE_INCONSISTENT;
                 break;
             }
             t = NSS_DES_EDE3;
@@ -1014,16 +1018,20 @@ sftk_CryptInit(CK_SESSION_HANDLE hSessio
         case CKM_DES3_CBC_PAD:
             context->doPad = PR_TRUE;
         /* fall thru */
         case CKM_DES3_CBC:
             if ((key_type != CKK_DES2) && (key_type != CKK_DES3)) {
                 crv = CKR_KEY_TYPE_INCONSISTENT;
                 break;
             }
+            if (pMechanism->ulParameterLen < 8) {
+                crv = CKR_DOMAIN_PARAMS_INVALID;
+                break;
+            }
             t = NSS_DES_EDE3_CBC;
         finish_des:
             context->blockSize = 8;
             att = sftk_FindAttribute(key, CKA_VALUE);
             if (att == NULL) {
                 crv = CKR_KEY_HANDLE_INVALID;
                 break;
             }