Skip to content

refactor: add cert validation to the acquisition, remove old enrollment#2012

Open
istankovic wants to merge 34 commits intomainfrom
ivan/x509-part-10
Open

refactor: add cert validation to the acquisition, remove old enrollment#2012
istankovic wants to merge 34 commits intomainfrom
ivan/x509-part-10

Conversation

@istankovic
Copy link
Copy Markdown
Member

@istankovic istankovic commented Apr 8, 2026

This does a couple of things:

  • move Wire-specific checks from wire_e2e_identity::acme outside the acme module, because that module should deal only with general acme things
  • hook certificate checks in the acquistion process, so the caller cannot get a certificate if it fails validation for any reason
  • add a way to add more trust anchors to the PKI environment
  • make PKI environment mandatory when checking certificates and extracting identities from certificates
  • remove the old enrollment code; the last remaining bits under legacy are mostly types that are still being used and are going to be reworked later

@istankovic istankovic changed the title Ivan/x509 part 10 refactor: add cert validation to the acquisition, remove old enrollment Apr 8, 2026
It's time. With CC 10, credentials are handled in an entirely different
way.
The code there does not really have to do with acme at all. It contains
functionality required by leaf certificate verification, and keeping it
there would make error handling more complex. So just move it to the
acquisition module, where it belongs.
This is where those errors belong and where they will be returned from.
This is for errors coming from x509_check.
It is not ideal that we're still depending on AcmeError, but the
identity module needs it and while the error handling is a mess,
we don't really want to rework it right now.
… field

This is going to be used in certificate checks.
They really belong in the acquisition and we want to make it obvious
that they're being performed as part of the acquisition process,
rather than them being implied.
And a bunch of other legacy code.
It is used in crypto. Ideally it would be only used internally in
e2e-identity, but we're not at that point yet.
And add it to the credential configuration.
… a trust anchor

In this case, the status is going to be revoked, which is wrong. So just
test for expiration of the second certificate.
…ror type

This is going to be used in cases where an operation that requires a PKI
environment is attempted, but no PKI environment is set.
If we're dealing with x509 credentials, the PKI must be provided,
otherwise extract_identity will return an error. Previously this was not
the case.

It is unclear whether we even need WireIdentity in its current form,
and, ideally, we should consider reworking it, but for now this should
suffice.

The whole thing around accessing the "inner" PKI environment is quite
inelegant, due to a bunch of async locks.
rust-pki commit 9fed8a806f02367a44eeb13d8b72b1a1dac347b8 contains a fix
for get_trust_anchors function, which we need.
…I env

This is a small step towards simplifying the relation between the outer
and inner PKI environments. With this, if we are inside the outer PKI
environment, we can now be sure that there is always _some_ inner PKI
environment.
The idea is that eventually PKI environment becomes fully flexible and allows
freely manipulating the internal trust store.

This implementation is not exactly ideal because we are adding a new
trust anchor store for each new trust anchor. Ideally, we would
implement the TrustAnchorSource trait, but that is a solid amount of
work which we don't really need right now.
…cert

Because the root CA cert (step-ca's self-signed cert) is also going to be
added to the PKI environment in order to allow path validation.
This allows us to do path validation, which is the last stage of the
acquisition process.
@istankovic istankovic marked this pull request as ready for review April 10, 2026 14:31
@istankovic istankovic requested a review from a team April 10, 2026 14:31
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.

1 participant