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

cosign/tuf: use trustroot #305

Merged
merged 3 commits into from
Nov 14, 2023
Merged

cosign/tuf: use trustroot #305

merged 3 commits into from
Nov 14, 2023

Conversation

jleightcap
Copy link
Contributor

@jleightcap jleightcap commented Nov 6, 2023

Summary

Towards #280, adapt Cosign and TUF to use trustroot.

As a roadmap, this PR is structured:

  • define the TUF trustroot types in src/tuf/repository_helper.rs.
  • refactor the TUF implementation to use trustroot and rustls-webpki+rustls-pki-types.
    • note: this now means that the trustroot is "prefetch"ed, then used in async contexts--see release notes.
    • this also introduces some lifetime information that propagates into other components.
  • misc. cleanup in changed components
    • pull the root JSON out of raw strings and into their own committed files.
    • Repository construction now uses the trustroot, with a FakeRepository option for BYO keys/certs--see release notes.

Release Note

The method of constructing a Repository with out-of-band trust materials (from Rekor public key and Fulcio certificate) has changed. See Cosign's verify example:

async fn fulcio_and_rekor_data(cli: &Cli) -> anyhow::Result<Box<dyn sigstore::tuf::Repository>> {
if cli.use_sigstore_tuf_data {
let repo: sigstore::errors::Result<SigstoreRepository> = spawn_blocking(|| {
info!("Downloading data from Sigstore TUF repository");
SigstoreRepository::new(None)?.prefetch()
})
.await
.map_err(|e| anyhow!("Error spawning blocking task inside of tokio: {}", e))?;
return Ok(Box::new(repo?));
};
let mut data = sigstore::tuf::FakeRepository::default();
if let Some(path) = cli.rekor_pub_key.as_ref() {
data.rekor_key = Some(
fs::read(path)
.map_err(|e| anyhow!("Error reading rekor public key from disk: {}", e))?,
);
}
if let Some(path) = cli.fulcio_cert.as_ref() {
let cert_data = fs::read(path)
.map_err(|e| anyhow!("Error reading fulcio certificate from disk: {}", e))?;
let certificate = sigstore::registry::Certificate {
encoding: sigstore::registry::CertificateEncoding::Pem,
data: cert_data,
};
data.fulcio_certs
.get_or_insert(Vec::new())
.push(certificate.try_into()?);
}
Ok(Box::new(data))
}

And the signature verification documentation:

sigstore-rs/src/lib.rs

Lines 83 to 103 in 8a269a3

//! // Provide both rekor and fulcio data -> this enables keyless verification
//! // Read rekor's key from the location generated by `cosign initialize`
//! let rekor_pub_key = fs::read("~/.sigstore/root/targets/rekor.pub")
//! .expect("Cannot read rekor public key");
//! // Read fulcio's certificate from the location generated by `cosign initialize`
//! let fulcio_cert_data = fs::read("~/.sigstore/root/targets/fulcio.crt.pem")
//! .expect("Cannot read fulcio certificate");
//! let fulcio_cert = sigstore::registry::Certificate {
//! encoding: sigstore::registry::CertificateEncoding::Pem,
//! data: fulcio_cert_data
//! };
//!
//! let mut repo = sigstore::tuf::FakeRepository::default();
//! repo.fulcio_certs.get_or_insert(Vec::new()).push(fulcio_cert.try_into().unwrap());
//! repo.rekor_key = Some(rekor_pub_key);
//!
//! let mut client = sigstore::cosign::ClientBuilder::default()
//! .with_trust_repository(&repo)
//! .expect("Cannot construct cosign client from given materials")
//! .build()
//! .expect("Unexpected failure while building Client");

Documentation

@jleightcap
Copy link
Contributor Author

jleightcap commented Nov 6, 2023

@woodruffw and @tnytown mentioned some ownership design considerations that I'm not sure I fully understood. A closer look at some lifetime choices e.g.

pub struct ClientBuilder<'a> {
oci_client_config: ClientConfig,
rekor_pub_key: Option<&'a [u8]>,
fulcio_certs: Vec<CertificateDer<'a>>,
// repo: Repository
#[cfg(feature = "cached-client")]
enable_registry_caching: bool,
}
would be fantastic.

@woodruffw
Copy link
Member

@woodruffw and @tnytown mentioned some ownership design considerations that I'm not sure I fully understood. A closer look at some lifetime choices e.g.

@tnytown can confirm, but I believe the question was whether it makes sense to allow the interior 'a lifetime on CertificateDer<'a> to "cascade" throughout the refactor. The alternative would be to create an OwnedCertificateDer wrapper that maintains a self-reference, using a crate like self_cell.

This is exactly what PyCA Cryptography does, turning a Certificate<'a> into an OwnedCertificate:

https://github.com/pyca/cryptography/blob/4c07d8eb289aaa0fbcdbcf370724122905fade02/src/rust/src/x509/certificate.rs#L29C1-L36

Whether or not this actually makes sense to do, however, is ultimately a design question. If the sigstore-rs maintainers are okay with the lifetime, then it probably doesn't make sense to do 🙂

@jleightcap jleightcap force-pushed the jl/cosign branch 4 times, most recently from 6e053f2 to a536bea Compare November 9, 2023 15:15
@jleightcap jleightcap changed the title cosign/tuf: init cosign/tuf: use trustroot Nov 9, 2023
@jleightcap jleightcap marked this pull request as ready for review November 9, 2023 15:37
Signed-off-by: Jack Leightcap <[email protected]>
Co-authored-by: Andrew Pan <[email protected]>
lukehinds
lukehinds previously approved these changes Nov 9, 2023
Copy link
Member

