Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly documentation of Firefox client behavior #859

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Rob--W
Copy link

@Rob--W Rob--W commented Aug 3, 2023

The current documentation of content signature verification and add-on certificate verification is inaccurate. This PR fixes a few inaccuracies.

References:

About the comment "Only end-entity certs can potentially end up here." (in ERROR_EXPIRED_CERTIFICATE / ERROR_NOT_YET_VALID_CERTIFICATE): I verified this locally and we have also observed before in the armagadd-on-2.0 incident (https://bugzilla.mozilla.org/show_bug.cgi?id=1548973); if expired intermediates were accepted, then we would not have had the incident.

References:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1846866 ignores pref
- https://bugzilla.mozilla.org/show_bug.cgi?id=1267318 ignores notAfter
- https://bugzilla.mozilla.org/show_bug.cgi?id=1713628 ignores notBefore

"Only end-entity certs can potentially end up here." (in
ERROR_EXPIRED_CERTIFICATE / ERROR_NOT_YET_VALID_CERTIFICATE): verified
locally and also observed before in the armagadd-on-2.0 incident
(https://bugzilla.mozilla.org/show_bug.cgi?id=1548973); if expired
intermediates were accepted, then we would not have had the incident.
@Rob--W Rob--W force-pushed the fix-doc-firefox-behavior branch from 1670d00 to 5b600a6 Compare August 3, 2023 00:55
@Rob--W
Copy link
Author

Rob--W commented Aug 3, 2023

@hwine Could you review/merge this? I don't know who else to ask here.

@hwine hwine self-requested a review August 3, 2023 16:58
# In Firefox 103+ (bug 1769669), roots are hard-coded in Firefox and the
# chosen root is dependent on the app.normandy.api_url pref, see
# https://searchfox.org/mozilla-central/rev/2bf90dc51ce7e8274ce208fbb9d68b3ff535185e/toolkit/components/normandy/lib/NormandyApi.sys.mjs#15-30
#
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hwine to do

  • also needs to be copied to production configs

# In Firefox 103+ (bug 1769669), roots are hard-coded in Firefox and the
# chosen root is dependent on multiple conditions, see
# https://searchfox.org/mozilla-central/rev/2bf90dc51ce7e8274ce208fbb9d68b3ff535185e/services/settings/Utils.sys.mjs#53-76,97-101,110-124
#
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hwine to do

  • also needs to be copied to production configs

@willdurand
Copy link
Member

@hwine the docs confused me last night when I was debugging staging content signature for remote settings, can we finalize this PR please?

@hwine
Copy link
Contributor

hwine commented Mar 22, 2024

Thanks for nudge -- I'll cut a separate ticket for the live config updates

Copy link
Contributor

@hwine hwine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - they're the SMEs here 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants