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.

Sign up for GitHub

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

Jump to bottom

CORE-3168 support p12 #21313

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

CORE-3168 support p12 #21313

wants to merge 6 commits into from

Conversation

michael-redpanda
Copy link
Contributor

Adds support for using PKCS#12 files in our TLS node config.

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

Features

  • Added support for using PKCS#12 files for TLS services
@michael-redpanda michael-redpanda requested a review from a team July 9, 2024 20:28
@michael-redpanda michael-redpanda self-assigned this Jul 9, 2024
@michael-redpanda michael-redpanda requested review from aanthony-rp, BenPope and oleiman and removed request for a team July 9, 2024 20:28
@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Jul 9, 2024 •
edited
Loading

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/51285#01909987-db34-4048-ad16-d61e783972e3

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/51285#01909988-4371-4f37-b903-68ef89793006

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/51285#01909988-4374-4260-96b8-e4a8fa10be48
oleiman
oleiman reviewed Jul 10, 2024
View reviewed changes
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. several comments, all nitpicks
src/v/config/tests/tls_config_convert_test.cc Outdated Show resolved Hide resolved
src/v/config/tls_config.cc Outdated Show resolved Hide resolved
src/v/config/tls_config.cc Outdated Show resolved Hide resolved
src/v/config/tls_config.h Outdated Show resolved Hide resolved
tests/rptest/services/tls.py Outdated Show resolved Hide resolved
tests/rptest/services/tls.py Outdated Show resolved Hide resolved
tests/rptest/tests/pkcs12_test.py Outdated Show resolved Hide resolved
aanthony-rp
aanthony-rp previously approved these changes Jul 10, 2024
View reviewed changes
Copy link
Contributor

@aanthony-rp aanthony-rp left a comment

Choose a reason for hiding this comment

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

This adds support for P12 password files, and also seems to extend the implementation to accept multiple key/cert file pairs instead of just a single pair. The test coverage looks good and I don't see any controversial points.
@michael-redpanda
Copy link
Contributor Author

Force push 4818ef6:

  • Updates from PR comments
oleiman
oleiman previously approved these changes Jul 10, 2024
View reviewed changes
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.

👍
src/v/config/tls_config.cc Outdated Show resolved Hide resolved
aanthony-rp
aanthony-rp previously approved these changes Jul 12, 2024
View reviewed changes
Copy link
Contributor

@aanthony-rp aanthony-rp left a comment

Choose a reason for hiding this comment

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

I'll trust Oren and Ben to approve the changes they requested earlier.
@michael-redpanda
Copy link
Contributor Author

Force push 12ea872:

  • Updates from PR commet
@michael-redpanda
configuration: Add support for P12 files
cb10bc4 
PKCS#12 files (or PFX files) can be used to hold various cryptographic
keys and certificates in a secure way.  These files are encrypted using
a user provided password and allows for secure transmission of keys and
certs from an issuer to an endpoint.  This commit allows for users of
Redpanda to use a P12 file (with a supplied password for decryption)
instead of a plain-text key and cert when setting up TLS configurations.

Signed-off-by: Michael Boquard <michael@redpanda.com>
@michael-redpanda
dt: Cleanup of tls.py
28a09c8 
Signed-off-by: Michael Boquard <michael@redpanda.com>
@michael-redpanda
dt: Added ability to generate P12 files
5151053 
Signed-off-by: Michael Boquard <michael@redpanda.com>
@michael-redpanda
dt: Added saving of PKCS12 file to node
c50a2c5 
Signed-off-by: Michael Boquard <michael@redpanda.com>
@michael-redpanda
dt: Added plumbing for PKCS12 file
18ddeb1 
Signed-off-by: Michael Boquard <michael@redpanda.com>
@michael-redpanda
dt: Added pkcs12 test
90d4bdb 
Added PKCS#12 file smoke test

Signed-off-by: Michael Boquard <michael@redpanda.com>
@michael-redpanda
Copy link
Contributor Author

Force push 90d4bdb:

  • One more nit fix
aanthony-rp
aanthony-rp approved these changes Jul 15, 2024
View reviewed changes
Copy link
Contributor

@aanthony-rp aanthony-rp left a comment

Choose a reason for hiding this comment

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

lgtm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/redpanda
5 participants
@michael-redpanda @vbotbuildovich @BenPope @oleiman @aanthony-rp