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
127 changes: 73 additions & 54 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,72 +6,45 @@ on:
push:
jobs:
rustfmt:
runs-on: ubuntu-18.04
runs-on: ubuntu-20.04

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)

with:
toolchain: stable
profile: minimal
components: rustfmt
- uses: briansmith/actions-checkout@v2
- uses: actions/checkout@v2
with:
persist-credentials: false
- run: cargo fmt --all -- --check

clippy:
runs-on: ubuntu-18.04
runs-on: ubuntu-20.04

steps:
- uses: briansmith/actions-rs-toolchain@v1
- uses: actions-rs/toolchain@v1
with:
toolchain: stable
profile: minimal
components: clippy

- uses: briansmith/actions-checkout@v2
- uses: actions/checkout@v2
with:
persist-credentials: false

- run: mk/clippy.sh

audit:
runs-on: ubuntu-18.04

steps:
- uses: briansmith/actions-rs-toolchain@v1
with:
toolchain: stable
profile: minimal

- uses: briansmith/actions-cache@v2
with:
path: |
~/.cargo/bin/cargo-audit
~/.cargo/.crates.toml
~/.cargo/.crates2.json
key: ${{ runner.os }}-v2-cargo-audit-0.13.1

- run: cargo install cargo-audit --vers "0.13.1"

- uses: briansmith/actions-checkout@v2
with:
persist-credentials: false

- run: cargo generate-lockfile

- run: cargo audit --deny warnings

deny:
runs-on: ubuntu-18.04
runs-on: ubuntu-20.04

steps:
- uses: briansmith/actions-rs-toolchain@v1
- uses: actions-rs/toolchain@v1
with:
toolchain: stable
profile: minimal

- uses: briansmith/actions-cache@v2
- uses: actions/cache@v2
with:
path: |
~/.cargo/bin/cargo-deny
Expand All @@ -81,15 +54,15 @@ jobs:

- run: cargo install cargo-deny --locked --vers "0.8.5"

- uses: briansmith/actions-checkout@v2
- uses: actions/checkout@v2
with:
persist-credentials: false

- run: cargo deny check

# Verify that documentation builds.
rustdoc:
runs-on: ubuntu-18.04
runs-on: ubuntu-20.04

strategy:
matrix:
Expand All @@ -102,29 +75,29 @@ jobs:
- target: x86_64-unknown-linux-gnu

steps:
- uses: briansmith/actions-rs-toolchain@v1
- uses: actions-rs/toolchain@v1
with:
override: true
target: ${{ matrix.target }}
toolchain: ${{ matrix.rust_channel }}

- uses: briansmith/actions-checkout@v2
- uses: actions/checkout@v2
with:
persist-credentials: false

- run: |
cargo doc --all-features

package:
runs-on: ubuntu-18.04
runs-on: ubuntu-20.04

steps:
- uses: briansmith/actions-rs-toolchain@v1
- uses: actions-rs/toolchain@v1
with:
toolchain: stable
profile: minimal

- uses: briansmith/actions-checkout@v2
- uses: actions/checkout@v2
with:
persist-credentials: false

Expand Down Expand Up @@ -166,35 +139,81 @@ jobs:
- beta

exclude:
# 1.46.0 doesn't support `-Clink-self-contained`.
- target: x86_64-unknown-linux-musl
- features: # Default
- features: --features=alloc
- features: --all-features
mode: --release
- features: --all-features
mode: # debug
rust_channel: nightly
- features: --all-features
mode: # debug
rust_channel: 1.46.0
- features: --all-features
mode: # debug
rust_channel: beta

include:
- features: # Default
target: x86_64-unknown-linux-gnu
mode: # debug
rust_channel: stable
host_os: ubuntu-20.04

- features: --features=alloc
target: x86_64-unknown-linux-gnu
mode: # debug
rust_channel: stable
host_os: ubuntu-20.04

- features: --all-features
target: x86_64-unknown-linux-gnu
mode: --release
rust_channel: stable
host_os: ubuntu-20.04

- features: --all-features
target: x86_64-unknown-linux-gnu
mode: # debug
rust_channel: nightly
host_os: ubuntu-20.04

- features: --all-features
target: x86_64-unknown-linux-gnu
mode: # debug
rust_channel: 1.46.0
host_os: ubuntu-20.04

- features: --all-features
target: x86_64-unknown-linux-gnu
mode: # debug
rust_channel: beta
host_os: ubuntu-20.04

- target: arm-unknown-linux-gnueabihf
host_os: ubuntu-18.04
host_os: ubuntu-20.04

- target: i686-pc-windows-msvc
host_os: windows-latest

- target: x86_64-unknown-linux-musl
host_os: ubuntu-18.04
host_os: ubuntu-20.04

- target: x86_64-unknown-linux-gnu
host_os: ubuntu-18.04
host_os: ubuntu-20.04

steps:
- if: ${{ contains(matrix.host_os, 'ubuntu') }}
run: sudo apt-get update -y

- uses: briansmith/actions-checkout@v2
- uses: actions/checkout@v2
with:
persist-credentials: false

- if: ${{ !contains(matrix.host_os, 'windows') }}
run: mk/install-build-tools.sh --target=${{ matrix.target }} ${{ matrix.features }}

