Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-28 15:23:51 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1539415 - make nsICertStorage (cert_storage) asynchronous for functions called from the main thread r=jcj,mgoodwin

Browse Source 
The Set* functions of nsICertStorage (SetRevocationByIssuerAndSerial,
SetRevocationBySubjectAndPubKey, SetEnrollment, and SetWhitelist) are called on
the main thread by the implementations that manage consuming remote security
information. We don't want to block the main thread, so this patch modifies
these functions to take a callback that will be called (on the original thread)
when the operation in question has been completed on a background thread.

The Get* functions of nsICertStorage (GetRevocationState, GetEnrollmentState,
and GetWhitelistState) should only be called off the main thread. For the most
part they are, but there are at least two main-thread certificate verifications
that can cause these functions to be called on the main thread. These instances
are in nsSiteSecurityService::ProcessPKPHeader and
ContentSignatureVerifier::CreateContextInternal and will be dealt with in
bug 1406854 bug 1534600, respectively.

Differential Revision: https://phabricator.services.mozilla.com/D25174

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Dana Keeler 2019-04-03 23:24:19 +00:00
parent 98518a7159
commit a483dcca02
17 changed files with 437 additions and 160 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Cargo.lock generated
View File

@ -432,7 +432,9 @@ name = "cert_storage"
version = "0.0.1"
dependencies = [
"base64 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)",
"crossbeam-utils 0.6.3 (registry+https://github.com/rust-lang/crates.io-index)",
"lmdb-rkv 0.11.2 (registry+https://github.com/rust-lang/crates.io-index)",
"moz_task 0.1.0",
"nserror 0.1.0",
"nsstring 0.1.0",
"rkv 0.9.4 (registry+https://github.com/rust-lang/crates.io-index)",

2
security/manager/ssl/cert_storage/Cargo.toml
View File

@ -5,7 +5,9 @@ authors = ["Dana Keeler <dkeeler@mozilla.com>", "Mark Goodwin <mgoodwin@mozilla.
[dependencies]
base64 = "0.10"
crossbeam-utils = "0.6.3"
lmdb-rkv = "0.11"
moz_task = { path = "../../../../xpcom/rust/moz_task" }
nserror = { path = "../../../../xpcom/rust/nserror" }
nsstring = { path = "../../../../xpcom/rust/nsstring" }
rkv = "^0.9"

282
security/manager/ssl/cert_storage/src/lib.rs
View File

@ -3,7 +3,9 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
extern crate base64;
extern crate crossbeam_utils;
extern crate lmdb;
extern crate moz_task;
extern crate nserror;
extern crate nsstring;
extern crate rkv;
@ -13,7 +15,15 @@ extern crate time;
extern crate xpcom;
extern crate style;
use crossbeam_utils::atomic::AtomicCell;
use lmdb::EnvironmentFlags;
use moz_task::{create_thread, is_main_thread, Task, TaskRunnable};
use nserror::{
nsresult, NS_ERROR_FAILURE, NS_ERROR_NOT_SAME_THREAD, NS_ERROR_NO_AGGREGATION,
NS_ERROR_UNEXPECTED, NS_OK,
};
use nsstring::{nsACString, nsAString, nsCStr, nsCString, nsString};
use rkv::{Rkv, SingleStore, StoreOptions, Value};
use sha2::{Digest, Sha256};
use std::collections::HashMap;
use std::ffi::{CStr, CString};
@ -24,14 +34,13 @@ use std::os::raw::c_char;
use std::path::PathBuf;
use std::slice;
use std::str;
use std::sync::RwLock;
use std::sync::{Arc, Mutex, RwLock};
use std::time::{Duration, SystemTime};
use style::gecko_bindings::structs::nsresult;
use xpcom::interfaces::{nsICertStorage, nsIFile, nsIObserver, nsIPrefBranch, nsISupports};
use xpcom::{nsIID, GetterAddrefs, RefPtr, XpCom};
use lmdb::EnvironmentFlags;
use rkv::{Rkv, SingleStore, StoreOptions, Value};
use xpcom::interfaces::{
nsICertStorage, nsICertStorageCallback, nsIFile, nsIObserver, nsIPrefBranch, nsISupports,
nsIThread,
};
use xpcom::{nsIID, GetterAddrefs, RefPtr, ThreadBoundRefPtr, XpCom};
const PREFIX_REV_IS: &str = "is";
const PREFIX_REV_SPK: &str = "spk";
@ -465,7 +474,8 @@ fn do_construct_cert_storage(
let path_buf = get_profile_path()?;
let cert_storage = CertStorage::allocate(InitCertStorage {
security_state: RwLock::new(SecurityState::new(path_buf)?),
security_state: Arc::new(RwLock::new(SecurityState::new(path_buf)?)),
thread: Mutex::new(create_thread("cert_storage")?),
});
unsafe {
@ -506,15 +516,97 @@ fn read_int_pref(name: &str) -> Result<i32, SecurityStateError> {
// supported. No matter, we can just check for failure and ignore
// any NS_ERROR_UNEXPECTED result.
let res = unsafe { (*prefs).GetIntPref((&pref_name).as_ptr(), (&mut pref_value) as *mut i32) };
if !res.succeeded() {
match res.0 {
r if r == nsresult::NS_ERROR_UNEXPECTED as u32 => (),
_ => return Err(SecurityStateError::from("could not read pref")),
}
match res {
NS_OK => Ok(pref_value),
NS_ERROR_UNEXPECTED => Ok(pref_value),
_ => Err(SecurityStateError::from("could not read pref")),
}
Ok(pref_value)
}
// This is a helper for defining a task that will perform a specific action on a background thread.
// Its arguments are the name of the task and the name of the function in SecurityState to call.
macro_rules! security_state_task {
($task_name:ident, $security_state_function_name:ident) => {
struct $task_name {
callback: AtomicCell<Option<ThreadBoundRefPtr<nsICertStorageCallback>>>,
security_state: Arc<RwLock<SecurityState>>,
argument_a: Vec<u8>,
argument_b: Vec<u8>,
state: i16,
result: AtomicCell<Option<nserror::nsresult>>,
}
impl $task_name {
fn new(
callback: &nsICertStorageCallback,
security_state: &Arc<RwLock<SecurityState>>,
argument_a: Vec<u8>,
argument_b: Vec<u8>,
state: i16,
) -> $task_name {
$task_name {
callback: AtomicCell::new(Some(ThreadBoundRefPtr::new(RefPtr::new(callback)))),
security_state: Arc::clone(security_state),
argument_a,
argument_b,
state,
result: AtomicCell::new(None),
}
}
}
impl Task for $task_name {
fn run(&self) {
let mut ss = match self.security_state.write() {
Ok(ss) => ss,
Err(_) => {
self.result.store(Some(NS_ERROR_FAILURE));
return;
}
};
// this is a no-op if the DB is already open
match ss.open_db() {
Ok(()) => {}
Err(_) => {
self.result.store(Some(NS_ERROR_FAILURE));
return;
}
};
match ss.$security_state_function_name(
&self.argument_a,
&self.argument_b,
self.state,
) {
Ok(_) => self.result.store(Some(NS_OK)),
Err(_) => self.result.store(Some(NS_ERROR_FAILURE)),
};
}
fn done(&self) -> Result<(), nsresult> {
let threadbound = self.callback.swap(None).ok_or(NS_ERROR_FAILURE)?;
let callback = threadbound.get_ref().ok_or(NS_ERROR_FAILURE)?;
let nsrv = match self.result.swap(None) {
Some(result) => unsafe { callback.Done(result) },
None => unsafe { callback.Done(NS_ERROR_FAILURE) },
};
match nsrv {
NS_OK => Ok(()),
e => Err(e),
}
}
}
};
}
security_state_task!(
SetRevocationByIssuerAndSerialTask,
set_revocation_by_issuer_and_serial
);
security_state_task!(
SetRevocationBySubjectAndPubKeyTask,
set_revocation_by_subject_and_pub_key
);
security_state_task!(SetEnrollmentTask, set_enrollment);
security_state_task!(SetWhitelistTask, set_whitelist);
#[no_mangle]
pub extern "C" fn cert_storage_constructor(
outer: *const nsISupports,
@ -522,14 +614,14 @@ pub extern "C" fn cert_storage_constructor(
result: *mut *mut xpcom::reexports::libc::c_void,
) -> nserror::nsresult {
if !outer.is_null() {
return nserror::NS_ERROR_NO_AGGREGATION;
return NS_ERROR_NO_AGGREGATION;
}
match do_construct_cert_storage(outer, iid, result) {
Ok(_) => nserror::NS_OK,
Ok(_) => NS_OK,
Err(_) => {
// In future: log something so we know what went wrong?
nserror::NS_ERROR_FAILURE
NS_ERROR_FAILURE
}
}
}
@ -538,7 +630,7 @@ macro_rules! try_ns {
($e:expr) => {
match $e {
Ok(value) => value,
Err(_) => return nserror::NS_ERROR_FAILURE,
Err(_) => return NS_ERROR_FAILURE,
}
};
}
@ -547,9 +639,17 @@ macro_rules! try_ns {
#[xpimplements(nsICertStorage, nsIObserver)]
#[refcnt = "atomic"]
struct InitCertStorage {
security_state: RwLock<SecurityState>,
security_state: Arc<RwLock<SecurityState>>,
thread: Mutex<RefPtr<nsIThread>>,
}
/// CertStorage implements the nsICertStorage interface. The actual work is done by the
/// SecurityState. To handle any threading issues, we have an atomic-refcounted read/write lock on
/// the one and only SecurityState. So, only one thread can use SecurityState's &mut self functions
/// at a time, while multiple threads can use &self functions simultaneously (as long as there are
/// no threads using an &mut self function). The Arc is to allow for the creation of background
/// tasks that use the SecurityState on the thread owned by CertStorage. This allows us to not block
/// the main thread.
#[allow(non_snake_case)]
impl CertStorage {
unsafe fn setup_prefs(&self) -> Result<(), SecurityStateError> {
@ -590,56 +690,83 @@ impl CertStorage {
issuer: *const nsACString,
serial: *const nsACString,
state: i16,
callback: *const nsICertStorageCallback,
) -> nserror::nsresult {
if issuer.is_null() || serial.is_null() {
return nserror::NS_ERROR_FAILURE;
if !is_main_thread() {
return NS_ERROR_NOT_SAME_THREAD;
}
if issuer.is_null() || serial.is_null() || callback.is_null() {
return NS_ERROR_FAILURE;
}
let issuer_decoded = try_ns!(base64::decode(&*issuer));
let serial_decoded = try_ns!(base64::decode(&*serial));
let mut ss = try_ns!(self.security_state.write());
try_ns!(ss.open_db()); // this is a no-op if the DB is already open
match ss.set_revocation_by_issuer_and_serial(&issuer_decoded, &serial_decoded, state) {
Ok(_) => nserror::NS_OK,
_ => nserror::NS_ERROR_FAILURE,
}
let task = Box::new(SetRevocationByIssuerAndSerialTask::new(
&*callback,
&self.security_state,
issuer_decoded,
serial_decoded,
state,
));
let thread = try_ns!(self.thread.lock());
let runnable = try_ns!(TaskRunnable::new("SetRevocationByIssuerAndSerial", task));
try_ns!(runnable.dispatch(&*thread));
NS_OK
}
unsafe fn SetRevocationBySubjectAndPubKey(
&self,
subject: *const nsACString,
pub_key_base64: *const nsACString,
pub_key_hash: *const nsACString,
state: i16,
callback: *const nsICertStorageCallback,
) -> nserror::nsresult {
if subject.is_null() || pub_key_base64.is_null() {
return nserror::NS_ERROR_FAILURE;
if !is_main_thread() {
return NS_ERROR_NOT_SAME_THREAD;
}
if subject.is_null() || pub_key_hash.is_null() || callback.is_null() {
return NS_ERROR_FAILURE;
}
let subject_decoded = try_ns!(base64::decode(&*subject));
let pub_key_decoded = try_ns!(base64::decode(&*pub_key_base64));
let mut ss = try_ns!(self.security_state.write());
try_ns!(ss.open_db()); // this is a no-op if the DB is already open
match ss.set_revocation_by_subject_and_pub_key(&subject_decoded, &pub_key_decoded, state) {
Ok(_) => nserror::NS_OK,
_ => nserror::NS_ERROR_FAILURE,
}
}
let pub_key_hash_decoded = try_ns!(base64::decode(&*pub_key_hash));
let task = Box::new(SetRevocationBySubjectAndPubKeyTask::new(
&*callback,
&self.security_state,
subject_decoded,
pub_key_hash_decoded,
state,
));
let thread = try_ns!(self.thread.lock());
let runnable = try_ns!(TaskRunnable::new("SetRevocationBySubjectAndPubKey", task));
try_ns!(runnable.dispatch(&*thread));
NS_OK
}
unsafe fn SetEnrollment(
&self,
issuer: *const nsACString,
serial: *const nsACString,
state: i16,
callback: *const nsICertStorageCallback,
) -> nserror::nsresult {
if issuer.is_null() || serial.is_null() {
return nserror::NS_ERROR_FAILURE;
if !is_main_thread() {
return NS_ERROR_NOT_SAME_THREAD;
}
if issuer.is_null() || serial.is_null() || callback.is_null() {
return NS_ERROR_FAILURE;
}
let issuer_decoded = try_ns!(base64::decode(&*issuer));
let serial_decoded = try_ns!(base64::decode(&*serial));
let mut ss = try_ns!(self.security_state.write());
try_ns!(ss.open_db()); // this is a no-op if the DB is already open
match ss.set_enrollment(&issuer_decoded, &serial_decoded, state) {
Ok(_) => nserror::NS_OK,
_ => nserror::NS_ERROR_FAILURE,
}
let task = Box::new(SetEnrollmentTask::new(
&*callback,
&self.security_state,
issuer_decoded,
serial_decoded,
state,
));
let thread = try_ns!(self.thread.lock());
let runnable = try_ns!(TaskRunnable::new("SetEnrollment", task));
try_ns!(runnable.dispatch(&*thread));
NS_OK
}
unsafe fn SetWhitelist(
@ -647,18 +774,27 @@ impl CertStorage {
issuer: *const nsACString,
serial: *const nsACString,
state: i16,
callback: *const nsICertStorageCallback,
) -> nserror::nsresult {
if issuer.is_null() || serial.is_null() {
return nserror::NS_ERROR_FAILURE;
if !is_main_thread() {
return NS_ERROR_NOT_SAME_THREAD;
}
if issuer.is_null() || serial.is_null() || callback.is_null() {
return NS_ERROR_FAILURE;
}
let issuer_decoded = try_ns!(base64::decode(&*issuer));
let serial_decoded = try_ns!(base64::decode(&*serial));
let mut ss = try_ns!(self.security_state.write());
try_ns!(ss.open_db()); // this is a no-op if the DB is already open
match ss.set_whitelist(&issuer_decoded, &serial_decoded, state) {
Ok(_) => nserror::NS_OK,
_ => nserror::NS_ERROR_FAILURE,
}
let task = Box::new(SetWhitelistTask::new(
&*callback,
&self.security_state,
issuer_decoded,
serial_decoded,
state,
));
let thread = try_ns!(self.thread.lock());
let runnable = try_ns!(TaskRunnable::new("SetWhitelist", task));
try_ns!(runnable.dispatch(&*thread));
NS_OK
}
unsafe fn GetRevocationState(
@ -669,8 +805,10 @@ impl CertStorage {
pub_key_base64: *const nsACString,
state: *mut i16,
) -> nserror::nsresult {
// TODO (bug 1541212): We really want to restrict this to non-main-threads only, but we
// can't do so until bug 1406854 and bug 1534600 are fixed.
if issuer.is_null() || serial.is_null() || subject.is_null() || pub_key_base64.is_null() {
return nserror::NS_ERROR_FAILURE;
return NS_ERROR_FAILURE;
}
// TODO (bug 1535752): If we're calling this function when we already have binary data (e.g.
// in a TrustDomain::GetCertTrust callback), we should be able to pass in the binary data
@ -708,9 +846,9 @@ impl CertStorage {
) {
Ok(st) => {
*state = st;
nserror::NS_OK
NS_OK
}
_ => nserror::NS_ERROR_FAILURE,
_ => NS_ERROR_FAILURE,
}
}
@ -720,8 +858,11 @@ impl CertStorage {
serial: *const nsACString,
state: *mut i16,
) -> nserror::nsresult {
if is_main_thread() {
return NS_ERROR_NOT_SAME_THREAD;
}
if issuer.is_null() || serial.is_null() {
return nserror::NS_ERROR_FAILURE;
return NS_ERROR_FAILURE;
}
let issuer_decoded = try_ns!(base64::decode(&*issuer));
let serial_decoded = try_ns!(base64::decode(&*serial));
@ -742,9 +883,9 @@ impl CertStorage {
match ss.get_enrollment_state(&issuer_decoded, &serial_decoded) {
Ok(st) => {
*state = st;
nserror::NS_OK
NS_OK
}
_ => nserror::NS_ERROR_FAILURE,
_ => NS_ERROR_FAILURE,
}
}
@ -754,8 +895,11 @@ impl CertStorage {
serial: *const nsACString,
state: *mut i16,
) -> nserror::nsresult {
if is_main_thread() {
return NS_ERROR_NOT_SAME_THREAD;
}
if issuer.is_null() || serial.is_null() {
return nserror::NS_ERROR_FAILURE;
return NS_ERROR_FAILURE;
}
let issuer_decoded = try_ns!(base64::decode(&*issuer));
let serial_decoded = try_ns!(base64::decode(&*serial));
@ -776,9 +920,9 @@ impl CertStorage {
match ss.get_whitelist_state(&issuer_decoded, &serial_decoded) {
Ok(st) => {
*state = st;
nserror::NS_OK
NS_OK
}
_ => nserror::NS_ERROR_FAILURE,
_ => NS_ERROR_FAILURE,
}
}
@ -791,7 +935,7 @@ impl CertStorage {
Err(_) => false,
};
nserror::NS_OK
NS_OK
}
unsafe fn IsWhitelistFresh(&self, fresh: *mut bool) -> nserror::nsresult {
@ -803,7 +947,7 @@ impl CertStorage {
Err(_) => false,
};
nserror::NS_OK
NS_OK
}
unsafe fn IsEnrollmentFresh(&self, fresh: *mut bool) -> nserror::nsresult {
@ -815,7 +959,7 @@ impl CertStorage {
Err(_) => false,
};
nserror::NS_OK
NS_OK
}
unsafe fn Observe(
@ -830,7 +974,7 @@ impl CertStorage {
let prefs: RefPtr<nsIPrefBranch> = match (*subject).query_interface() {
Some(pb) => pb,
_ => return nserror::NS_ERROR_FAILURE,
_ => return NS_ERROR_FAILURE,
};
// Convert our wstring pref_name to a cstring (via nsCString's
@ -845,7 +989,7 @@ impl CertStorage {
let pref_name = match CString::new(name_string.as_str_unchecked()) {
Ok(n) => n,
_ => return nserror::NS_ERROR_FAILURE,
_ => return NS_ERROR_FAILURE,
};
let res = prefs.GetIntPref((&pref_name).as_ptr(), (&mut pref_value) as *mut i32);
@ -860,6 +1004,6 @@ impl CertStorage {
}
_ => (),
}
nserror::NS_OK
NS_OK
}
}

67
security/manager/ssl/nsICertStorage.idl
View File

@ -9,59 +9,90 @@
#define NS_CERTSTORAGE_CONTRACTID "@mozilla.org/security/certstorage;1"
%}
/**
* Callback type used to notify callers that an operation performed by
* nsICertStorage has completed. Indicates the result of the requested
* operation.
*/
[scriptable, function, uuid(3f8fe26a-a436-4ad4-9c1c-a53c60973c31)]
interface nsICertStorageCallback : nsISupports {
[must_use]
void done(in nsresult result);
};
[scriptable, uuid(327100a7-3401-45ef-b160-bf880f1016fd)]
interface nsICertStorage : nsISupports {
const short STATE_UNSET = 0;
const short STATE_ENFORCE = 1;
/**
* Set the revocation state of a certificate by issuer and serial number:
* issuer name (base-64 encoded DER) and serial number (base-64 encoded DER).
* Asynchronously set the revocation state of a certificate by issuer and
* serial number. Both issuer name and serial number are base64-encoded.
* Pass STATE_UNSET to mark the certificate as not revoked.
* Pass STATE_ENFORCE to mark the certificate as revoked.
* The given callback is called with the result of the operation when it
* completes.
* Must only be called from the main thread.
*/
[must_use]
void setRevocationByIssuerAndSerial(in ACString issuer,
in ACString serialNumber,
in short state);
in short state,
in nsICertStorageCallback callback);
/**
* Set the revocation state of a certificate by subject and public key hash:
* subject name (base-64 encoded DER) and hash of public key (base-64 encoded
* sha-256 hash of the public key).
* state (short) is STATE_ENFORCE for revoked certs, STATE_UNSET otherwise.
* Asynchronously set the revocation state of a certificate by subject and
* public key hash (the hash algorithm should be SHA-256). Both subject name
* and public key hash are base64-encoded.
* Pass STATE_UNSET to mark the certificate as not revoked.
* Pass STATE_ENFORCE to mark the certificate as revoked.
* The given callback is called with the result of the operation when it
* completes.
* Must only be called from the main thread.
*/
[must_use]
void setRevocationBySubjectAndPubKey(in ACString subject,
in ACString pubKeyHash,
in short state);
in short state,
in nsICertStorageCallback callback);
/**
* Set the whitelist state of an intermediate certificate by issuer and
* serial number:
* issuer name (base-64 encoded DER) and serial number (base-64 encoded DER).
* Asynchronously set the whitelist state of an intermediate certificate by
* issuer and serial number. Both are base64-encoded.
* state (short) is STATE_ENFORCE for whitelisted certs, STATE_UNSET otherwise.
* The given callback is called with the result of the operation when it
* completes.
* Must only be called from the main thread.
*/
[must_use]
void setWhitelist(in ACString issuer,
in ACString serialNumber,
in short state);
in short state,
in nsICertStorageCallback callback);
/**
* Set the CRLite enrollment state of a certificate by issuer and serial
* number:
* issuer name (base-64 encoded DER) and serial number (base-64 encoded DER).
* Asynchronously set the CRLite enrollment state of a certificate by issuer
* and serial number. Both are base64-encoded.
* state (short) is STATE_ENFORCE for enrolled certs, STATE_UNSET otherwise.
* The given callback is called with the result of the operation when it
* completes.
* Must only be called from the main thread.
*/
[must_use]
void setEnrollment(in ACString issuer,
in ACString serialNumber,
in short state);
in short state,
in nsICertStorageCallback callback);
/**
* Get the revocation state of a certificate.
* Get the revocation state of a certificate. STATE_UNSET indicates the
* certificate is not revoked. STATE_ENFORCE indicates the certificate is
* revoked.
* issuer - issuer name, DER, Base64 encoded
* serial - serial number, DER, Base64 encoded
* subject - subject name, DER, Base64 encoded
* pubkey - public key, DER, Base64 encoded
* Must not be called from the main thread.
*/
[must_use]
short getRevocationState(in ACString issuer,
@ -73,6 +104,7 @@ interface nsICertStorage : nsISupports {
* Get the CRLite enrollment status of a certificate.
* issuer - issuer name, DER, Base64 encoded
* serial - serial number, DER, Base64 encoded
* Must not be called from the main thread.
*/
[must_use]
short getEnrollmentState(in ACString issuer,
@ -82,6 +114,7 @@ interface nsICertStorage : nsISupports {
* Get the whitelist status of an intermediate certificate.
* issuer - issuer name, DER, Base64 encoded
* serial - serial number, DER, Base64 encoded
* Must not be called from the main thread.
*/
[must_use]
short getWhitelistState(in ACString issuer,

11
security/manager/ssl/tests/mochitest/browser/browser_certViewer.js
View File

@ -102,10 +102,13 @@ add_task(async function testRevoked() {
// likely) and subject (less likely).
let certBlocklist = Cc["@mozilla.org/security/certstorage;1"]
.getService(Ci.nsICertStorage);
certBlocklist.setRevocationBySubjectAndPubKey(
"MBIxEDAOBgNVBAMMB3Jldm9rZWQ=", // CN=revoked
"VCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8=", // hash of the shared key
Ci.nsICertStorage.STATE_ENFORCE); // yes, we want this to be revoked
let result = await new Promise((resolve) =>
certBlocklist.setRevocationBySubjectAndPubKey(
"MBIxEDAOBgNVBAMMB3Jldm9rZWQ=", // CN=revoked
"VCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8=", // hash of the shared key
Ci.nsICertStorage.STATE_ENFORCE, // yes, we want this to be revoked
resolve));
Assert.equal(result, Cr.NS_OK, "setting revocation state should succeed");
let cert = await readCertificate("revoked.pem", ",,");
let win = await displayCertificate(cert);
// As of bug 1312827, OneCRL only applies to TLS web server certificates, so

87
security/manager/ssl/tests/unit/test_cert_storage.js
View File

@ -108,23 +108,22 @@ const certBlocklistJSON = `{
"serialNumber": "Rym6o+VN9xgZXT/QLrvN/nv1ZN4="
},` +
// These items correspond to an entry in sample_revocations.txt where:
// isser name is "another imaginary issuer" base-64 encoded, and
// serialNumbers are:
// "serial2." base-64 encoded, and
// "another serial." base-64 encoded
// isser name is the base-64 encoded subject DN for the shared Test
// Intermediate and the serialNumbers are base-64 encoded 78 and 31,
// respectively.
// We need this to ensure that existing items are retained if they're
// also in the blocklist
` {
"id": "7",
"last_modified": 100000000000000000006,
"issuerName": "YW5vdGhlciBpbWFnaW5hcnkgaXNzdWVy",
"serialNumber": "c2VyaWFsMi4="
"issuerName": "MBwxGjAYBgNVBAMMEVRlc3QgSW50ZXJtZWRpYXRl",
"serialNumber": "Tg=="
},` +
` {
"id": "8",
"last_modified": 100000000000000000006,
"issuerName": "YW5vdGhlciBpbWFnaW5hcnkgaXNzdWVy",
"serialNumber": "YW5vdGhlciBzZXJpYWwu"
"issuerName": "MBwxGjAYBgNVBAMMEVRlc3QgSW50ZXJtZWRpYXRl",
"serialNumber": "Hw=="
},` +
// This item revokes same-issuer-ee.pem by subject and pubKeyHash.
` {
@ -185,12 +184,6 @@ function load_cert(cert, trust) {
addCertFromFile(certDB, file, trust);
}
function test_is_revoked(certList, issuerString, serialString, subjectString,
pubKeyString) {
return certList.getRevocationState(btoa(issuerString), btoa(serialString),
btoa(subjectString), btoa(pubKeyString)) == Ci.nsICertStorage.STATE_ENFORCE;
}
function fetch_blocklist() {
Services.prefs.setBoolPref("services.settings.load_dump", false);
Services.prefs.setBoolPref("services.settings.verify_signature", false);
@ -212,32 +205,39 @@ function run_test() {
.getService(Ci.nsICertStorage);
add_task(async function() {
// check some existing items in revocations.txt are blocked. Since the
// CertBlocklistItems don't know about the data they contain, we can use
// arbitrary data (not necessarily DER) to test if items are revoked or not.
// check some existing items in revocations.txt are blocked.
// This test corresponds to:
// issuer: c29tZSBpbWFnaW5hcnkgaXNzdWVy
// serial: c2VyaWFsLg==
ok(test_is_revoked(certList, "some imaginary issuer", "serial."),
"issuer / serial pair should be blocked");
// issuer: MBIxEDAOBgNVBAMMB1Rlc3QgQ0E= (CN=Test CA)
// serial: Kg== (42)
let file = "test_onecrl/ee-revoked-by-revocations-txt.pem";
await verify_cert(file, SEC_ERROR_REVOKED_CERTIFICATE);
// This test corresponds to:
// issuer: YW5vdGhlciBpbWFnaW5hcnkgaXNzdWVy
// serial: c2VyaWFsLg==
ok(test_is_revoked(certList, "another imaginary issuer", "serial."),
"issuer / serial pair should be blocked");
// issuer: MBwxGjAYBgNVBAMMEVRlc3QgSW50ZXJtZWRpYXRl (CN=Test Intermediate)
// serial: Tg== (78)
file = "test_onecrl/another-ee-revoked-by-revocations-txt.pem";
await verify_cert(file, SEC_ERROR_REVOKED_CERTIFICATE);
// And this test corresponds to:
// issuer: YW5vdGhlciBpbWFnaW5hcnkgaXNzdWVy
// serial: c2VyaWFsMi4=
// issuer: MBwxGjAYBgNVBAMMEVRlc3QgSW50ZXJtZWRpYXRl (CN=Test Intermediate)
// serial: Hw== (31)
// (we test this issuer twice to ensure we can read multiple serials)
ok(test_is_revoked(certList, "another imaginary issuer", "serial2."),
"issuer / serial pair should be blocked");
file = "test_onecrl/another-ee-revoked-by-revocations-txt-serial-2.pem";
await verify_cert(file, SEC_ERROR_REVOKED_CERTIFICATE);
// Test that a certificate revoked by subject and public key hash in
// revocations.txt is revoked
// subject: MCsxKTAnBgNVBAMMIEVFIFJldm9rZWQgQnkgU3ViamVjdCBhbmQgUHViS2V5
// (CN=EE Revoked By Subject and PubKey)
// pubkeyhash: VCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8 (this is the
// shared RSA SPKI)
file = "test_onecrl/ee-revoked-by-subject-and-pubkey.pem";
await verify_cert(file, SEC_ERROR_REVOKED_CERTIFICATE);
// Soon we'll load a blocklist which revokes test-int.pem, which issued
// test-int-ee.pem.
// Check the cert validates before we load the blocklist
let file = "test_onecrl/test-int-ee.pem";
file = "test_onecrl/test-int-ee.pem";
await verify_cert(file, PRErrorCodeSuccess);
// The blocklist also revokes other-test-ca.pem, which issued
@ -257,24 +257,23 @@ function run_test() {
add_task(async function() {
// The blocklist will be loaded now. Let's check the data is sane.
// In particular, we should still have the revoked issuer / serial pair
// that was in both revocations.txt and the blocklist.
ok(test_is_revoked(certList, "another imaginary issuer", "serial2."),
"issuer / serial pair should be blocked");
// that was in revocations.txt but not the blocklist.
let file = "test_onecrl/ee-revoked-by-revocations-txt.pem";
await verify_cert(file, SEC_ERROR_REVOKED_CERTIFICATE);
// Check that both serials in the certItem with multiple serials were read
// properly
ok(test_is_revoked(certList, "another imaginary issuer", "serial2."),
"issuer / serial pair should be blocked");
ok(test_is_revoked(certList, "another imaginary issuer", "another serial."),
"issuer / serial pair should be blocked");
// We should also still have the revoked issuer / serial pairs that were in
// revocations.txt and are also in the blocklist.
file = "test_onecrl/another-ee-revoked-by-revocations-txt.pem";
await verify_cert(file, SEC_ERROR_REVOKED_CERTIFICATE);
file = "test_onecrl/another-ee-revoked-by-revocations-txt-serial-2.pem";
await verify_cert(file, SEC_ERROR_REVOKED_CERTIFICATE);
// test a subject / pubKey revocation
ok(test_is_revoked(certList, "nonsense", "more nonsense",
"some imaginary subject", "some imaginary pubkey"),
"issuer / serial pair should be blocked");
// The cert revoked by subject and pubkeyhash should still be revoked.
file = "test_onecrl/ee-revoked-by-subject-and-pubkey.pem";
await verify_cert(file, SEC_ERROR_REVOKED_CERTIFICATE);
// Check the blocklisted intermediate now causes a failure
let file = "test_onecrl/test-int-ee.pem";
file = "test_onecrl/test-int-ee.pem";
await verify_cert(file, SEC_ERROR_REVOKED_CERTIFICATE);
await verify_non_tls_usage_succeeds(file);

17
security/manager/ssl/tests/unit/test_onecrl/another-ee-revoked-by-revocations-txt-serial-2.pem Normal file
View File

@ -0,0 +1,17 @@
-----BEGIN CERTIFICATE-----
MIICyTCCAbGgAwIBAgIBHzANBgkqhkiG9w0BAQsFADAcMRowGAYDVQQDDBFUZXN0
IEludGVybWVkaWF0ZTAiGA8yMDE3MTEyNzAwMDAwMFoYDzIwMjAwMjA1MDAwMDAw
WjAwMS4wLAYDVQQDDCVBbm90aGVyIEVFIFJldm9rZWQgYnkgcmV2b2NhdGlvbnMu
dHh0MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAuohRqESOFtZB/W62
iAY2ED08E9nq5DVKtOz1aFdsJHvBxyWo4NgfvbGcBptuGobya+KvWnVramRxCHql
WqdFh/cc1SScAn7NQ/weadA4ICmTqyDDSeTbuUzCa2wO7RWCD/F+rWkasdMCOosq
Qe6ncOAPDY39ZgsrsCSSpH25iGF5kLFXkD3SO8XguEgfqDfTiEPvJxbYVbdmWqp+
ApAvOnsQgAYkzBxsl62WYVu34pYSwHUxowyR3bTK9/ytHSXTCe+5Fw6naOGzey8i
b2njtIqVYR3uJtYlnauRCE42yxwkBCy/Fosv5fGPmRcxuLP+SSP6clHEMdUDrNoY
CjXtjQIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQAIQ+d/lUQIsj5fM+TEUfSMru53
QD6JMJ3tuX0Of2UVU3U+EpIzV4xR55XgAmLLOotLfikYpsckFY2c5Yp2FnO0poFB
sOru9szMyC+B9Wj10Le1sa5HUVe/vj5vj1NICcM7MV4VuAYsx2viC96m818Rxd99
+Zz/BzbWQS+NqsKrrepjOMPReeSFsk3X51f2s0hTv5rPNW3UMVuUXRFEAQkhXIMW
A41lBzx1ohje7VauCWZnyO6dn9rSOTZ0ma397R65p8I7P0R1coENktSUP4e3x1js
dxjvoxHhrZ8Lo0MXnnU0DBkYu7UcgKSQZP+NkqclqK7B4UScYx0YY15VkvMU
-----END CERTIFICATE-----

3
security/manager/ssl/tests/unit/test_onecrl/another-ee-revoked-by-revocations-txt-serial-2.pem.certspec Normal file
View File

@ -0,0 +1,3 @@
issuer:Test Intermediate
subject:Another EE Revoked by revocations.txt
serialNumber:31

17
security/manager/ssl/tests/unit/test_onecrl/another-ee-revoked-by-revocations-txt.pem Normal file
View File

@ -0,0 +1,17 @@
-----BEGIN CERTIFICATE-----
MIICyTCCAbGgAwIBAgIBTjANBgkqhkiG9w0BAQsFADAcMRowGAYDVQQDDBFUZXN0
IEludGVybWVkaWF0ZTAiGA8yMDE3MTEyNzAwMDAwMFoYDzIwMjAwMjA1MDAwMDAw
WjAwMS4wLAYDVQQDDCVBbm90aGVyIEVFIFJldm9rZWQgYnkgcmV2b2NhdGlvbnMu
dHh0MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAuohRqESOFtZB/W62
iAY2ED08E9nq5DVKtOz1aFdsJHvBxyWo4NgfvbGcBptuGobya+KvWnVramRxCHql
WqdFh/cc1SScAn7NQ/weadA4ICmTqyDDSeTbuUzCa2wO7RWCD/F+rWkasdMCOosq
Qe6ncOAPDY39ZgsrsCSSpH25iGF5kLFXkD3SO8XguEgfqDfTiEPvJxbYVbdmWqp+
ApAvOnsQgAYkzBxsl62WYVu34pYSwHUxowyR3bTK9/ytHSXTCe+5Fw6naOGzey8i
b2njtIqVYR3uJtYlnauRCE42yxwkBCy/Fosv5fGPmRcxuLP+SSP6clHEMdUDrNoY
CjXtjQIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQBOCgOoQ7mFPuX2cX34hxKKhwyQ
erXW5Z9PssdiEPeOUDFsk9ZN0i3G3pfD4+dz1ZZgLVs83OJLLrs3Ij12EIkcO04J
hbbw2BGtGk0MbFAwr1xGTm9Anz9N5rHu+OpHuvqeix3mbLlbm8otIUFLDp5pQ2Ql
GtKvrWpS0eHd8I0Q7I9bGAdrS4mq15EX8AHbOu2BwmqxV1Aw/oRDs1twZn6NijA7
2+0ZFREpIquy1NcR5AQCagKTAHz6SjybYzyY8aYdjuU+D5bnpE2FScmeu0RBV6F0
o2Jc0zICufQstXjB9/9ucHWZ8YMWz/yyjbhOGnu5vWzeoFKBToP1uDdL3R5L
-----END CERTIFICATE-----

3
security/manager/ssl/tests/unit/test_onecrl/another-ee-revoked-by-revocations-txt.pem.certspec Normal file
View File

@ -0,0 +1,3 @@
issuer:Test Intermediate
subject:Another EE Revoked by revocations.txt
serialNumber:78

17
security/manager/ssl/tests/unit/test_onecrl/ee-revoked-by-revocations-txt.pem Normal file
View File

@ -0,0 +1,17 @@
-----BEGIN CERTIFICATE-----
MIICtzCCAZ+gAwIBAgIBKjANBgkqhkiG9w0BAQsFADASMRAwDgYDVQQDDAdUZXN0
IENBMCIYDzIwMTcxMTI3MDAwMDAwWhgPMjAyMDAyMDUwMDAwMDBaMCgxJjAkBgNV
BAMMHUVFIFJldm9rZWQgYnkgcmV2b2NhdGlvbnMudHh0MIIBIjANBgkqhkiG9w0B
AQEFAAOCAQ8AMIIBCgKCAQEAuohRqESOFtZB/W62iAY2ED08E9nq5DVKtOz1aFds
JHvBxyWo4NgfvbGcBptuGobya+KvWnVramRxCHqlWqdFh/cc1SScAn7NQ/weadA4
ICmTqyDDSeTbuUzCa2wO7RWCD/F+rWkasdMCOosqQe6ncOAPDY39ZgsrsCSSpH25
iGF5kLFXkD3SO8XguEgfqDfTiEPvJxbYVbdmWqp+ApAvOnsQgAYkzBxsl62WYVu3
4pYSwHUxowyR3bTK9/ytHSXTCe+5Fw6naOGzey8ib2njtIqVYR3uJtYlnauRCE42
yxwkBCy/Fosv5fGPmRcxuLP+SSP6clHEMdUDrNoYCjXtjQIDAQABMA0GCSqGSIb3
DQEBCwUAA4IBAQCJp/S6OqKCcf1vjoxGs2jvp6sN7+TduI41X0HGbJYSY/iFOV0b
dW1ndrjB0oVuQzYUYaNLGCJpA+v8iA7fNy3kWwgpFjf4bisKZaMaKMIeJAciD8sq
qdj8GamkrHkidPLEVX3pkxRYh+pk3EF+EKTfc3+OHj7QzUR/oddC1iNUlVt1MdrJ
Mkd+HSibz44RSaSSD6Pw5/YhvQ6TMh1H3stnCo+YNlIqY2zVxl1/trgOoiUmGo/t
ha4Bgp6Dje6p7LCj4b/9XR9BFDUp+kY2dB9uRcJXBBH5pl5w137cnMvrcO7skNra
1ynY/rJyDD8Gv7S99+a2xPC2PWOTEoJbLoDU
-----END CERTIFICATE-----

3
security/manager/ssl/tests/unit/test_onecrl/ee-revoked-by-revocations-txt.pem.certspec Normal file
View File

@ -0,0 +1,3 @@
issuer:Test CA
subject:EE Revoked by revocations.txt
serialNumber:42

18
security/manager/ssl/tests/unit/test_onecrl/ee-revoked-by-subject-and-pubkey.pem Normal file
View File

@ -0,0 +1,18 @@
-----BEGIN CERTIFICATE-----
MIICzTCCAbWgAwIBAgIUP6M5biClp7bgN4tr/Qf9l7YUKl0wDQYJKoZIhvcNAQEL
BQAwEjEQMA4GA1UEAwwHVGVzdCBDQTAiGA8yMDE3MTEyNzAwMDAwMFoYDzIwMjAw
MjA1MDAwMDAwWjArMSkwJwYDVQQDDCBFRSBSZXZva2VkIEJ5IFN1YmplY3QgYW5k
IFB1YktleTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALqIUahEjhbW
Qf1utogGNhA9PBPZ6uQ1SrTs9WhXbCR7wcclqODYH72xnAabbhqG8mvir1p1a2pk
cQh6pVqnRYf3HNUknAJ+zUP8HmnQOCApk6sgw0nk27lMwmtsDu0Vgg/xfq1pGrHT
AjqLKkHup3DgDw2N/WYLK7AkkqR9uYhheZCxV5A90jvF4LhIH6g304hD7ycW2FW3
ZlqqfgKQLzp7EIAGJMwcbJetlmFbt+KWEsB1MaMMkd20yvf8rR0l0wnvuRcOp2jh
s3svIm9p47SKlWEd7ibWJZ2rkQhONsscJAQsvxaLL+Xxj5kXMbiz/kkj+nJRxDHV
A6zaGAo17Y0CAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAUejs7I3P35tHViuEJbvp
UxvnEqhKn/5TVA1YadoMrS3pEAw8GELWWKGpO4ox+6t7V9geiLKuOtqWIpsDCCdn
MYUzAd979FA25h4F3GX+9r4RrW8oy+G9+UVsJ8lUspxX5f8xYM3kpKwNYdTsLYm2
d94jQfCfEkCD2o8Aq/SZ05iJNTeHKoZDf8CGCFm7N92sOk9fVKA0SmLHJIjv2fWD
drZpVzVd98rBGSTbXZ28xHqOGOVAwjRB2S0sIljNbKEKVkMLPvZluaS0ATFE8uqX
yT6l7b0YH8M7JUhHjXMud2/D+gu0wdPqNPkEHQi7ReY5vjhiBkfPGJlRzRTkXyz9
Ew==
-----END CERTIFICATE-----

2
security/manager/ssl/tests/unit/test_onecrl/ee-revoked-by-subject-and-pubkey.pem.certspec Normal file
View File

@ -0,0 +1,2 @@
issuer:Test CA
subject:EE Revoked By Subject and PubKey

4
security/manager/ssl/tests/unit/test_onecrl/moz.build
View File

@ -6,6 +6,10 @@
# Temporarily disabled. See bug 1256495.
#test_certificates = (
# 'another-ee-revoked-by-revocations-txt-serial-2.pem',
# 'another-ee-revoked-by-revocations-txt.pem',
# 'ee-revoked-by-revocations-txt.pem',
# 'ee-revoked-by-subject-and-pubkey.pem',
# 'same-issuer-ee.pem',
# 'test-int-ee.pem',
#)

26
security/manager/ssl/tests/unit/test_onecrl/sample_revocations.txt
View File

@ -19,19 +19,21 @@ Neither is this issuer, though the serial is fine
dGVzdA==
in this case, issuer is fine but not the serial
# Next two entries; we can add valid base-64 encoded data for some basic tests:
# issuer is "some imaginary issuer" base-64 encoded
# and serial "serial." base-64 encoded
c29tZSBpbWFnaW5hcnkgaXNzdWVy
c2VyaWFsLg==
# issuer is "another imaginary issuer" base-64 encoded
# serials are "serial." and "serial2." base-64 encoded
YW5vdGhlciBpbWFnaW5hcnkgaXNzdWVy
c2VyaWFsLg==
# issuer is the base-64 encoded subject DN for the shared Test CA
# serial is the base-64 encoded integer 42
MBIxEDAOBgNVBAMMB1Rlc3QgQ0E=
Kg==
# issuer is the base-64 encoded subject DN for the shared Test Intermediate
# the first serial is the base-64 encoded integer 78
# the second serial is the base-64 encoded integer 31
MBwxGjAYBgNVBAMMEVRlc3QgSW50ZXJtZWRpYXRl
Tg==
Hw==
c2VyaWFsMi4=
# subject is "some imaginary subject", base-64 encoded
# pubKeyHash is the sha256 hash of "some imaginary pubkey" base-64 encoded
c29tZSBpbWFnaW5hcnkgc3ViamVjdA==
blBNgTxORaii2Sqe9bQcYsmfJ3kiXOLiKLzQNJ2wZYE=
# subject is base-64 encoded subject DN "CN=EE Revoked By Subject and PubKey"
# pubKeyHash is the base-64 encoded sha256 hash of the shared RSA SPKI
MCsxKTAnBgNVBAMMIEVFIFJldm9rZWQgQnkgU3ViamVjdCBhbmQgUHViS2V5
VCIlmPM9NkgFQtrs4Oa5TeFcDu6MWRTKSNdePEhOgD8
# and some more data to ensure that mixed items don't cause parsing failure
a DN
a serial

36
services/common/blocklist-clients.js
View File

@ -34,6 +34,18 @@ const PREF_BLOCKLIST_GFX_COLLECTION = "services.blocklist.gfx.collectio
const PREF_BLOCKLIST_GFX_CHECKED_SECONDS = "services.blocklist.gfx.checked";
const PREF_BLOCKLIST_GFX_SIGNER = "services.blocklist.gfx.signer";
function setRevocationByIssuerAndSerial(certStorage, issuer, serial, state) {
return new Promise((resolve) =>
certStorage.setRevocationByIssuerAndSerial(issuer, serial, state, resolve)
);
}
function setRevocationBySubjectAndPubKey(certStorage, subject, pubKey, state) {
return new Promise((resolve) =>
certStorage.setRevocationBySubjectAndPubKey(subject, pubKey, state, resolve)
);
}
/**
* Revoke the appropriate certificates based on the records from the blocklist.
*
@ -44,14 +56,12 @@ async function updateCertBlocklist({ data: { created, updated, deleted } }) {
.getService(Ci.nsICertStorage);
for (let item of deleted) {
if (item.issuerName && item.serialNumber) {
certList.setRevocationByIssuerAndSerial(item.issuerName,
item.serialNumber,
Ci.nsICertStorage.STATE_UNSET);
} else if (item.subject && item.pubKeyHash) {
certList.setRevocationBySubjectAndPubKey(item.subject,
item.pubKeyHash,
Ci.nsICertStorage.STATE_UNSET);
}
await setRevocationByIssuerAndSerial(certList, item.issuerName, item.serialNumber,
Ci.nsICertStorage.STATE_UNSET);
} else if (item.subject && item.pubKeyHash) {
await setRevocationBySubjectAndPubKey(certList, item.subject, item.pubKeyHash,
Ci.nsICertStorage.STATE_UNSET);
}
}
const toAdd = created.concat(updated.map(u => u.new));
@ -59,13 +69,11 @@ async function updateCertBlocklist({ data: { created, updated, deleted } }) {
for (let item of toAdd) {
try {
if (item.issuerName && item.serialNumber) {
certList.setRevocationByIssuerAndSerial(item.issuerName,
item.serialNumber,
Ci.nsICertStorage.STATE_ENFORCE);
await setRevocationByIssuerAndSerial(certList, item.issuerName, item.serialNumber,
Ci.nsICertStorage.STATE_ENFORCE);
} else if (item.subject && item.pubKeyHash) {
certList.setRevocationBySubjectAndPubKey(item.subject,
item.pubKeyHash,
Ci.nsICertStorage.STATE_ENFORCE);
await setRevocationBySubjectAndPubKey(certList, item.subject, item.pubKeyHash,
Ci.nsICertStorage.STATE_ENFORCE);
}
} catch (e) {
// prevent errors relating to individual blocklist entries from