-
Notifications
You must be signed in to change notification settings - Fork 51
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
Modernize and update metadata for rustls fork #1
Conversation
87d315f
to
598913e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In ef0c3d3 you say Ubuntu 18 is deprecated. By whom? According to https://ubuntu.com/about/release-cycle it's supported by Ubuntu until April 2023. It's fine to drop support in this crate, but probably we should document that; or follow the example of *ring* and just document what platforms we test on rather than committing to supporting specific platforms.
|
||
steps: | ||
- uses: briansmith/actions-rs-toolchain@v1 | ||
- uses: actions-rs/toolchain@v1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was using Brian's own fork of the actions-rs repo as part of the hardening efforts described here:
briansmith/ring#1256
briansmith/ring#1257
For context, see https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-third-party-actions.
I believe the choice to use a local fork of the actions (as opposed to pinning to a hash) is because there is a repository setting "allow local actions only" that helps enforce a policy of not default-trusting updates to third party actions. So I think the most straightforward thing for us is to also clone these actions into the rustls organization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'm aware of some of the context for why this was done the way it is.
As for doing the same here: we could do that, but then we should also do that across the other repos in the rustls org? It feels like a big job and it's not obvious to me that there are big enough risks here that I should be prioritizing this. And even if we should, I don't think it should be necessarily part of this PR.
So my take is that for now, Brian's forks are probably less- or equally well maintained compared to the originals (and there are currently no secrets in this repository, either).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like there are two options for dealing with GH actions poor security defaults:
- totally distrust the CI process: it gets read-only access to the repo (same as anything else on the internet), and no special access to secrets etc
- carefully curate and control the CI process, so we can trust it with secrets and privileged repo access
I think we should be expending effort doing (1) -- and I've just checked this repo has minimal GITHUB_TOKEN permissions (which should be the case for the other rustls-org repos already)
Sorry, yes, 18.04 is specifically deprecated for GitHub Actions: IMO since there is no platform-specific code in webpki, I feel confident that it will keep working in 18.04 even if we only test it on 20.04 and later. In general across all the crates I maintain it's been extremely uncommon to find cross-platform issues except in Quinn where we're using a bunch of very low-level networking features. As for scope, this PR is really intended to (a) make this crate testable (as a rustls dependency) and (b) make sure we have working CI. I'd prefer not to be doing much more in this PR. We definitely also need to revise the README, but that's orthogonal enough that it feels better to do it in a separate PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. 👍🏻
No description provided.