Closed Bug 1715142 Opened 3 years ago Closed 3 years ago

separate public key pinning implementation from HSTS implementation

Categories

(Core :: Security: PSM, task, P1)

task

Tracking

()

RESOLVED FIXED
91 Branch
Tracking Status
firefox91 --- fixed

People

(Reporter: keeler, Assigned: keeler)

Details

(Whiteboard: [psm-assigned])

Attachments

(3 files)

The public key pinning implementation is much less complex than the HSTS implementation, and only needs a small subset of the parameters of the latter. Furthermore, the information it relies on is static, and so is safe to access from content processes. The aim of this bug is to separate the two implementations, thus simplifying both of them and avoiding some unnecessary IPC calls.

This patch converts the pinning preference
"security.cert_pinning.enforcement_level" to be static. It also removes some
unused pinning preferences and parameters.

The public key pinning implementation is much less complex than the HSTS
implementation, and only needs a small subset of the parameters of the latter.
Furthermore, the information it relies on is static, and so is safe to access
from content processes. This patch separates the two implementations, thus
simplifying both of them and avoiding some unnecessary IPC calls in the
process.

Depends on D117095

Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ab3060a5f69e
convert pinning to use a static pref r=rmf
https://hg.mozilla.org/integration/autoland/rev/83206685ca0b
introduce nsIPublicKeyPinningService and remove 'type' parameter from nsISiteSecurityService r=rmf,necko-reviewers

Backed out for causing marionette failures on test_navigation.py and mochitest failures on browser_setIgnoreCertificateErrors.js.

Push with failures

Failure log for marionette

Failure log for mochitest

Backout link

Flags: needinfo?(dkeeler)

Previously, SetDisableAllSecurityChecksAndLetAttackersInterceptMyData would
only work as expected if another operation happened to clear the TLS session
cache (namely, changing a preference that caused nsNSSComponent to change its
TLS options and clear the TLS session cache). This patch ensures that this
function works without relying on such coincidences.

Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f8a7bd4519c6
clear the TLS session cache in SetDisableAllSecurityChecksAndLetAttackersInterceptMyData r=rmf
https://hg.mozilla.org/integration/autoland/rev/f58d5156f332
convert pinning to use a static pref r=rmf
https://hg.mozilla.org/integration/autoland/rev/7e67994f6a65
introduce nsIPublicKeyPinningService and remove 'type' parameter from nsISiteSecurityService r=rmf,necko-reviewers
Flags: needinfo?(dkeeler)
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1051579018f1
clear the TLS session cache in SetDisableAllSecurityChecksAndLetAttackersInterceptMyData r=rmf
https://hg.mozilla.org/integration/autoland/rev/6a10fc0722ef
convert pinning to use a static pref r=rmf
https://hg.mozilla.org/integration/autoland/rev/8fa99e3f1e73
introduce nsIPublicKeyPinningService and remove 'type' parameter from nsISiteSecurityService r=rmf,necko-reviewers
Depends on: 1716227
Regressions: 1716227
No longer depends on: 1716227
No longer regressions: 1716227
Flags: needinfo?(dkeeler)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: