Bug 1182740 - treat keypair and certificate as an atomic pair to avoid invalid assertions. r=stomlinson

This commit is contained in:
Mark Hammond 2015-07-29 16:06:29 +10:00
parent 6a36b7ba32
commit 590111aca0
4 changed files with 212 additions and 115 deletions

View File

@ -121,12 +121,8 @@ function configureFxAccountIdentity() {
storageManager.initialize(user);
return new AccountState(storageManager);
},
getCertificate(data, keyPair, mustBeValidUntil) {
this.cert = {
validUntil: this.now() + 10000,
cert: "certificate",
};
return Promise.resolve(this.cert.cert);
_getAssertion(audience) {
return Promise.resolve("assertion");
},
getCertificateSigned() {
return Promise.resolve();

View File

@ -499,60 +499,20 @@ FxAccountsInternal.prototype = {
})
},
/**
* returns a promise that fires with the keypair.
*/
getKeyPair: Task.async(function* (mustBeValidUntil) {
// If the debugging pref to ignore cached authentication credentials is set for Sync,
// then don't use any cached key pair, i.e., generate a new one and get it signed.
// The purpose of this pref is to expedite any auth errors as the result of a
// expired or revoked FxA session token, e.g., from resetting or changing the FxA
// password.
let ignoreCachedAuthCredentials = false;
try {
ignoreCachedAuthCredentials = Services.prefs.getBoolPref("services.sync.debug.ignoreCachedAuthCredentials");
} catch(e) {
// Pref doesn't exist
}
let currentState = this.currentAccountState;
let accountData = yield currentState.getUserAccountData("keyPair");
if (!ignoreCachedAuthCredentials && accountData.keyPair && (accountData.keyPair.validUntil > mustBeValidUntil)) {
log.debug("getKeyPair: already have a keyPair");
return accountData.keyPair.keyPair;
}
// Otherwse, create a keypair and set validity limit.
let willBeValidUntil = this.now() + KEY_LIFETIME;
let kp = yield new Promise((resolve, reject) => {
jwcrypto.generateKeyPair("DS160", (err, kp) => {
if (err) {
return reject(err);
}
log.debug("got keyPair");
let toUpdate = {
keyPair: {
keyPair: kp,
validUntil: willBeValidUntil
},
cert: null
};
currentState.updateUserAccountData(toUpdate).then(() => {
resolve(kp);
}).catch(err => {
log.error("Failed to update account data with keypair and cert");
});
});
});
return kp;
}),
/**
* returns a promise that fires with the assertion. If there is no verified
* signed-in user, fires with null.
*/
getAssertion: function getAssertion(audience) {
return this._getAssertion(audience);
},
// getAssertion() is "public" so screws with our mock story. This
// implementation method *can* be (and is) mocked by tests.
_getAssertion: function _getAssertion(audience) {
log.debug("enter getAssertion()");
let currentState = this.currentAccountState;
let mustBeValidUntil = this.now() + ASSERTION_USE_PERIOD;
return currentState.getUserAccountData().then(data => {
if (!data) {
// No signed-in user
@ -562,12 +522,17 @@ FxAccountsInternal.prototype = {
// Signed-in user has not verified email
return null;
}
return this.getKeyPair(mustBeValidUntil).then(keyPair => {
return this.getCertificate(data, keyPair, mustBeValidUntil)
.then(cert => {
return this.getAssertionFromCert(data, keyPair, cert, audience);
});
});
if (!data.sessionToken) {
// can't get a signed certificate without a session token, but that
// should be impossible - make log noise about it.
log.error("getAssertion called without a session token!");
return null;
}
return this.getKeypairAndCertificate(currentState).then(
({keyPair, certificate}) => {
return this.getAssertionFromCert(data, keyPair, certificate, audience);
}
);
}).then(result => currentState.resolve(result));
},
@ -832,34 +797,91 @@ FxAccountsInternal.prototype = {
},
/**
* returns a promise that fires with a certificate.
* returns a promise that fires with {keyPair, certificate}.
*/
getCertificate: Task.async(function* (data, keyPair, mustBeValidUntil) {
// TODO: get the lifetime from the cert's .exp field
let currentState = this.currentAccountState;
let accountData = yield currentState.getUserAccountData("cert");
if (accountData.cert && accountData.cert.validUntil > mustBeValidUntil) {
log.debug(" getCertificate already had one");
return accountData.cert.cert;
getKeypairAndCertificate: Task.async(function* (currentState) {
// If the debugging pref to ignore cached authentication credentials is set for Sync,
// then don't use any cached key pair/certificate, i.e., generate a new
// one and get it signed.
// The purpose of this pref is to expedite any auth errors as the result of a
// expired or revoked FxA session token, e.g., from resetting or changing the FxA
// password.
let ignoreCachedAuthCredentials = false;
try {
ignoreCachedAuthCredentials = Services.prefs.getBoolPref("services.sync.debug.ignoreCachedAuthCredentials");
} catch(e) {
// Pref doesn't exist
}
let mustBeValidUntil = this.now() + ASSERTION_USE_PERIOD;
let accountData = yield currentState.getUserAccountData(["cert", "keyPair", "sessionToken"]);
let keyPairValid = !ignoreCachedAuthCredentials &&
accountData.keyPair &&
(accountData.keyPair.validUntil > mustBeValidUntil);
let certValid = !ignoreCachedAuthCredentials &&
accountData.cert &&
(accountData.cert.validUntil > mustBeValidUntil);
// TODO: get the lifetime from the cert's .exp field
if (keyPairValid && certValid) {
log.debug("getKeypairAndCertificate: already have keyPair and certificate");
return {
keyPair: accountData.keyPair.rawKeyPair,
certificate: accountData.cert.rawCert
}
}
// We are definately going to generate a new cert, either because it has
// already expired, or the keyPair has - and a new keyPair means we must
// generate a new cert.
// A keyPair has a longer lifetime than a cert, so it's possible we will
// have a valid keypair but an expired cert, which means we can skip
// keypair generation.
// Either way, the cert will require hitting the network, so bail now if
// we know that's going to fail.
if (Services.io.offline) {
throw new Error(ERROR_OFFLINE);
}
let willBeValidUntil = this.now() + CERT_LIFETIME;
let cert = yield this.getCertificateSigned(data.sessionToken,
keyPair.serializedPublicKey,
CERT_LIFETIME);
log.debug("getCertificate got a new one: " + !!cert);
if (cert) {
let keyPair;
if (keyPairValid) {
keyPair = accountData.keyPair;
} else {
let keyWillBeValidUntil = this.now() + KEY_LIFETIME;
keyPair = yield new Promise((resolve, reject) => {
jwcrypto.generateKeyPair("DS160", (err, kp) => {
if (err) {
return reject(err);
}
log.debug("got keyPair");
resolve({
rawKeyPair: kp,
validUntil: keyWillBeValidUntil,
});
});
});
}
// and generate the cert.
let certWillBeValidUntil = this.now() + CERT_LIFETIME;
let certificate = yield this.getCertificateSigned(accountData.sessionToken,
keyPair.rawKeyPair.serializedPublicKey,
CERT_LIFETIME);
log.debug("getCertificate got a new one: " + !!certificate);
if (certificate) {
// Cache both keypair and cert.
let toUpdate = {
keyPair,
cert: {
cert: cert,
validUntil: willBeValidUntil
}
rawCert: certificate,
validUntil: certWillBeValidUntil,
},
};
yield currentState.updateUserAccountData(toUpdate);
}
return cert;
return {
keyPair: keyPair.rawKeyPair,
certificate: certificate,
}
}),
getUserAccountData: function() {

View File

@ -166,6 +166,22 @@ function MockFxAccounts() {
});
}
/*
* Some tests want a "real" fxa instance - however, we still mock the storage
* to keep the tests fast on b2g.
*/
function MakeFxAccounts(internal = {}) {
if (!internal.newAccountState) {
// we use a real accountState but mocked storage.
internal.newAccountState = function(credentials) {
let storage = new MockStorageManager();
storage.initialize(credentials);
return new AccountState(storage);
};
}
return new FxAccounts(internal);
}
add_test(function test_non_https_remote_server_uri_with_requireHttps_false() {
Services.prefs.setBoolPref(
"identity.fxaccounts.allowHttp",
@ -195,16 +211,8 @@ add_test(function test_non_https_remote_server_uri() {
});
add_task(function test_get_signed_in_user_initially_unset() {
// This test, unlike many of the the rest, uses a (largely) un-mocked
// FxAccounts instance.
let account = new FxAccounts({
newAccountState(credentials) {
// we use a real accountState but mocked storage.
let storage = new MockStorageManager();
storage.initialize(credentials);
return new AccountState(storage);
},
});
_("Check getSignedInUser initially and after signout reports no user");
let account = MakeFxAccounts();
let credentials = {
email: "foo@example.com",
uid: "1234@lcip.org",
@ -241,38 +249,22 @@ add_task(function test_get_signed_in_user_initially_unset() {
do_check_eq(result, null);
});
add_task(function* test_getCertificate() {
_("getCertificate()");
// This test, unlike many of the the rest, uses a (largely) un-mocked
// FxAccounts instance.
// We do mock the storage to keep the test fast on b2g.
let fxa = new FxAccounts({
newAccountState(credentials) {
// we use a real accountState but mocked storage.
let storage = new MockStorageManager();
storage.initialize(credentials);
return new AccountState(storage);
},
});
add_task(function* test_getCertificateOffline() {
_("getCertificateOffline()");
let fxa = MakeFxAccounts();
let credentials = {
email: "foo@example.com",
uid: "1234@lcip.org",
assertion: "foobar",
sessionToken: "dead",
kA: "beef",
kB: "cafe",
verified: true
verified: true,
};
yield fxa.setSignedInUser(credentials);
// Test that an expired cert throws if we're offline.
fxa.internal.currentAccountState.cert = {
validUntil: Date.parse("Mon, 13 Jan 2000 21:45:06 GMT")
};
let offline = Services.io.offline;
Services.io.offline = true;
// This call would break from missing parameters ...
yield fxa.internal.getCertificate().then(
yield fxa.internal.getKeypairAndCertificate(fxa.internal.currentAccountState).then(
result => {
Services.io.offline = offline;
do_throw("Unexpected success");
@ -283,8 +275,99 @@ add_task(function* test_getCertificate() {
do_check_eq(err, "Error: OFFLINE");
}
);
yield fxa.signOut(/*localOnly = */true);
});
add_task(function* test_getCertificateCached() {
_("getCertificateCached()");
let fxa = MakeFxAccounts();
let credentials = {
email: "foo@example.com",
uid: "1234@lcip.org",
sessionToken: "dead",
verified: true,
// A cached keypair and cert that remain valid.
keyPair: {
validUntil: Date.now() + KEY_LIFETIME + 10000,
rawKeyPair: "good-keypair",
},
cert: {
validUntil: Date.now() + CERT_LIFETIME + 10000,
rawCert: "good-cert",
},
};
yield fxa.setSignedInUser(credentials);
let {keyPair, certificate} = yield fxa.internal.getKeypairAndCertificate(fxa.internal.currentAccountState);
// should have the same keypair and cert.
do_check_eq(keyPair, credentials.keyPair.rawKeyPair);
do_check_eq(certificate, credentials.cert.rawCert);
yield fxa.signOut(/*localOnly = */true);
});
add_task(function* test_getCertificateExpiredCert() {
_("getCertificateExpiredCert()");
let fxa = MakeFxAccounts({
getCertificateSigned() {
return "new cert";
}
});
let credentials = {
email: "foo@example.com",
uid: "1234@lcip.org",
sessionToken: "dead",
verified: true,
// A cached keypair that remains valid.
keyPair: {
validUntil: Date.now() + KEY_LIFETIME + 10000,
rawKeyPair: "good-keypair",
},
// A cached certificate which has expired.
cert: {
validUntil: Date.parse("Mon, 13 Jan 2000 21:45:06 GMT"),
rawCert: "expired-cert",
},
};
yield fxa.setSignedInUser(credentials);
let {keyPair, certificate} = yield fxa.internal.getKeypairAndCertificate(fxa.internal.currentAccountState);
// should have the same keypair but a new cert.
do_check_eq(keyPair, credentials.keyPair.rawKeyPair);
do_check_neq(certificate, credentials.cert.rawCert);
yield fxa.signOut(/*localOnly = */true);
});
add_task(function* test_getCertificateExpiredKeypair() {
_("getCertificateExpiredKeypair()");
let fxa = MakeFxAccounts({
getCertificateSigned() {
return "new cert";
},
});
let credentials = {
email: "foo@example.com",
uid: "1234@lcip.org",
sessionToken: "dead",
verified: true,
// A cached keypair that has expired.
keyPair: {
validUntil: Date.now() - 1000,
rawKeyPair: "expired-keypair",
},
// A cached certificate which remains valid.
cert: {
validUntil: Date.now() + CERT_LIFETIME + 10000,
rawCert: "expired-cert",
},
};
yield fxa.setSignedInUser(credentials);
let {keyPair, certificate} = yield fxa.internal.getKeypairAndCertificate(fxa.internal.currentAccountState);
// even though the cert was valid, the fact the keypair was not means we
// should have fetched both.
do_check_neq(keyPair, credentials.keyPair.rawKeyPair);
do_check_neq(certificate, credentials.cert.rawCert);
yield fxa.signOut(/*localOnly = */true);
});
// Sanity-check that our mocked client is working correctly
add_test(function test_client_mock() {

View File

@ -184,14 +184,10 @@ this.configureFxAccountIdentity = function(authService,
let accountState = new AccountState(storageManager);
return accountState;
},
getCertificate(data, keyPair, mustBeValidUntil) {
let cert = {
validUntil: this.now() + CERT_LIFETIME,
cert: "certificate",
};
this.currentAccountState.updateUserAccountData({cert: cert});
return Promise.resolve(cert.cert);
_getAssertion(audience) {
return Promise.resolve("assertion");
},
};
fxa = new FxAccounts(MockInternal);