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

CORE-4182: dt/tls: Create chain of CRLs #19865

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

michael-redpanda
Copy link
Contributor

@michael-redpanda michael-redpanda commented Jun 17, 2024

The TLSChainCACertManager class in ducktape is updated to chain CRLs for each individual CA.

This PR is required to fix DT tests before enabling OpenSSL in Seastar which will be done in a subsequent vtools PR

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.1.x
  • v23.3.x
  • v23.2.x

Release Notes

  • None

The TLSChainCACertManager class in ducktape is updated to chain CRLs for
each individual CA.

Signed-off-by: Michael Boquard <michael@redpanda.com>
@michael-redpanda michael-redpanda requested a review from a team June 17, 2024 19:52
@michael-redpanda michael-redpanda self-assigned this Jun 17, 2024
@michael-redpanda michael-redpanda requested review from pgellert and oleiman and removed request for a team June 17, 2024 19:52
Copy link
Member

@oleiman oleiman left a comment

Choose a reason for hiding this comment

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

lgtm

# Now do the same for the CRLs
crl_files = [ca.crl for ca in self._cas]
crl_out = self._with_dir('ca', 'signing-crl-chain.crl')
pathlib.Path(out).touch()
Copy link
Member

Choose a reason for hiding this comment

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

actually, did you mean

Suggested change
pathlib.Path(out).touch()
pathlib.Path(crl_out).touch()

Copy link
Member

Choose a reason for hiding this comment

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

doesn't look like it matters, so I'm not sure what it was doing there previously 🤷

out = self._with_dir('ca', 'signing-ca-chain.pem')
pathlib.Path(out).touch()
with open(out, 'w') as outfile:
for fname in reversed(files):
for fname in reversed(ca_files):
Copy link
Contributor

Choose a reason for hiding this comment

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

question: I'm curious why reversed is needed here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good question! The ordering does matter, we effectively want a chain of certs that ends with the root cert:

Issuer CA 1 --- signed by ---> Issuer CA 2 --- signed by ---> Root CA

I think RFC5280 deals with this but I can't find the right section to reference

@michael-redpanda michael-redpanda merged commit 182fb1d into dev Jun 18, 2024
13 of 18 checks passed
@michael-redpanda michael-redpanda deleted the CORE-4182-Enable-OSSL-In-Seastar branch June 18, 2024 12:05
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.

3 participants