@lukehinds lukehinds left a comment

Choose a reason for hiding this comment

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

First pass looks good to me.

src/tuf/trustroot.rs Show resolved Hide resolved
Signed-off-by: Jack Leightcap <[email protected]>
Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

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

It looks good to me. I left some minor comments.

There's just one major objection I have: the usage of the rustls-webpki crate. This brings back the ring dependency which was removed by @Xynnn007 sometimes ago.

@Xynnn007 what do you think about that?

src/cosign/client_builder.rs Outdated Show resolved Hide resolved
src/tuf/mod.rs Outdated Show resolved Hide resolved
Cargo.toml Show resolved Hide resolved
src/tuf/mod.rs Show resolved Hide resolved
src/tuf/mod.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Signed-off-by: Jack Leightcap <[email protected]>
Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution and for having gone through the requested changes

@flavio flavio merged commit f89751d into sigstore:main Nov 14, 2023
6 checks passed
@jleightcap jleightcap deleted the jl/cosign branch November 14, 2023 13:08
@jleightcap jleightcap mentioned this pull request Nov 16, 2023
flavio added a commit that referenced this pull request Dec 13, 2023
What's Changed
==============
* chore(deps): Update rstest requirement from 0.17.0 to 0.18.1 by @dependabot in #282
* chore(deps): do not enable default features of chrono by @flavio in #286
* chore(deps): Update pem requirement from 2.0 to 3.0 by @dependabot in #289
* conformance: add conformance CLI and action by @jleightcap in #287
* chore: fix clippy warnings by @flavio in #292
* chore(deps): Bump actions/checkout from 3.5.3 to 3.6.0 by @dependabot in #291
* chore(deps): Update tough requirement from 0.13 to 0.14 by @dependabot in #290
* chore(deps): update to latest version of picky by @flavio in #293
* chore(deps): Bump actions/checkout from 3.6.0 to 4.0.0 by @dependabot in #294
* chore: add repository link to Cargo metadata by @flavio in #297
* chore(deps): Update cached requirement from 0.44.0 to 0.45.1 by @dependabot in #298
* chore(deps): Bump actions/checkout from 4.0.0 to 4.1.0 by @dependabot in #302
* chore(deps): Update cached requirement from 0.45.1 to 0.46.0 by @dependabot in #301
* chore(deps): Update testcontainers requirement from 0.14 to 0.15 by @dependabot in #303
* chore(deps): Bump actions/checkout from 4.1.0 to 4.1.1 by @dependabot in #304
* cosign/tuf: use trustroot by @jleightcap in #305
* Fix broken tests, update deps by @flavio in #313

New Contributors
================

* @jleightcap made their first contribution in #287

**Full Changelog**: v0.7.2...v0.8.0

Signed-off-by: Luke Hinds <[email protected]>
Co-authored-by: Flavio Castelli <[email protected]>
flavio added a commit that referenced this pull request Dec 13, 2023
What's Changed
==============
* chore(deps): Update rstest requirement from 0.17.0 to 0.18.1 by @dependabot in #282
* chore(deps): do not enable default features of chrono by @flavio in #286
* chore(deps): Update pem requirement from 2.0 to 3.0 by @dependabot in #289
* conformance: add conformance CLI and action by @jleightcap in #287
* chore: fix clippy warnings by @flavio in #292
* chore(deps): Bump actions/checkout from 3.5.3 to 3.6.0 by @dependabot in #291
* chore(deps): Update tough requirement from 0.13 to 0.14 by @dependabot in #290
* chore(deps): update to latest version of picky by @flavio in #293
* chore(deps): Bump actions/checkout from 3.6.0 to 4.0.0 by @dependabot in #294
* chore: add repository link to Cargo metadata by @flavio in #297
* chore(deps): Update cached requirement from 0.44.0 to 0.45.1 by @dependabot in #298
* chore(deps): Bump actions/checkout from 4.0.0 to 4.1.0 by @dependabot in #302
* chore(deps): Update cached requirement from 0.45.1 to 0.46.0 by @dependabot in #301
* chore(deps): Update testcontainers requirement from 0.14 to 0.15 by @dependabot in #303
* chore(deps): Bump actions/checkout from 4.1.0 to 4.1.1 by @dependabot in #304
* cosign/tuf: use trustroot by @jleightcap in #305
* Fix broken tests, update deps by @flavio in #313

New Contributors
================

* @jleightcap made their first contribution in #287

**Full Changelog**: v0.7.2...v0.8.0

Signed-off-by: Luke Hinds <[email protected]>
Co-authored-by: Flavio Castelli <[email protected]>
flavio added a commit that referenced this pull request Dec 21, 2023
Add ability to sign artifacts à la sigstore-python, towards Bundle signing and verification.
Follow-up from #305, and the prerequisite motivation to #311.

Signed-off-by: Jack Leightcap <[email protected]>
Signed-off-by: Andrew Pan <[email protected]>
Signed-off-by: Andrew Pan <[email protected]>
Signed-off-by: Jack Leightcap <[email protected]>
Co-authored-by: Andrew Pan <[email protected]>
Co-authored-by: Andrew Pan <[email protected]>
Co-authored-by: Andrew Pan <[email protected]>
Co-authored-by: Flavio Castelli <[email protected]>
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.

6 participants