-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add infrastructure information to trusted root #259
Comments
@woodruffw @jku @kommendorkapten @haydentherapper @codysoyland @steiza I'm been wanting to have this feature for a while because it really makes sigstore-java's initialization code fully tied to the |
Yes, 👍 , I obviously also like this.
Feels like that: I think it is probably a suggestion in any case?
Current use cases involve taking the first item on the list and then throwing the list away (essentially replacing the hard coded dex URL that everyone now uses)... but I guess it can be a list.
can you expand on the potential use cases, I don't know what these are good for. |
Yeah, my soft preference would be for this being a single member, unless we know of a specific case where a Sigstore configuration/deployment is going to need/use multiple OIDC endpoints. I can see multiple endpoints getting pretty confusing 🙂
IMO Edit: Correcting myself: OIDC discovery says |
This does mesh well with the meaning "this certificate issuer suggests using this OIDC endpoint". |
oh I forgot about #183 😞 Anyway, so... to summarize current sentiment:
|
I think william made a good case against well_known_configuration |
ah I misread that. kk, lemme update the summary. |
I think it should not be per-CA and should be its own configuration. A CA can accept multiple OIDC providers, Fulcio does this. In Sigstore's trust root, we would not be distributing this list (or at least, I don't think we should because that list updates frequently and having to re-sign TUF targets frequently with our current setup is not ideal. Maybe a question for later). Sigstore's trust root would only contain the Dex instance, which can update independent of the CA.
|
Fuclio can accept multiple oidc providers, but fulcio is explicitly configured to accept a dex instance. What I want to provide a reference for is the dex instance -- so maybe |
Yeah, I could see this being confusing. Then again the provider isn't necessarily a Dex instance either 🤔 -- maybe we could call it |
I' thinking if this should be a separate config file, e.g |
At least for
I don't think we need to know that Fulcio "trusts" Dex, per se, although maybe that's implied. What signing clients need is a way to go from an arbitrary |
Yeah I strongly prefer a single file. The dex instance is something that is very much coupled with fulcio. Its not config that has to be used by clients either. It just simply exists to inform people that public-good uses |
There's a lot here, but I think I caught up - the main problem we're trying to solve is how can signing libraries get information about which OIDC provider(s) to offer to a user, without making the user supply that information as an additional flag? Today that information lives inside of Fulcio's configuration. We distribute some information to the client already via the trusted root, but which OIDC provider(s) to use is more of a hint to client libraries and users, and I think we really want to keep the concerns of the trust root as narrowly defined as possible. Maybe it would make sense for a Fulcio instance to advertise its trusted OIDC provider(s) publicly, via some well-known URL? Then client libraries would only need to know which Fulcio instance to use to get this information, which they need to know anyways. On the verification side, there's really two parts to verification: does the signature verify with my trust root and then does the signature / metadata comply with my policies. I think what OIDC provider(s) you trust when verifying is really a policy question, and again mixing this into the trust root feels like a mixing of concerns. |
We actually do advertise which OIDC providers we trust! For the discussion of whether this should be in its own file or not, I think what's different here is this is only going to be used for signing, never verification. The token is never verified during verification because it's no longer present. I think it'd be fine to still bundle this in the existing trusted root spec.
For history, the primary reason Sigstore is operating its own oidc provider is so that we can control the lifetime of the issued tokens. As a client, I would recommend users use the Sigstore-managed Dex instance for this reason, but we know some won't use it, either because they're doing workload signing or if their federated identity provider is not supported by our Dex instance. |
So to avoid overloading the trusted_root, should we just have an infrastructure.json as a target that has all the infra urls? This would make initializing clients very straight forward. |
Yes, this is my preference. As it's only the signers that need this, I think we should avoid mixing concerns in What else configuration would we want to have in this file? I'm thinking about naming. |
I'm not a fan
The idea that fulcio servers can have an optional wellknown file that may contain a "suggested interactive OIDC URL" seems much better than just another file in the TUF repository (assuming that the extra field in trusted_root.json does not sound appealing). |
The overloading IMHO comes from the content in the file, not how it's managed. The I don't think it would be that much harder for a client to load two files instead of one when a signing operation should happen? They can still be merged into a single struct in-memory if desired. |
I meant to add to this, but I guess I forgot. The current |
Agreed! Storing data in a TUF repository has a bit of slowness built into it, so if we can be more data-driven by the services this is preferable. There is one problem though, there can be many OPs that have the same challenge claim (e.g. An example entry looks like this: {
"issuerUrl": "https://oauth2.sigstore.dev/auth",
"audience": "sigstore",
"challengeClaim": "email",
"spiffeTrustDomain": ""
}, Could we extend this data with more parameters that could guide the client for using the public good instance, like a |
It's not about programming difficulty, but user experience and CLI design. imagine a setup where TUF is not used: the user is trying to bootstrap their client to use some specific infrastructure (that is not the public good one):
But again, trusted root + a wellknown file in fulcio domain does not have these problems so I'm fine with that. |
I agree with this in the abstract, but I think in practice these concerns are already mixed 😅 -- signing is defined as also including a verification step over each signed artifact (although most clients don't currently do this), and signers also independently depend on the trust root for key material during SCT and CSR response verification. One random idea that I just had that could be a suitable compromise: maybe the we could define a new message that encompasses the trusted root, but that also includes a single reference OIDC endpoint for signing purposes? Sigstore could then continue to serve the "normal" trust root via TUF, and clients could accept the "super" one via the CLI. But that might cause even more confusion 😕 |
I have been thinking in these terms too, like this: message TrustedRoot {
...
}
message OIDCProviders {
...
}
message ClientConfig {
tr TrustedRoot
op OIDCProviders
} This would give us pretty good flexibility, we can ship two files via TUF, signing clients can assemble them in to a Maybe it is a bit hacky if we ship to files and clients assembles them from the TUF root if not provided as a command line parameter? I'm not sure tbh 🤔 I think this may work and be "good enough" 😬 |
Yeah, this is the case I'm worried about 😅 -- the goal here is to reduce user confusion around "what" instance of Sigstore they're communicating with/what materials they're trusting, and two inputs instead of one means 3 error states, one valid state in the worst case. (I agree with @jku that a well-known file also solves these issues adequately, so I'm good with that!) |
I can prepare a PR that updates the comment section for the trust root, that Fulcio CAs exposes the OIDC config at But there is still one problem I believe as I mentioned in the above comment, currently there are three OPs that use the |
Using a well-known Fulcio endpoint for this is my preferred solution, but like @kommendorkapten is saying, I'm not sure that the existing One option is to have the Fulcio configuration optionally specify something like |
(Accidental close, sorry!)
Agreed with this -- my 0.02c preference would be for these being an "atomic" file/message, to avoid the proliferation of CLI states like we've seen with clients so far (most of which end up being invalid/non-functional). A user could of course still populate that "infrastructure" input with incompatible fields, but at least the API/CLI facing component is simplified. In practice, the current
Does this sound generally workable to everyone? I think this fulfills the original use case with only a very small addition to the |
Sgtm; I'm not super fond of the "first" active tlog/ca is the signing item, since conventions are less explicit. Is it too weird for that to be an index in the infra/signer submessage referencing an item in the ca/log/ts fields? |
It might be better to require the client specify which CA or tlog will be used in signing when there are multiple (or via an index or key hint or something). |
Agreed that the implicitness isn't ideal. Bouncing off @haydentherapper's idea: maybe we could add a |
I'm not sure the Some filtering will need to happen by the client, and some operations may fail, as during the overlapping time, both instances may not actually be concurrently active. Or is the idea to use "certificateAuthorities": [
{
"for_signing": true,
"subject": {
"organization": "sigstore.dev",
"commonName": "sigstore"
},
"uri": "https://fulcio.sigstage.dev",
"certChain": {
"certificates": [...]
}
},
{
"for_signing": false,
"subject": {
"organization": "foo.bar",
"commonName": "baz"
},
"uri": "https://foo.bar.test",
"certChain": {
"certificates": [...]
}
},
] In this case the client can verify signatures from the CA |
Yep, exactly! |
So, there's another potential issue with defining signing cues in trusted_root. If there are two trusted_roots that need to be merged -- say an org wants to trust the public instance and their own private infra. Merging is pretty seamless if the trust root follows the current design -- in that merge conflicts are basically non existent. Clients may even be able to just handle this in the future With the bool injected into the trusted root, now someone has to handle the conflict of two potential signing infras -- maybe processing this each time. I think I still lean to the simplicity of signing.json just being a single separate file with like 4 urls in it (or a few url groups -- to handle log lists or whatever) |
Yes, this is what I would prefer as well (as I have mentioned before). Separation of concerns, I think the design is easier to reason about in that way and we can specialize each file for the use-case. |
I understand this, but I think it might be the wrong thing to optimize for: IMO merging cryptographic materials from composite should always be manually cross-checked, so optimizing for ease of merge isn't quite as valuable as reducing the number of error states that a user can surface. That being said, I agree that the current Given that, maybe @kommendorkapten's proposal in #259 (comment) is the most workable? With that, a user could pass in |
yeah good point :) In the case of #259 (comment) I think I would still like to see SigningTargets (or something named like that) instead of just OIDCProviders that has fully populated signing information. where it might look more like:
|
Yeah, I'm okay with that -- I can start on the PR here and we can iterate on naming/ultimate structure 🙂 |
Opened #277 to iterate on this! |
I don't want to muddy this conversation even more... but how do we define which log to contact when a rekor entry is not provided during verification? Or is this just a valid usecase anymore. The java client's initial design was to go looking for the entry in rekor.sigstore.dev. But which that being configurable, it makes less sense. |
Should we say this is solved by v2+ bundles because inclusion proof is required, so offline verification should only be done? If a v1 bundle is presented without a proof, then default to the public instance or a provided URL? And if you want to force online verification, then we should require a URL presented? |
Sure, but are we asserting that all verification must include a rekor entry? Is I guess now it's |
No, you're correct, a rekor entry is not required - code. This depends on policy, particularly whether we want the client to assert a default policy (what you're suggesting here would be at least one log contains the entry with a default log) vs the user presenting a policy (at which point they can specify which logs to query). |
or should it just query ALL logs it knows about (in the trusted_root) until it finds it? Which I think probably makes sense... so maybe this wasn't confusing at all and java just had a weird design. |
All as a default, with the option to specify a specific log if you know where it is, seems reasonable. |
I think this may need further clarifications in the client spec, but I would treat it like this. SigningWhen a client is signing an artifact it should publish the signature on every transparency log in the VerificationHere we don't need any update IMO. We already have support for |
For That being said, I think the changes here don't affect the verification path anyways: the |
Should clients be removing the option of verification separates? Only bundles now? |
That's the direction we're slowly moving to with |
As an aside, one thing we don't have currently that ct log lists do is the notion of operators, so that you can group a set of logs together under the same operator. Policy is defined by the number of operators, not unique logs, so you can't submit to two sharded logs by the same operator and pass a policy. We don't need this right now since we freeze old log shards and we don't have multiple logs operated by one shard though, and it should be straightforward to add later on. |
For a given trusted root (especially with sigstore public good), the fulio/ca instance should optionally be allowed to specify a(or many?) sigstore specific trusted oidc provider(s).
Why?
This allows clients to fully initialize their state from a single trusted_root.json, currently the only information missing is what would be required to initialize the oidc client for dex.
For example:
client_id
,well_known_configuration
The text was updated successfully, but these errors were encountered: