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

Remove unnecessary actions #1

Closed
wants to merge 2 commits into from
Closed

Remove unnecessary actions #1

wants to merge 2 commits into from

Conversation

cipherboy
Copy link
Owner

Testing PR for GH Actions...

HashiCorp Vault v1.14.10 reports fixing the following security
vulnerability:

> auth/cert: compare public keys of trusted non-CA certificates with
> incoming client certificates to prevent trusting certs with the same
> serial number but not the same public/private key. [GH-25649]

From this, we can deduce that there is an issue with validating the
authenticity of leaf certificates. Luckily there is only one code path
so the fix location is obvious.

Note that the issue lies in how the check is performed: a client may
present an arbitrary certificate from an arbitrary CA (even a
self-signed one if it contains an AuthorityKeyId extension), which
will be accepted, assuming it matches the AKID and Serial Number of
a present, explicitly trusted leaf certificate.

In particular, while the authenticity of the TLS connection is verified
(insofar as the presented certificate matches the TLS channel via valid
signature), the authorization check is malformed as it does not
correctly tie the channel's connection key to the key contained in the
certificate.

This check is sufficient to differentiate certificates from different
CAs, say, for the purpose of chain building, but is not strong enough
for authentication and authorization.

In the past, several CAs (such as the production grade Dogtag PKI or
a manual OpenSSL CA) have defaulted to sequential serial numbers,
though, AKIDs would likely still be unique unless key material was
reused between different CAs (typically unlikely unless an intermediate
CA was re-issued with the same key material but longer expiration).

However, most CAs correspond to the CA/BF's guidelines which require
at least 20 bits of entropy, and thus would be less likely to run into
this organically, even with reused CA public keys.

See also: https://github.com/hashicorp/vault/releases/tag/v1.14.10
See also: https://discuss.hashicorp.com/t/hcsec-2024-05-vault-cert-auth-method-did-not-correctly-validate-non-ca-certificates/63382
See also: https://cabforum.org/working-groups/server/baseline-requirements/documents/

Resolves: openbao#172

Signed-off-by: Alexander Scheel <alexander.m.scheel@gmail.com>
This retains the original HashiCorp upstream build & test pipelines,
cleaning them up for OpenBao and removing HashiCorp internal tooling
references that aren't necessary for us.

The CI pipeline currently fails with test errors and commenting will
need to be tested on the main repository with an appropriately scoped
token. However, builds pass and produce usable, unsigned artifacts.

This can form the basis of a proper (signed) release pipeline
eventually, taking actions from the build stage of the tagged
release commit and signing and verifying them.

In order to fix CI, some changes to the Go modules were done, removing
redundant tooling packages and re-adding the kubernetes integration
tests. This also fixes CI to correctly run api & sdk tests, fixing openbao#61
again.

Removed, unnecessary actions:

 - actionlint was used to allow-list actions upstream,
 - add-hashicorp-contributed-label was used to add a label to internal
   PRs for visibility,
 - backport was the tool to automatically backport PRs,
 - milestone-checker was used to ensure PRs had appropriate milestones
   prior to merge,
 - oss was used to classify issues against the specified label category
 - remove-labels was used to clean up issues & PRs
 - security-scan requires internal tooling not made public
 - test-ci-bootstrap & test-ci-cleanup are both part of the complex Enos
   integration tests, which were removed in
   85455fb due to resource
   requirements.

Resolves: openbao#31
Resolves: openbao#42
Resolves: openbao#152
Related: openbao#153

Signed-off-by: Alexander Scheel <alexander.m.scheel@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant