Bug 1714621 - Add functionality to verify GMP's update xml content signatures. r=Gijs,robwu a=RyanVM

Also driveby fix a log string to print the proper function name.

Differential Revision: https://phabricator.services.mozilla.com/D127566
This commit is contained in:
Bryce Seager van Dyk 2021-10-27 17:51:28 +00:00
parent 7bfd5085af
commit 5bc14d4dde
4 changed files with 233 additions and 14 deletions

View File

@ -4128,10 +4128,26 @@ pref("browser.search.separatePrivateDefault.ui.enabled", false);
// Update service URL for GMP install/updates:
pref("media.gmp-manager.url", "https://aus5.mozilla.org/update/3/GMP/%VERSION%/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/update.xml");
// When |media.gmp-manager.checkContentSignature| is true, then the reply
// containing the update xml file is expected to provide a content signature
// header. Information from this header will be used to validate the response.
// If this header is not present, is malformed, or cannot be determined as
// valid then the update will fail.
#ifdef EARLY_BETA_OR_EARLIER
// The plan is to have the feature gated by this pref to eventually replace
// the features controlled by the media.gmp-manager.cert.* prefs. Once that
// happens we can remove related code and prefs, but while testing we'll use
// this to gate (see bug 1714621 for more info).
pref("media.gmp-manager.checkContentSignature", true);
#endif
// When |media.gmp-manager.cert.requireBuiltIn| is true or not specified the
// final certificate and all certificates the connection is redirected to before
// the final certificate for the url specified in the |media.gmp-manager.url|
// preference must be built-in.
// preference must be built-in. The check related to this pref is not done if
// |media.gmp-manager.checkContentSignature| is set to true (the content
// signature check provides protection that supersedes the built in
// requirement).
pref("media.gmp-manager.cert.requireBuiltIn", true);
// The |media.gmp-manager.certs.| preference branch contains branches that are
@ -4146,8 +4162,10 @@ pref("media.gmp-manager.cert.requireBuiltIn", true);
// If these conditions aren't met it will be treated the same as when there is
// no update available. This validation will not be performed when the
// |media.gmp-manager.url.override| user preference has been set for testing updates or
// when the |media.gmp-manager.cert.checkAttributes| preference is set to false. Also,
// the |media.gmp-manager.url.override| preference should ONLY be used for testing.
// when the |media.gmp-manager.cert.checkAttributes| preference is set to false.
// This check will also not be done if the |media.gmp-manager.checkContentSignature|
// pref is set to true. Also, the |media.gmp-manager.url.override| preference should
// ONLY be used for testing.
// IMPORTANT! app.update.certs.* prefs should also be updated if these
// are updated.
pref("media.gmp-manager.cert.checkAttributes", true);

View File

@ -93,7 +93,7 @@ function downloadJSON(uri) {
* load the sources from local build configuration.
*/
function downloadLocalConfig() {
let log = getScopedLogger("GMPInstallManager.checkForAddons");
let log = getScopedLogger("GMPInstallManager.downloadLocalConfig");
return Promise.all(
LOCAL_GMP_SOURCES.map(conf => {
return downloadJSON(conf.src).then(addons => {
@ -194,9 +194,21 @@ GMPInstallManager.prototype = {
this._deferred = PromiseUtils.defer();
// Should content signature checking of Balrog replies be used? If so this
// will be done instead of the older cert pinning method.
let checkContentSignature = GMPPrefs.getBool(
GMPPrefs.KEY_CHECK_CONTENT_SIGNATURE,
false
);
let allowNonBuiltIn = true;
let certs = null;
if (!Services.prefs.prefHasUserValue(GMPPrefs.KEY_URL_OVERRIDE)) {
// Only check certificates if we're not using a custom URL, and only if
// we're not checking a content signature.
if (
!Services.prefs.prefHasUserValue(GMPPrefs.KEY_URL_OVERRIDE) &&
!checkContentSignature
) {
allowNonBuiltIn = !GMPPrefs.getString(
GMPPrefs.KEY_CERT_REQUIREBUILTIN,
true
@ -208,10 +220,14 @@ GMPInstallManager.prototype = {
let url = await this._getURL();
log.info(
`Fetching product addon list url=${url}, allowNonBuiltIn=${allowNonBuiltIn}, certs=${certs}, checkContentSignature=${checkContentSignature}`
);
let addonPromise = ProductAddonChecker.getProductAddonList(
url,
allowNonBuiltIn,
certs
certs,
checkContentSignature
).catch(downloadLocalConfig);
addonPromise.then(

View File

@ -140,6 +140,7 @@ var GMPPrefs = {
KEY_URL_OVERRIDE: "media.gmp-manager.url.override",
KEY_CERT_CHECKATTRS: "media.gmp-manager.cert.checkAttributes",
KEY_CERT_REQUIREBUILTIN: "media.gmp-manager.cert.requireBuiltIn",
KEY_CHECK_CONTENT_SIGNATURE: "media.gmp-manager.checkContentSignature",
KEY_UPDATE_LAST_CHECK: "media.gmp-manager.lastCheck",
KEY_SECONDS_BETWEEN_CHECKS: "media.gmp-manager.secondsBetweenChecks",
KEY_UPDATE_ENABLED: "media.gmp-manager.updateEnabled",

View File

@ -19,6 +19,10 @@ const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm");
XPCOMUtils.defineLazyGlobalGetters(this, ["XMLHttpRequest"]);
XPCOMUtils.defineLazyModuleGetters(this, {
ServiceRequest: "resource://gre/modules/ServiceRequest.jsm",
});
// This exists so that tests can override the XHR behaviour for downloading
// the addon update XML file.
var CreateXHR = function() {
@ -28,9 +32,9 @@ var CreateXHR = function() {
var logger = Log.repository.getLogger("addons.productaddons");
/**
* Number of milliseconds after which we need to cancel `downloadXML`.
* Number of milliseconds after which we need to cancel `downloadXMLWithRequest`.
*
* Bug 1087674 suggests that the XHR we use in `downloadXML` may
* Bug 1087674 suggests that the XHR we use in `downloadXMLWithRequest` may
* never terminate in presence of network nuisances (e.g. strange
* antivirus behavior). This timeout is a defensive measure to ensure
* that we fail cleanly in such case.
@ -60,6 +64,132 @@ function getRequestStatus(request) {
return request.channel.QueryInterface(Ci.nsIRequest).status;
}
/**
* A wrapper around `ServiceRequest` that behaves like a limited `fetch()`.
* This doesn't handle headers like fetch, but can be expanded as callers need.
*
* Use this in order to leverage the `beConservative` flag, for
* example to avoid using HTTP3 to fetch critical data.
*
* @param input a resource
* @returns a Response object
*/
async function conservativeFetch(input) {
return new Promise(function(resolve, reject) {
const request = new ServiceRequest({ mozAnon: true });
request.onerror = () =>
reject(new TypeError("NetworkError: Network request failed"));
request.ontimeout = () =>
reject(new TypeError("Timeout: Network request failed"));
request.onabort = () => reject(new DOMException("Aborted", "AbortError"));
request.onload = () => {
const responseAttributes = {
status: request.status,
statusText: request.statusText,
url: request.responseURL,
};
resolve(new Response(request.response, responseAttributes));
};
const method = "GET";
request.open(method, input, true);
request.send();
});
}
/**
* Verifies the content signature on GMP's update.xml. When we fetch update.xml
* balrog should send back content signature headers, which this function
* is used to verify.
*
* @param data
* The data received from balrog. I.e. the xml contents of update.xml.
* @param contentSignatureHeader
* The contents of the 'content-signature' header received along with
* `data`.
* @return A promise that will resolve to nothing if the signature verification
* succeeds, or rejects on failure, with an Error containing a string
* that explains what failed.
*/
async function verifyGmpContentSignature(data, contentSignatureHeader) {
if (!contentSignatureHeader) {
logger.warn(
"Unexpected missing content signature header during content signature validation"
);
throw new Error(
"Content signature validation failed: missing content signature header"
);
}
// Split out the header. It should contain a the following fields, separated by a semicolon
// - x5u - a URI to the cert chain. See also https://datatracker.ietf.org/doc/html/rfc7515#section-4.1.5
// - p384ecdsa - the signature to verify. See also https://github.com/mozilla-services/autograph/blob/main/signer/contentsignaturepki/README.md
const headerFields = contentSignatureHeader
.split(";") // Split fields...
.map(s => s.trim()) // Remove whitespace...
.map(s => [
// Break each field into it's name and value. This more verbose version is
// used instead of `split()` to handle values that contain = characters. This
// shouldn't happen for the signature because it's base64_url (no = padding),
// but it's not clear if it's possible for the x5u URL (as part of a query).
// Guard anyway, better safe than sorry.
s.substring(0, s.indexOf("=")), // Get field name...
s.substring(s.indexOf("=") + 1), // and field value.
]);
let x5u;
let signature;
for (const [fieldName, fieldValue] of headerFields) {
if (fieldName == "x5u") {
x5u = fieldValue;
} else if (fieldName == "p384ecdsa") {
// The signature needs to contain 'p384ecdsa', so stich it back together.
signature = `p384ecdsa=${fieldValue}`;
}
}
if (!x5u) {
logger.warn("Unexpected missing x5u during content signature validation");
throw new Error("Content signature validation failed: missing x5u");
}
if (!signature) {
logger.warn(
"Unexpected missing signature during content signature validation"
);
throw new Error("Content signature validation failed: missing signature");
}
// The x5u field should contain the location of the cert chain, fetch it.
// Use `conservativeFetch` so we get conservative behaviour and ensure (more)
// reliable fetching.
const certChain = await (await conservativeFetch(x5u)).text();
const verifier = Cc[
"@mozilla.org/security/contentsignatureverifier;1"
].createInstance(Ci.nsIContentSignatureVerifier);
let valid;
try {
valid = await verifier.asyncVerifyContentSignature(
data,
signature,
certChain,
"aus.content-signature.mozilla.org"
);
} catch (err) {
logger.warn(`Unexpected error while validating content signature: ${err}`);
throw new Error(`Content signature validation failed: ${err}`);
}
if (!valid) {
logger.warn("Unexpected invalid content signature found during validation");
throw new Error("Content signature is not valid");
}
}
/**
* Downloads an XML document from a URL optionally testing the SSL certificate
* for certain attributes.
@ -71,10 +201,14 @@ function getRequestStatus(request) {
* @param allowedCerts
* The list of certificate attributes to match the SSL certificate
* against or null to skip checks.
* @return a promise that resolves to the DOM document downloaded or rejects
* @return a promise that resolves to the XHR request on success or rejects
* with a JS exception in case of error.
*/
function downloadXML(url, allowNonBuiltIn = false, allowedCerts = null) {
function downloadXMLWithRequest(
url,
allowNonBuiltIn = false,
allowedCerts = null
) {
return new Promise((resolve, reject) => {
let request = CreateXHR();
// This is here to let unit test code override XHR
@ -133,7 +267,7 @@ function downloadXML(url, allowNonBuiltIn = false, allowedCerts = null) {
return;
}
resolve(request.responseXML);
resolve(request);
};
request.addEventListener("error", fail);
@ -146,6 +280,43 @@ function downloadXML(url, allowNonBuiltIn = false, allowedCerts = null) {
});
}
/**
* Downloads an XML document from a URL optionally testing the SSL certificate
* for certain attributes, and/or testing the content signature.
*
* @param url
* The url to download from.
* @param allowNonBuiltIn
* Whether to trust SSL certificates without a built-in CA issuer.
* @param allowedCerts
* The list of certificate attributes to match the SSL certificate
* against or null to skip checks.
* @param verifyContentSignature
* When true, will verify the content signature information from the
* response header. Failure to verify will result in an error.
* @return a promise that resolves to the DOM document downloaded or rejects
* with a JS exception in case of error.
*/
async function downloadXML(
url,
allowNonBuiltIn = false,
allowedCerts = null,
verifyContentSignature = false
) {
let request = await downloadXMLWithRequest(
url,
allowNonBuiltIn,
allowedCerts
);
if (verifyContentSignature) {
await verifyGmpContentSignature(
request.response,
request.getResponseHeader("content-signature")
);
}
return request.responseXML;
}
/**
* Parses a list of add-ons from a DOM document.
*
@ -355,7 +526,7 @@ var verifyFile = async function(properties, path) {
const ProductAddonChecker = {
/**
* Downloads a list of add-ons from a URL optionally testing the SSL
* certificate for certain attributes.
* certificate for certain attributes, and/or testing the content signature.
*
* @param url
* The url to download from.
@ -364,12 +535,25 @@ const ProductAddonChecker = {
* @param allowedCerts
* The list of certificate attributes to match the SSL certificate
* against or null to skip checks.
* @param verifyContentSignature
* When true, will verify the content signature information from the
* response header. Failure to verify will result in an error.
* @return a promise that resolves to an object containing the list of add-ons
* and whether the local fallback was used, or rejects with a JS
* exception in case of error.
*/
getProductAddonList(url, allowNonBuiltIn = false, allowedCerts = null) {
return downloadXML(url, allowNonBuiltIn, allowedCerts).then(parseXML);
getProductAddonList(
url,
allowNonBuiltIn = false,
allowedCerts = null,
verifyContentSignature = false
) {
return downloadXML(
url,
allowNonBuiltIn,
allowedCerts,
verifyContentSignature
).then(parseXML);
},
/**