bug 1465976 - remove all find*ByName APIs from PSM PKCS#11 module/slot/token interfaces r=fkiefer,jcj

Before this patch, we exposed a few interfaces that revolved around mapping a
name to a specific PKCS#11 module, slot, or token. These APIs were all either
problematic and/or unnecessary. In theory there could be two tokens in different
modules with the same name, so nsIPK11TokenDB.findTokenByName wasn't guaranteed
to return what the consumer expected it to. In general, these APIs were used by
front-end code to go from a handle on the specific object in question to a
string identifier and then back to a handle on the object. This was unnecessary
- we can just retain the original handle.

MozReview-Commit-ID: IbqLbV4wceA

--HG--
extra : rebase_source : 05d39afd6bed0aa5e7694e1c79baf836edc03214
This commit is contained in:
David Keeler 2018-05-31 14:46:06 -07:00
Родитель 3a90f6c376
Коммит 23798b7e5f
17 изменённых файлов: 103 добавлений и 239 удалений

Просмотреть файл

@ -18,6 +18,7 @@
#include "nsIInterfaceRequestor.h"
#include "nsIInterfaceRequestorUtils.h"
#include "nsIKeygenThread.h"
#include "nsIPK11Token.h"
#include "nsIPromptService.h"
#include "nsIProtectedAuthThread.h"
#include "nsIWindowWatcher.h"
@ -60,7 +61,7 @@ nsNSSDialogs::Init()
NS_IMETHODIMP
nsNSSDialogs::SetPassword(nsIInterfaceRequestor* ctx,
const nsAString& tokenName,
nsIPK11Token* token,
/*out*/ bool* canceled)
{
// |ctx| is allowed to be null.
@ -75,8 +76,18 @@ nsNSSDialogs::SetPassword(nsIInterfaceRequestor* ctx,
do_CreateInstance(NS_DIALOGPARAMBLOCK_CONTRACTID);
if (!block) return NS_ERROR_FAILURE;
nsresult rv = block->SetString(1, PromiseFlatString(tokenName).get());
if (NS_FAILED(rv)) return rv;
nsCOMPtr<nsIMutableArray> objects = nsArrayBase::Create();
if (!objects) {
return NS_ERROR_FAILURE;
}
nsresult rv = objects->AppendElement(token);
if (NS_FAILED(rv)) {
return rv;
}
rv = block->SetObjects(objects);
if (NS_FAILED(rv)) {
return rv;
}
rv = nsNSSDialogHelper::openDialog(parent,
"chrome://pippki/content/changepassword.xul",

Просмотреть файл

@ -12,7 +12,7 @@ const nsIPKCS11Slot = Ci.nsIPKCS11Slot;
const nsIPK11Token = Ci.nsIPK11Token;
var params;
var tokenName = "";
var token;
var pw1;
function doPrompt(msg) {
@ -24,70 +24,47 @@ function onLoad() {
pw1 = document.getElementById("pw1");
params = window.arguments[0].QueryInterface(Ci.nsIDialogParamBlock);
tokenName = params.GetString(1);
token = params.objects.GetElementAt(0).QueryInterface(Ci.nsIPK11Token);
document.getElementById("tokenName").setAttribute("value", tokenName);
document.getElementById("tokenName").setAttribute("value", token.name);
process();
}
function process() {
let bundle = document.getElementById("pippki_bundle");
let oldpwbox = document.getElementById("oldpw");
let msgBox = document.getElementById("message");
// If the token is unitialized, don't use the old password box.
// Otherwise, do.
if ((token.needsLogin() && token.needsUserInit) || !token.needsLogin()) {
oldpwbox.setAttribute("hidden", "true");
msgBox.setAttribute("value", bundle.getString("password_not_set"));
msgBox.setAttribute("hidden", "false");
let tokenDB = Cc["@mozilla.org/security/pk11tokendb;1"]
.getService(Ci.nsIPK11TokenDB);
let token;
if (tokenName.length > 0) {
token = tokenDB.findTokenByName(tokenName);
} else {
token = tokenDB.getInternalKeyToken();
}
if (token) {
let oldpwbox = document.getElementById("oldpw");
let msgBox = document.getElementById("message");
if ((token.needsLogin() && token.needsUserInit) || !token.needsLogin()) {
oldpwbox.setAttribute("hidden", "true");
msgBox.setAttribute("value", bundle.getString("password_not_set"));
msgBox.setAttribute("hidden", "false");
if (!token.needsLogin()) {
oldpwbox.setAttribute("inited", "empty");
} else {
oldpwbox.setAttribute("inited", "true");
}
// Select first password field
document.getElementById("pw1").focus();
if (!token.needsLogin()) {
oldpwbox.setAttribute("inited", "empty");
} else {
// Select old password field
oldpwbox.setAttribute("hidden", "false");
msgBox.setAttribute("hidden", "true");
oldpwbox.setAttribute("inited", "false");
oldpwbox.focus();
oldpwbox.setAttribute("inited", "true");
}
// Select first password field
document.getElementById("pw1").focus();
} else {
// Select old password field
oldpwbox.setAttribute("hidden", "false");
msgBox.setAttribute("hidden", "true");
oldpwbox.setAttribute("inited", "false");
oldpwbox.focus();
}
if (params) {
// Return value 0 means "canceled"
params.SetInt(1, 0);
}
// Return value 0 means "canceled"
params.SetInt(1, 0);
checkPasswords();
}
function setPassword() {
let tokenDB = Cc["@mozilla.org/security/pk11tokendb;1"]
.getService(Ci.nsIPK11TokenDB);
let token;
if (tokenName.length > 0) {
token = tokenDB.findTokenByName(tokenName);
} else {
token = tokenDB.getInternalKeyToken();
}
var oldpwbox = document.getElementById("oldpw");
var initpw = oldpwbox.getAttribute("inited");
var bundle = document.getElementById("pippki_bundle");

Просмотреть файл

@ -4,6 +4,7 @@
"use strict";
const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm", {});
const { XPCOMUtils } = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm", {});
const nsIPKCS11Slot = Ci.nsIPKCS11Slot;
const nsIPKCS11Module = Ci.nsIPKCS11Module;
@ -40,16 +41,10 @@ function doConfirm(msg) {
function RefreshDeviceList() {
let modules = secmoddb.listModules();
while (modules.hasMoreElements()) {
let module = modules.getNext().QueryInterface(nsIPKCS11Module);
let slotnames = [];
for (let module of XPCOMUtils.IterSimpleEnumerator(modules,
Ci.nsIPKCS11Module)) {
let slots = module.listSlots();
while (slots.hasMoreElements()) {
let slot = slots.getNext().QueryInterface(nsIPKCS11Slot);
// Token names are preferred because NSS prefers lookup by token name.
slotnames.push(slot.tokenName ? slot.tokenName : slot.name);
}
AddModule(module.name, slotnames);
AddModule(module, slots);
}
// Set the text on the FIPS button.
@ -82,22 +77,25 @@ function AddModule(module, slots) {
var item = document.createElement("treeitem");
var row = document.createElement("treerow");
var cell = document.createElement("treecell");
cell.setAttribute("label", module);
cell.setAttribute("label", module.name);
row.appendChild(cell);
item.appendChild(row);
var parent = document.createElement("treechildren");
for (let slot of slots) {
for (let slot of XPCOMUtils.IterSimpleEnumerator(slots, Ci.nsIPKCS11Slot)) {
var child_item = document.createElement("treeitem");
var child_row = document.createElement("treerow");
var child_cell = document.createElement("treecell");
child_cell.setAttribute("label", slot);
child_cell.setAttribute("label", slot.name);
child_row.appendChild(child_cell);
child_item.appendChild(child_row);
child_item.setAttribute("pk11kind", "slot");
// 'slot' is an attribute on any HTML element, hence 'slotObject' instead.
child_item.slotObject = slot;
parent.appendChild(child_item);
}
item.appendChild(parent);
item.setAttribute("pk11kind", "module");
item.module = module;
item.setAttribute("open", "true");
item.setAttribute("container", "true");
tree.appendChild(item);
@ -108,28 +106,19 @@ var selected_module;
/* get the slot selected by the user (can only be one-at-a-time) */
function getSelectedItem() {
var tree = document.getElementById("device_tree");
if (tree.currentIndex < 0) return;
var item = tree.contentView.getItemAtIndex(tree.currentIndex);
let tree = document.getElementById("device_tree");
if (tree.currentIndex < 0) {
return;
}
let item = tree.contentView.getItemAtIndex(tree.currentIndex);
selected_slot = null;
selected_module = null;
if (item) {
var kind = item.getAttribute("pk11kind");
var module_name;
let kind = item.getAttribute("pk11kind");
if (kind == "slot") {
// get the module cell for this slot cell
var cell = item.parentNode.parentNode.firstChild.firstChild;
module_name = cell.getAttribute("label");
var module = secmoddb.findModuleByName(module_name);
// get the cell for the selected row (the slot to display)
cell = item.firstChild.firstChild;
var slot_name = cell.getAttribute("label");
selected_slot = module.findSlotByName(slot_name);
selected_slot = item.slotObject;
} else { // (kind == "module")
// get the cell for the selected row (the module to display)
cell = item.firstChild.firstChild;
module_name = cell.getAttribute("label");
selected_module = secmoddb.findModuleByName(module_name);
selected_module = item.module;
}
}
}
@ -360,7 +349,9 @@ function changePassword() {
getSelectedItem();
let params = Cc[nsDialogParamBlock]
.createInstance(nsIDialogParamBlock);
params.SetString(1, selected_slot.tokenName);
let objects = Cc["@mozilla.org/array;1"].createInstance(Ci.nsIMutableArray);
objects.appendElement(selected_slot.getToken());
params.objects = objects;
window.openDialog("changepassword.xul", "", "chrome,centerscreen,modal",
params);
showSlotInfo();

Просмотреть файл

@ -156,32 +156,6 @@ PKCS11ModuleDB::AddModule(const nsAString& aModuleName,
return NS_OK;
}
NS_IMETHODIMP
PKCS11ModuleDB::FindModuleByName(const nsAString& name,
/*out*/ nsIPKCS11Module** _retval)
{
NS_ENSURE_ARG_POINTER(_retval);
nsresult rv = BlockUntilLoadableRootsLoaded();
if (NS_FAILED(rv)) {
return rv;
}
nsAutoCString moduleNameNormalized;
rv = NormalizeModuleNameIn(name, moduleNameNormalized);
if (NS_FAILED(rv)) {
return rv;
}
UniqueSECMODModule mod(SECMOD_FindModule(moduleNameNormalized.get()));
if (!mod) {
return NS_ERROR_FAILURE;
}
nsCOMPtr<nsIPKCS11Module> module = new nsPKCS11Module(mod.get());
module.forget(_retval);
return NS_OK;
}
NS_IMETHODIMP
PKCS11ModuleDB::ListModules(nsISimpleEnumerator** _retval)
{

Просмотреть файл

@ -20,6 +20,7 @@
#include "nsITokenPasswordDialogs.h"
#include "nsNSSComponent.h"
#include "nsNSSHelper.h"
#include "nsPK11TokenDB.h"
#include "pk11func.h"
#include "pk11sdr.h" // For PK11SDR_Encrypt, PK11SDR_Decrypt
#include "ssl.h" // For SSL_ClearSessionCache
@ -212,7 +213,9 @@ SecretDecoderRing::ChangePassword()
return NS_ERROR_NOT_AVAILABLE;
}
NS_ConvertUTF8toUTF16 tokenName(PK11_GetTokenName(slot.get()));
// nsPK11Token::nsPK11Token takes its own reference to slot, so we pass a
// non-owning pointer here.
nsCOMPtr<nsIPK11Token> token = new nsPK11Token(slot.get());
nsCOMPtr<nsITokenPasswordDialogs> dialogs;
nsresult rv = getNSSDialogs(getter_AddRefs(dialogs),
@ -224,7 +227,7 @@ SecretDecoderRing::ChangePassword()
nsCOMPtr<nsIInterfaceRequestor> ctx = new PipUIContext();
bool canceled; // Ignored
return dialogs->SetPassword(ctx, tokenName, &canceled);
return dialogs->SetPassword(ctx, token, &canceled);
}
NS_IMETHODIMP

Просмотреть файл

@ -28,14 +28,4 @@ interface nsIPK11TokenDB : nsISupports
* Get the internal key database token
*/
nsIPK11Token getInternalKeyToken();
/*
* Find a token by name. Throws NS_ERROR_FAILURE if no token with the given
* name can be found.
* @param tokenName a string identifying the name of the token. Must be
* non-empty.
* @return a token with the given name
*/
[must_use]
nsIPK11Token findTokenByName(in AUTF8String tokenName);
};

Просмотреть файл

@ -16,10 +16,6 @@ interface nsIPKCS11Module : nsISupports
readonly attribute AUTF8String name;
[must_use]
readonly attribute AUTF8String libName;
[must_use]
nsIPKCS11Slot findSlotByName(in AUTF8String name);
[must_use]
nsISimpleEnumerator listSlots();
};

Просмотреть файл

@ -26,9 +26,6 @@ interface nsIPKCS11ModuleDB : nsISupports
in long cryptoMechanismFlags,
in long cipherFlags);
[must_use]
nsIPKCS11Module findModuleByName(in AString name);
[must_use]
nsISimpleEnumerator listModules();

Просмотреть файл

@ -5,6 +5,7 @@
#include "nsISupports.idl"
interface nsIInterfaceRequestor;
interface nsIPK11Token;
/**
* This is the interface for setting and changing password
@ -17,11 +18,11 @@ interface nsITokenPasswordDialogs : nsISupports
* Brings up a dialog to set the password on a token.
*
* @param ctx A user interface context.
* @param tokenName Name of the token.
* @param token {nsIPK11Token} The token.
* @return true if the user canceled the dialog, false otherwise.
*/
[must_use]
boolean setPassword(in nsIInterfaceRequestor ctx, in AString tokenName);
boolean setPassword(in nsIInterfaceRequestor ctx, in nsIPK11Token token);
};
%{C++

Просмотреть файл

@ -2525,8 +2525,8 @@ setPassword(PK11SlotInfo* slot, nsIInterfaceRequestor* ctx)
}
bool canceled;
NS_ConvertUTF8toUTF16 tokenName(PK11_GetTokenName(slot));
rv = dialogs->SetPassword(ctx, tokenName, &canceled);
nsCOMPtr<nsIPK11Token> token = new nsPK11Token(slot);
rv = dialogs->SetPassword(ctx, token, &canceled);
if (NS_FAILED(rv)) {
return rv;
}

Просмотреть файл

@ -292,29 +292,3 @@ nsPK11TokenDB::GetInternalKeyToken(nsIPK11Token** _retval)
return NS_OK;
}
NS_IMETHODIMP
nsPK11TokenDB::FindTokenByName(const nsACString& tokenName,
/*out*/ nsIPK11Token** _retval)
{
NS_ENSURE_ARG_POINTER(_retval);
nsresult rv = BlockUntilLoadableRootsLoaded();
if (NS_FAILED(rv)) {
return rv;
}
if (tokenName.IsEmpty()) {
return NS_ERROR_ILLEGAL_VALUE;
}
UniquePK11SlotInfo slot(
PK11_FindSlotByName(PromiseFlatCString(tokenName).get()));
if (!slot) {
return NS_ERROR_FAILURE;
}
nsCOMPtr<nsIPK11Token> token = new nsPK11Token(slot.get());
token.forget(_retval);
return NS_OK;
}

Просмотреть файл

@ -226,42 +226,6 @@ nsPKCS11Module::GetLibName(/*out*/ nsACString& libName)
return NS_OK;
}
NS_IMETHODIMP
nsPKCS11Module::FindSlotByName(const nsACString& name,
/*out*/ nsIPKCS11Slot** _retval)
{
NS_ENSURE_ARG_POINTER(_retval);
const nsCString& flatName = PromiseFlatCString(name);
MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("Getting \"%s\"", flatName.get()));
UniquePK11SlotInfo slotInfo;
UniquePK11SlotList slotList(PK11_FindSlotsByNames(mModule->dllName,
flatName.get() /*slotName*/,
nullptr /*tokenName*/,
false));
if (!slotList) {
/* name must be the token name */
slotList.reset(PK11_FindSlotsByNames(mModule->dllName, nullptr /*slotName*/,
flatName.get() /*tokenName*/, false));
}
if (slotList && slotList->head && slotList->head->slot) {
slotInfo.reset(PK11_ReferenceSlot(slotList->head->slot));
}
if (!slotInfo) {
// workaround - the builtin module has no name
if (!flatName.EqualsLiteral("Root Certificates")) {
// Give up.
return NS_ERROR_FAILURE;
}
slotInfo.reset(PK11_ReferenceSlot(mModule->slots[0]));
}
nsCOMPtr<nsIPKCS11Slot> slot = new nsPKCS11Slot(slotInfo.get());
slot.forget(_retval);
return NS_OK;
}
NS_IMETHODIMP
nsPKCS11Module::ListSlots(nsISimpleEnumerator** _retval)
{

Просмотреть файл

@ -42,14 +42,6 @@ const gMockPKCS11ModuleDB = {
throw new Error("not expecting getInternalFIPS() to be called");
},
findModuleByName() {
throw new Error("not expecting findModuleByName() to be called");
},
findSlotByName() {
throw new Error("not expecting findSlotByName() to be called");
},
listModules() {
throw new Error("not expecting listModules() to be called");
},

Просмотреть файл

@ -23,9 +23,6 @@ function checkTestModuleNotPresent() {
ok(!(module.libName && module.libName.includes("pkcs11testmodule")),
"Non-test module lib name should not include 'pkcs11testmodule'");
}
throws(() => gModuleDB.findModuleByName("PKCS11 Test Module"),
/NS_ERROR_FAILURE/, "Test module should not be findable by name");
}
/**
@ -52,9 +49,6 @@ function checkTestModuleExists() {
ok(testModule.libName.includes(ctypes.libraryName("pkcs11testmodule")),
"Test module lib name should include lib name of 'pkcs11testmodule'");
notEqual(gModuleDB.findModuleByName("PKCS11 Test Module"), null,
"Test module should be findable by name");
return testModule;
}
@ -107,13 +101,6 @@ function run_test() {
deepEqual(testModuleSlotNames, expectedSlotNames,
"Actual and expected slot names should be equal");
// Check that finding the test slot by name is possible, and that trying to
// find a non-present slot fails.
notEqual(testModule.findSlotByName("Test PKCS11 Slot"), null,
"Test slot should be findable by name");
throws(() => testModule.findSlotByName("Not Present"), /NS_ERROR_FAILURE/,
"Non-present slot should not be findable by name");
// Check that deleting the test module makes it disappear from the module list.
let pkcs11ModuleDB = Cc["@mozilla.org/security/pkcs11moduledb;1"]
.getService(Ci.nsIPKCS11ModuleDB);

Просмотреть файл

@ -17,21 +17,26 @@ function run_test() {
libraryFile.append(ctypes.libraryName("pkcs11testmodule"));
ok(libraryFile.exists(), "The pkcs11testmodule file should exist");
let pkcs11ModuleDB = Cc["@mozilla.org/security/pkcs11moduledb;1"]
let moduleDB = Cc["@mozilla.org/security/pkcs11moduledb;1"]
.getService(Ci.nsIPKCS11ModuleDB);
throws(() => pkcs11ModuleDB.addModule("Root Certs", libraryFile.path, 0, 0),
throws(() => moduleDB.addModule("Root Certs", libraryFile.path, 0, 0),
/NS_ERROR_ILLEGAL_VALUE/,
"Adding a module named 'Root Certs' should fail.");
throws(() => pkcs11ModuleDB.addModule("", libraryFile.path, 0, 0),
throws(() => moduleDB.addModule("", libraryFile.path, 0, 0),
/NS_ERROR_ILLEGAL_VALUE/,
"Adding a module with an empty name should fail.");
let bundle =
Services.strings.createBundle("chrome://pipnss/locale/pipnss.properties");
let rootsModuleName = bundle.GetStringFromName("RootCertModuleName");
let rootsModule = pkcs11ModuleDB.findModuleByName(rootsModuleName);
notEqual(rootsModule, null,
"Should be able to find builtin roots module by localized name.");
equal(rootsModule.name, rootsModuleName,
"Builtin roots module should have correct localized name.");
let foundRootsModule = false;
for (let module of XPCOMUtils.IterSimpleEnumerator(moduleDB.listModules(),
Ci.nsIPKCS11Module)) {
if (module.name == rootsModuleName) {
foundRootsModule = true;
break;
}
}
ok(foundRootsModule,
"Should be able to find builtin roots module by localized name.");
}

Просмотреть файл

@ -8,13 +8,31 @@
// Ensure that the appropriate initialization has happened.
do_get_profile();
function find_slot_by_name(module, name) {
for (let slot of XPCOMUtils.IterSimpleEnumerator(module.listSlots(),
Ci.nsIPKCS11Slot)) {
if (slot.name == name) {
return slot;
}
}
return null;
}
function run_test() {
loadPKCS11TestModule(false);
let moduleDB = Cc["@mozilla.org/security/pkcs11moduledb;1"]
.getService(Ci.nsIPKCS11ModuleDB);
let testModule = moduleDB.findModuleByName("PKCS11 Test Module");
let testSlot = testModule.findSlotByName("Test PKCS11 Slot 二");
let testModule;
for (let module of XPCOMUtils.IterSimpleEnumerator(moduleDB.listModules(),
Ci.nsIPKCS11Module)) {
if (module.name == "PKCS11 Test Module") {
testModule = module;
break;
}
}
let testSlot = find_slot_by_name(testModule, "Test PKCS11 Slot 二");
notEqual(testSlot, null, "should be able to find 'Test PKCS11 Slot 二'");
equal(testSlot.name, "Test PKCS11 Slot 二",
"Actual and expected name should match");
@ -37,7 +55,8 @@ function run_test() {
"Spot check: the actual and expected test token labels should be equal");
ok(!testToken.isInternalKeyToken, "This token is not the internal key token");
testSlot = testModule.findSlotByName("Empty PKCS11 Slot");
testSlot = find_slot_by_name(testModule, "Empty PKCS11 Slot");
notEqual(testSlot, null, "should be able to find 'Empty PKCS11 Slot'");
equal(testSlot.tokenName, null, "Empty slot is empty");
equal(testSlot.status, Ci.nsIPKCS11Slot.SLOT_NOT_PRESENT,
"Actual and expected status should match");

Просмотреть файл

@ -13,21 +13,4 @@ function run_test() {
notEqual(tokenDB.getInternalKeyToken(), null,
"The internal token should be non-null");
throws(() => tokenDB.findTokenByName("Test PKCS11 Tokeñ Label"),
/NS_ERROR_FAILURE/,
"Non-present test token 1 should not be findable by name");
throws(() => tokenDB.findTokenByName("Test PKCS11 Tokeñ 2 Label"),
/NS_ERROR_FAILURE/,
"Non-present test token 2 should not be findable by name");
loadPKCS11TestModule(false);
// Test Token 1 is simulated to insert and remove itself in a tight loop, so
// we don't bother testing that it's present.
notEqual(tokenDB.findTokenByName("Test PKCS11 Tokeñ 2 Label"), null,
"Test token 2 should be findable by name after loading test module");
throws(() => tokenDB.findTokenByName(""), /NS_ERROR_ILLEGAL_VALUE/,
"nsIPK11TokenDB.findTokenByName should throw given an empty name");
}