- uses: briansmith/actions-rs-toolchain@v1
- uses: actions-rs/toolchain@v1
with:
override: true
target: ${{ matrix.target }}
Expand Down Expand Up @@ -233,20 +252,20 @@ jobs:
# TODO: targets
include:
- target: x86_64-unknown-linux-musl
host_os: ubuntu-18.04
host_os: ubuntu-20.04

steps:
- if: ${{ contains(matrix.host_os, 'ubuntu') }}
run: sudo apt-get update -y

- uses: briansmith/actions-checkout@v2
- uses: actions/checkout@v2
with:
persist-credentials: false

- if: ${{ !contains(matrix.host_os, 'windows') }}
run: RING_COVERAGE=1 mk/install-build-tools.sh --target=${{ matrix.target }} ${{ matrix.features }}

- uses: briansmith/actions-rs-toolchain@v1
- uses: actions-rs/toolchain@v1
with:
override: true
target: ${{ matrix.target }}
Expand All @@ -259,7 +278,7 @@ jobs:
run: |
RING_COVERAGE=1 mk/cargo.sh +${{ matrix.rust_channel }} test -vv --target=${{ matrix.target }} ${{ matrix.cargo_options }} ${{ matrix.features }} ${{ matrix.mode }}

- uses: briansmith/codecov-codecov-action@v1
- uses: codecov/codecov-action@v1
with:
directory: ./target/${{ matrix.target }}/debug/coverage/reports
fail_ci_if_error: true
Expand Down
8 changes: 3 additions & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,14 @@
# OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

[package]
authors = ["Brian Smith <[email protected]>"]
categories = ["cryptography", "no-std"]
description = "Web PKI X.509 Certificate Verification."
documentation = "https://briansmith.org/rustdoc/webpki/"
edition = "2018"
license-file = "LICENSE"
name = "webpki"
name = "rustls-webpki"
readme = "README.md"
repository = "https://github.com/briansmith/webpki"
version = "0.21.4"
repository = "https://github.com/rustls/webpki"
version = "0.22.0-alpha.1"

include = [
"Cargo.toml",
Expand Down
3 changes: 2 additions & 1 deletion deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ allow = [
"LicenseRef-ring",
"LicenseRef-webpki",
"MIT",
"Unicode-DFS-2016",
]
confidence-threshold = 1.0

Expand All @@ -23,7 +24,7 @@ license-files = [
# XXX: Figure out how to deal with the Google-source test data
# https://github.com/briansmith/webpki/issues/148.
[[licenses.clarify]]
name = "webpki"
name = "rustls-webpki"
expression = "LicenseRef-webpki"
license-files = [
{ path = "LICENSE", hash = 0x001c7e6c },
Expand Down
2 changes: 1 addition & 1 deletion mk/cargo.sh
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ for arg in $*; do
done

# See comments in install-build-tools.sh.
llvm_version=12
llvm_version=15

case $target in
aarch64-linux-android)
Expand Down
2 changes: 1 addition & 1 deletion mk/clippy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ IFS=$'\n\t'
export NULL=""
cargo clippy \
--target-dir=target/clippy \
--all-features ---all-targets \
--all-features --all-targets \
-- \
--deny missing_docs \
--deny warnings \
Expand Down
4 changes: 2 additions & 2 deletions mk/install-build-tools.sh
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ esac
if [ -n "$use_clang" ]; then
# https://github.com/rust-lang/rust/pull/79365 upgraded the coverage file
# format to one that only LLVM 11+ can use
llvm_version=12
llvm_version=15
sudo apt-key add mk/llvm-snapshot.gpg.key
sudo add-apt-repository "deb http://apt.llvm.org/bionic/ llvm-toolchain-bionic-$llvm_version main"
sudo add-apt-repository "deb http://apt.llvm.org/focal/ llvm-toolchain-focal-$llvm_version main"
sudo apt-get update
install_packages clang-$llvm_version llvm-$llvm_version
fi
2 changes: 1 addition & 1 deletion src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
use core::fmt;

/// An error that occurs during certificate validation or name validation.
#[derive(Clone, Copy, Debug, PartialEq)]
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum Error {
/// The encoding of some ASN.1 DER-encoded item is invalid.
// TODO: Rename to `BadDer` in the next release.
Expand Down
17 changes: 5 additions & 12 deletions src/name/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,12 @@ pub fn verify_cert_dns_name(
cert.subject_alt_name,
Err(Error::CertNotValidForName),
&|name| {
match name {
GeneralName::DnsName(presented_id) => {
match dns_name::presented_id_matches_reference_id(presented_id, dns_name) {
Some(true) => {
return NameIteration::Stop(Ok(()));
}
Some(false) => (),
None => {
return NameIteration::Stop(Err(Error::BadDER));
}
}
if let GeneralName::DnsName(presented_id) = name {
match dns_name::presented_id_matches_reference_id(presented_id, dns_name) {
Some(true) => return NameIteration::Stop(Ok(())),
Some(false) => (),
None => return NameIteration::Stop(Err(Error::BadDER)),
}
_ => (),
}
NameIteration::KeepGoing
},
Expand Down
2 changes: 1 addition & 1 deletion src/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
/// Internally this is merely a UNIX timestamp: a count of non-leap
/// seconds since the start of 1970. This type exists to assist
/// unit-of-measure correctness.
#[derive(Debug, Clone, Copy, PartialEq, PartialOrd)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd)]
pub struct Time(u64);

impl Time {
Expand Down
Loading