freedomofpress / securedrop-https-everywhere-ruleset

HTTPS Everywhere ruleset for human-readable Onion URLs for SecureDrop instances
https://securedrop.org/https-everywhere/
10 stars 3 forks source link

Strengthen CI checks to sanity check rulesets #21

Open conorsch opened 3 years ago

conorsch commented 3 years ago

The review process for PRs into this repo is currently entirely manual: https://github.com/freedomofpress/securedrop-https-everywhere-ruleset/blob/276c1da5f83bef16661d4367772a7b50f611701a/.github/PULL_REQUEST_TEMPLATE.md

Some simple checks we can perform in CI are:

More suggestions welcome.

conorsch commented 3 years ago

Also: do the number of rules gzipped and number of signatures match 1:1? They didn't in https://github.com/freedomofpress/securedrop-https-everywhere-ruleset/pull/19#pullrequestreview-548678385, for example.

conorsch commented 3 years ago

Also: Onion URLs should not have trailing slashes, e.g. https://github.com/freedomofpress/securedrop-https-everywhere-ruleset/pull/31/files#r570428975

redshiftzero commented 3 years ago

for validating signatures in CI, referring to this logic over in HTTPS everywhere is a good starting point: https://github.com/EFForg/https-everywhere/blob/d30051e17921148a64d1f5a5c1e2ed8118916f05/chromium/background-scripts/update.js#L146-L156

eloquence commented 3 years ago

Here's an example verification invocation that seems to work for me:

$ openssl dgst -signature rulesets-signature.1612546470.sha256  -sigopt rsa_padding_mode:pss -sigopt rsa_pss_saltlen:32  -verify public_release.pem < default.rulesets.1612546470.gz 
Verified OK

This is using the same options that are specified in https://github.com/EFForg/https-everywhere/blob/master/utils/sign-rulesets/async-airgap.sh#L19.

If that looks right to y'all, I can take a stab at adding this to CI and the README.

conorsch commented 3 years ago

Looks great! I'm able to run locally:

$ openssl dgst -signature rulesets-signature.$(cat latest-rulesets-timestamp).sha256  -sigopt rsa_padding_mode:pss -sigopt rsa_pss_saltlen:32  -verify public_release.pem default.rulesets.$(cat latest-rulesets-timestamp).gz
Verified OK

You can optionally omit <, and just provide the filename. Detecting non-zero exit code is good enough to fail there!

eloquence commented 3 years ago

(Retitled and checklistified to clarify remaining scope :)