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

Switch main branch to use 2021 signing key #66

Merged
merged 16 commits into from
Aug 19, 2021
Merged

Conversation

zenmonkeykstop
Copy link
Contributor

Status

Ready for review

This PR pushes the historical changes in the key-rotation-2021 branch to main, updating the current ruleset to the latest one signed with the 2021 release key.

Review Checklist

  • The ruleset has been verified by modifying the HTTPS Everywhere configuration in a Tor Browser 10.5+ instance pointing to Path Prefix: https://raw.githubusercontent.com/freedomofpress/securedrop-https-everywhere-ruleset/$BRANCH_NAME
  • nginx config is correct ( there's a comment about removing a line but I think it should actually stay, as the base URL has changed)

Post-Deployment Checklist

  • Update httpse CI to populate the 2021 ruleset from the main branch, instead of from key-rotation-2021.

maeve-fpf and others added 15 commits May 26, 2021 12:08
Required for the 2021 key rotation for the SD release signing key.
Used the scripts in the repo to convert the ascii-armored pubkey to a
jwk.
Generated the PEM format public key component via:

  openssl rsa -in key.pem -outform PEM -pubout -out public.pem

where `key.pem` is the RSA privkey in PEM format (via `openpgp2pem`).
Account for external 2021 path in the nginx redirect
Add Al Jazeera Investigative Unit address to the 2021 channel
Updates main to use 2021 key (merged with -Xtheirs merge strategy)
@maeve-fpf
Copy link
Contributor

LGTM, just blocking on https://github.com/freedomofpress/k8s-configs/pull/205 being merged.

I don't mind, but If it bothers you at all one of my commits is duplicated with [TEST] in the commit message (probably from being cherry-picked) and it could be rebased out.

@zenmonkeykstop
Copy link
Contributor Author

I'm fine with it, given the nature of the repo I'd rather preserve the history intact.

@conorsch
Copy link
Contributor

Tagging myself in on review here. Will coordinate actual merge (and therefore deploy) with @maeve-fpf ahead of time. First, I'll review the changes locally.

@eloquence eloquence added this to Ready for Review in SecureDrop Team Board Aug 18, 2021
@conorsch conorsch moved this from Ready for Review to Under Review in SecureDrop Team Board Aug 18, 2021
# This line has been adjusted for the temporary different external URL.
# Don't include this change in the main branch.
rewrite ^/https-everywhere($|//+)(.*) /https-everywhere-2021/$2 permanent;
rewrite ^/https-everywhere-2021($|//+)(.*) /https-everywhere-2021/$2 permanent;
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately this is where it gets kind of gnarly: our ingress transforms all external requests (/https-everywhere-2021/, or, formerly, /https-everywhere/) to internal requests for /https-everywhere/, because that's what's being served by this container (files are copied to /opt/nginx/root/https-everywhere/). So we need to leave this rewrite as is for it to do its thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Roger that, will amend and at least clarify the comment as a warning to posterity.

@@ -1,5 +1,5 @@
# sha256 as of 2020-09-25 for mainline-alpine
FROM nginx@sha256:4635b632d2aaf8c37c8a1cf76a1f96d11b899f74caa2c6946ea56d0a5af02c0c
# sha256 as of 2021-08-18 for nginx:mainline-alpine
Copy link
Contributor

Choose a reason for hiding this comment

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

I've been moving toward putting the tag in the FROM line, a la nginx:mainline-alpine@sha256:... and then having only the datestamp in the comment. (Didn't know you could use that syntax until recently!)

Copy link
Contributor

Choose a reason for hiding this comment

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

TIL! That's much clearer, will do.

Bumps tag to latest in the "mainline-alpine". Hadn't been updated in a
while. Clarifies comment in nginx config, preserving the new route
logic.
@conorsch
Copy link
Contributor

Comments addressed, @maeve-fpf. I'm going to push this to staging, as you advised, and if no problems, will proceed with prod deploy later today.

Copy link
Contributor

@conorsch conorsch left a comment

Choose a reason for hiding this comment

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

Looks good! Compared the rules with what's on main, and confirmed the signatures are valid. When configuring a new channel in Tor Browser, I'm able to pull in 2021.7.9, same as the prod endpoint.

@conorsch conorsch merged commit 9a9ecc5 into main Aug 19, 2021
SecureDrop Team Board automation moved this from Under Review to Done Aug 19, 2021
@legoktm legoktm deleted the switch-main-to-2021 branch June 10, 2024 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants