Bug 1379273 - make softoken resettable via PK11_ResetToken r=franziskus,ttaubert
authorDavid Keeler <dkeeler@mozilla.com>
Tue, 01 Aug 2017 16:45:07 +0200
changeset 13497 102381007af090fba357b7b777a5a8dcb67a600f
parent 13496 4fbcc53f0ac630d006dc668cde354d2da6562982
child 13498 bb2e1fd1d7a9bbd9b33116713e20b8de97958300
push id2299
push userfranziskuskiefer@gmail.com
push dateWed, 02 Aug 2017 08:23:00 +0000
reviewersfranziskus, ttaubert
bugs1379273
Bug 1379273 - make softoken resettable via PK11_ResetToken r=franziskus,ttaubert Summary: Two issues prevented PK11_ResetToken from working properly: 1. The backing DB tables would be dropped and never recreated, preventing future operations from working. 2. The needLogin property of the SFTKSlot would not be updated properly, preventing PK11_InitPin (and thus other operations) from succeeding. Reviewers: ttaubert, franziskus Reviewed By: ttaubert, franziskus Differential Revision: https://nss-review.dev.mozaws.net/D382
gtests/manifest.mn
gtests/softoken_gtest/Makefile
gtests/softoken_gtest/manifest.mn
gtests/softoken_gtest/softoken_gtest.cc
gtests/softoken_gtest/softoken_gtest.gyp
lib/softoken/pkcs11.c
lib/softoken/sdb.c
nss.gyp
tests/gtests/gtests.sh
--- a/gtests/manifest.mn
+++ b/gtests/manifest.mn
@@ -18,18 +18,19 @@ UTIL_SRCDIRS = \
 endif
 
 ifneq ($(NSS_BUILD_SOFTOKEN_ONLY),1)
 ifneq ($(NSS_BUILD_UTIL_ONLY),1)
 NSS_SRCDIRS = \
 	certdb_gtest \
 	certhigh_gtest \
 	pk11_gtest \
+	softoken_gtest \
 	ssl_gtest \
-        nss_bogo_shim \
+	nss_bogo_shim \
 	$(NULL)
 endif
 endif
 
 DIRS = \
 	$(LIB_SRCDIRS) \
 	$(UTIL_SRCDIRS) \
 	$(NSS_SRCDIRS) \
copy from gtests/util_gtest/Makefile
copy to gtests/softoken_gtest/Makefile
copy from gtests/util_gtest/manifest.mn
copy to gtests/softoken_gtest/manifest.mn
--- a/gtests/util_gtest/manifest.mn
+++ b/gtests/softoken_gtest/manifest.mn
@@ -2,28 +2,24 @@
 # 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/.
 CORE_DEPTH = ../..
 DEPTH      = ../..
 MODULE = nss
 
 CPPSRCS = \
-	util_utf8_unittest.cc \
-	util_b64_unittest.cc \
-	util_pkcs11uri_unittest.cc \
+	softoken_gtest.cc \
 	$(NULL)
 
 INCLUDES += \
 	-I$(CORE_DEPTH)/gtests/google_test/gtest/include \
-	-I$(CORE_DEPTH)/gtests/common \
 	-I$(CORE_DEPTH)/cpputil \
 	$(NULL)
 
 REQUIRES = nspr gtest
 
-PROGRAM = util_gtest
+PROGRAM = softoken_gtest
 
 EXTRA_LIBS = \
 	$(DIST)/lib/$(LIB_PREFIX)gtest.$(LIB_SUFFIX) \
-	$(DIST)/lib/$(LIB_PREFIX)nssutil.$(LIB_SUFFIX) \
 	$(DIST)/lib/$(LIB_PREFIX)gtestutil.$(LIB_SUFFIX) \
 	$(NULL)
copy from gtests/common/gtests.cc
copy to gtests/softoken_gtest/softoken_gtest.cc
--- a/gtests/common/gtests.cc
+++ b/gtests/softoken_gtest/softoken_gtest.cc
@@ -1,26 +1,132 @@
+#include <cstdlib>
+
 #include "nspr.h"
 #include "nss.h"
-#include "ssl.h"
+#include "pk11pub.h"
 
-#include <cstdlib>
+#include "scoped_ptrs.h"
 
 #define GTEST_HAS_RTTI 0
 #include "gtest/gtest.h"
 
+namespace nss_test {
+
+// Given a prefix, attempts to create a unique directory that the user can do
+// work in without impacting other tests. For example, if given the prefix
+// "scratch", a directory like "scratch05c17b25" will be created in the current
+// working directory (or the location specified by NSS_GTEST_WORKDIR, if
+// defined).
+// Upon destruction, the implementation will attempt to delete the directory.
+// However, no attempt is made to first remove files in the directory - the
+// user is responsible for this. If the directory is not empty, deleting it will
+// fail.
+// Statistically, it is technically possible to fail to create a unique
+// directory name, but this is extremely unlikely given the expected workload of
+// this implementation.
+class ScopedUniqueDirectory {
+public:
+  explicit ScopedUniqueDirectory(const std::string& prefix);
+
+  // NB: the directory must be empty upon destruction
+  ~ScopedUniqueDirectory() {
+    assert(rmdir(mPath.c_str()) == 0);
+  }
+
+  const std::string& GetPath() { return mPath; }
+
+private:
+  static const int RETRY_LIMIT = 5;
+  static void GenerateRandomName(/*in/out*/ std::string& prefix);
+  static bool TryMakingDirectory(/*in/out*/ std::string& prefix);
+
+  std::string mPath;
+};
+
+ScopedUniqueDirectory::ScopedUniqueDirectory(const std::string& prefix)
+{
+  std::string path;
+  const char* workingDirectory = PR_GetEnvSecure("NSS_GTEST_WORKDIR");
+  if (workingDirectory) {
+    path.assign(workingDirectory);
+  }
+  path.append(prefix);
+  for (int i = 0; i < RETRY_LIMIT; i++) {
+    std::string pathCopy(path);
+    // TryMakingDirectory will modify its input. If it fails, we want to throw
+    // away the modified result.
+    if (TryMakingDirectory(pathCopy)) {
+      mPath.assign(pathCopy);
+      break;
+    }
+  }
+  assert(mPath.length() > 0);
+}
+
+void
+ScopedUniqueDirectory::GenerateRandomName(std::string& prefix)
+{
+  std::stringstream ss;
+  ss << prefix;
+  // RAND_MAX is at least 32767.
+  ss << std::setfill('0') << std::setw(4) << std::hex << rand() << rand();
+  // This will overwrite the value of prefix. This is a little inefficient, but
+  // at least it makes the code simple.
+  ss >> prefix;
+}
+
+bool
+ScopedUniqueDirectory::TryMakingDirectory(std::string& prefix)
+{
+  GenerateRandomName(prefix);
+#if defined(_WIN32)
+  return _mkdir(prefix.c_str()) == 0;
+#else
+  return mkdir(prefix.c_str(), 0777) == 0;
+#endif
+}
+
+class SoftokenTest : public ::testing::Test {
+protected:
+  SoftokenTest() : mNSSDBDir("SoftokenTest.d-") { }
+
+  virtual void SetUp() {
+    std::string nssInitArg("sql:");
+    nssInitArg.append(mNSSDBDir.GetPath());
+    ASSERT_EQ(SECSuccess, NSS_Initialize(nssInitArg.c_str(), "", "", SECMOD_DB,
+                                         NSS_INIT_NOROOTINIT));
+  }
+
+  virtual void TearDown() {
+    ASSERT_EQ(SECSuccess, NSS_Shutdown());
+    const std::string& nssDBDirPath = mNSSDBDir.GetPath();
+    ASSERT_EQ(0, unlink((nssDBDirPath + "/cert9.db").c_str()));
+    ASSERT_EQ(0, unlink((nssDBDirPath + "/key4.db").c_str()));
+    ASSERT_EQ(0, unlink((nssDBDirPath + "/pkcs11.txt").c_str()));
+  }
+
+  ScopedUniqueDirectory mNSSDBDir;
+};
+
+TEST_F(SoftokenTest, ResetSoftokenEmptyPassword) {
+  ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
+  ASSERT_TRUE(slot);
+  EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, nullptr));
+  EXPECT_EQ(SECSuccess, PK11_ResetToken(slot.get(), nullptr));
+  EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, nullptr));
+}
+
+TEST_F(SoftokenTest, ResetSoftokenNonEmptyPassword) {
+  ScopedPK11SlotInfo slot(PK11_GetInternalKeySlot());
+  ASSERT_TRUE(slot);
+  EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, "password"));
+  EXPECT_EQ(SECSuccess, PK11_ResetToken(slot.get(), nullptr));
+  EXPECT_EQ(SECSuccess, PK11_InitPin(slot.get(), nullptr, "password2"));
+}
+
+}  // namespace nss_test
+
 int main(int argc, char **argv) {
   ::testing::InitGoogleTest(&argc, argv);
 
-  if (NSS_NoDB_Init(nullptr) != SECSuccess) {
-    return 1;
-  }
-  if (NSS_SetDomesticPolicy() != SECSuccess) {
-    return 1;
-  }
-  int rv = RUN_ALL_TESTS();
-
-  if (NSS_Shutdown() != SECSuccess) {
-    return 1;
-  }
-
-  return rv;
+  return RUN_ALL_TESTS();
 }
