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

Modernize and update metadata for rustls fork #1

Merged
merged 11 commits into from
Sep 2, 2022
Merged

Modernize and update metadata for rustls fork #1

merged 11 commits into from
Sep 2, 2022

Conversation

djc
Copy link
Member

@djc djc commented Aug 31, 2022

No description provided.

@djc djc requested review from ctz and jsha August 31, 2022 20:38
Copy link
Contributor

@jsha jsha left a 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
Copy link
Contributor

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.

Copy link
Member Author

@djc djc Sep 2, 2022

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).

Copy link
Member

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:

  1. 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
  2. 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)

@djc
Copy link
Member Author

djc commented Sep 2, 2022

Sorry, yes, 18.04 is specifically deprecated for GitHub Actions:

https://github.blog/changelog/2022-08-09-github-actions-the-ubuntu-18-04-actions-runner-image-is-being-deprecated-and-will-be-removed-by-12-1-22/

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.

Copy link
Contributor

@jsha jsha left a comment

Choose a reason for hiding this comment

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

Sounds good. 👍🏻

@djc djc merged commit dcfe0b4 into main Sep 2, 2022
@ctz ctz deleted the clippy branch March 13, 2023 19:01
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