copy from gtests/util_gtest/util_gtest.gyp
copy to gtests/softoken_gtest/softoken_gtest.gyp
--- a/gtests/util_gtest/util_gtest.gyp
+++ b/gtests/softoken_gtest/softoken_gtest.gyp
@@ -3,49 +3,46 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 {
   'includes': [
     '../../coreconf/config.gypi',
     '../common/gtest.gypi',
   ],
   'targets': [
     {
-      'target_name': 'util_gtest',
+      'target_name': 'softoken_gtest',
       'type': 'executable',
       'sources': [
-        'util_utf8_unittest.cc',
-        'util_b64_unittest.cc',
-        'util_pkcs11uri_unittest.cc',
-        '<(DEPTH)/gtests/common/gtests.cc',
+        'softoken_gtest.cc',
       ],
       'dependencies': [
         '<(DEPTH)/exports.gyp:nss_exports',
+        '<(DEPTH)/lib/util/util.gyp:nssutil3',
         '<(DEPTH)/gtests/google_test/google_test.gyp:gtest',
-        '<(DEPTH)/lib/util/util.gyp:nssutil',
-        '<(DEPTH)/lib/nss/nss.gyp:nss_static',
-        '<(DEPTH)/lib/pk11wrap/pk11wrap.gyp:pk11wrap_static',
-        '<(DEPTH)/lib/cryptohi/cryptohi.gyp:cryptohi',
-        '<(DEPTH)/lib/certhigh/certhigh.gyp:certhi',
-        '<(DEPTH)/lib/certdb/certdb.gyp:certdb',
-        '<(DEPTH)/lib/base/base.gyp:nssb',
-        '<(DEPTH)/lib/dev/dev.gyp:nssdev',
-        '<(DEPTH)/lib/pki/pki.gyp:nsspki',
-        '<(DEPTH)/lib/ssl/ssl.gyp:ssl',
-        '<(DEPTH)/lib/libpkix/libpkix.gyp:libpkix',
       ],
       'conditions': [
-        [ 'OS=="win"', {
-          'libraries': [
-            'advapi32.lib',
+        [ 'test_build==1', {
+          'dependencies': [
+            '<(DEPTH)/lib/nss/nss.gyp:nss_static',
+            '<(DEPTH)/lib/pk11wrap/pk11wrap.gyp:pk11wrap_static',
+            '<(DEPTH)/lib/cryptohi/cryptohi.gyp:cryptohi',
+            '<(DEPTH)/lib/certhigh/certhigh.gyp:certhi',
+            '<(DEPTH)/lib/certdb/certdb.gyp:certdb',
+            '<(DEPTH)/lib/base/base.gyp:nssb',
+            '<(DEPTH)/lib/dev/dev.gyp:nssdev',
+            '<(DEPTH)/lib/pki/pki.gyp:nsspki',
+            '<(DEPTH)/lib/ssl/ssl.gyp:ssl',
+          ],
+        }, {
+          'dependencies': [
+            '<(DEPTH)/lib/nss/nss.gyp:nss3',
+            '<(DEPTH)/lib/ssl/ssl.gyp:ssl3',
           ],
         }],
       ],
-      'defines': [
-        'NSS_USE_STATIC_LIBS'
-      ],
     }
   ],
   'target_defaults': {
     'include_dirs': [
       '../../lib/util'
     ]
   },
   'variables': {
--- a/lib/softoken/pkcs11.c
+++ b/lib/softoken/pkcs11.c
@@ -3561,17 +3561,16 @@ sftk_MechAllowsOperation(CK_MECHANISM_TY
 
 /* NSC_InitToken initializes a token. */
 CK_RV
 NSC_InitToken(CK_SLOT_ID slotID, CK_CHAR_PTR pPin,
               CK_ULONG ulPinLen, CK_CHAR_PTR pLabel)
 {
     SFTKSlot *slot = sftk_SlotFromID(slotID, PR_FALSE);
     SFTKDBHandle *handle;
-    SFTKDBHandle *certHandle;
     SECStatus rv;
     unsigned int i;
     SFTKObject *object;
 
     CHECK_FORK();
 
     if (slot == NULL)
         return CKR_SLOT_ID_INVALID;
@@ -3609,29 +3608,26 @@ NSC_InitToken(CK_SLOT_ID slotID, CK_CHAR
 
     /* then clear out the key database */
     handle = sftk_getKeyDB(slot);
     if (handle == NULL) {
         return CKR_TOKEN_WRITE_PROTECTED;
     }
 
     rv = sftkdb_ResetKeyDB(handle);
+    /* clear the password */
+    sftkdb_ClearPassword(handle);
+    /* update slot->needLogin (should be true now since no password is set) */
+    sftk_checkNeedLogin(slot, handle);
     sftk_freeDB(handle);
     if (rv != SECSuccess) {
         return CKR_DEVICE_ERROR;
     }
 
-    /* finally  mark all the user certs as non-user certs */
-    certHandle = sftk_getCertDB(slot);
-    if (certHandle == NULL)
-        return CKR_OK;
-
-    sftk_freeDB(certHandle);
-
-    return CKR_OK; /*is this the right function for not implemented*/
+    return CKR_OK;
 }
 
 /* NSC_InitPIN initializes the normal user's PIN. */
 CK_RV
 NSC_InitPIN(CK_SESSION_HANDLE hSession,
             CK_CHAR_PTR pPin, CK_ULONG ulPinLen)
 {
     SFTKSession *sp = NULL;
--- a/lib/softoken/sdb.c
+++ b/lib/softoken/sdb.c
@@ -1595,17 +1595,17 @@ loser:
     if (sqlDB) {
         sdb_closeDBLocal(sdb_p, sqlDB);
     }
     UNLOCK_SQLITE()
 
     return error;
 }
 
-static const char RESET_CMD[] = "DROP TABLE IF EXISTS %s;";
+static const char RESET_CMD[] = "DELETE FROM %s;";
 CK_RV
 sdb_Reset(SDB *sdb)
 {
     SDBPrivate *sdb_p = sdb->private;
     sqlite3 *sqlDB = NULL;
     char *newStr;
     int sqlerr = SQLITE_OK;
     CK_RV error = CKR_OK;
@@ -1616,27 +1616,29 @@ sdb_Reset(SDB *sdb)
     }
 
     LOCK_SQLITE()
     error = sdb_openDBLocal(sdb_p, &sqlDB, NULL);
     if (error != CKR_OK) {
         goto loser;
     }
 
-    /* delete the key table */
-    newStr = sqlite3_mprintf(RESET_CMD, sdb_p->table);
-    if (newStr == NULL) {
-        error = CKR_HOST_MEMORY;
-        goto loser;
+    if (tableExists(sqlDB, sdb_p->table)) {
+        /* delete the contents of the key table */
+        newStr = sqlite3_mprintf(RESET_CMD, sdb_p->table);
+        if (newStr == NULL) {
+            error = CKR_HOST_MEMORY;
+            goto loser;
+        }
+        sqlerr = sqlite3_exec(sqlDB, newStr, NULL, 0, NULL);
+        sqlite3_free(newStr);
+
+        if (sqlerr != SQLITE_OK)
+            goto loser;
     }
-    sqlerr = sqlite3_exec(sqlDB, newStr, NULL, 0, NULL);
-    sqlite3_free(newStr);
-
-    if (sqlerr != SQLITE_OK)
-        goto loser;
 
     /* delete the password entry table */
     sqlerr = sqlite3_exec(sqlDB, "DROP TABLE IF EXISTS metaData;",
                           NULL, 0, NULL);
 
 loser:
     /* fix up the error if necessary */
     if (error == CKR_OK) {
--- a/nss.gyp
+++ b/nss.gyp
@@ -163,16 +163,17 @@
             'cmd/tstclnt/tstclnt.gyp:tstclnt',
             'cmd/vfychain/vfychain.gyp:vfychain',
             'cmd/vfyserv/vfyserv.gyp:vfyserv',
             'gtests/certhigh_gtest/certhigh_gtest.gyp:certhigh_gtest',
             'gtests/der_gtest/der_gtest.gyp:der_gtest',
             'gtests/certdb_gtest/certdb_gtest.gyp:certdb_gtest',
             'gtests/freebl_gtest/freebl_gtest.gyp:prng_gtest',
             'gtests/pk11_gtest/pk11_gtest.gyp:pk11_gtest',
+            'gtests/softoken_gtest/softoken_gtest.gyp:softoken_gtest',
             'gtests/ssl_gtest/ssl_gtest.gyp:ssl_gtest',
             'gtests/util_gtest/util_gtest.gyp:util_gtest',
             'gtests/nss_bogo_shim/nss_bogo_shim.gyp:nss_bogo_shim',
           ],
           'conditions': [
             [ 'OS=="linux"', {
               'dependencies': [
                 'cmd/lowhashtest/lowhashtest.gyp:lowhashtest',
--- a/tests/gtests/gtests.sh
+++ b/tests/gtests/gtests.sh
@@ -78,13 +78,13 @@ gtest_start()
 gtest_cleanup()
 {
   html "</TABLE><BR>"
   cd ${QADIR}
   . common/cleanup.sh
 }
 
 ################## main #################################################
-GTESTS="prng_gtest certhigh_gtest certdb_gtest der_gtest pk11_gtest util_gtest freebl_gtest"
+GTESTS="prng_gtest certhigh_gtest certdb_gtest der_gtest pk11_gtest util_gtest freebl_gtest softoken_gtest"
 SOURCE_DIR="$PWD"/../..
 gtest_init $0
 gtest_start
 gtest_cleanup