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

Backport [1.7.x]: Cassandra: Refactor PEM parsing logic (#11861) #11921

Merged
merged 1 commit into from
Jun 23, 2021

Conversation

pcman312
Copy link
Contributor

Backports #11861 to 1.7.x

Original PR description:

Description

This fixes an issue in the Cassandra database plugin TLS logic where providing only a custom CA doesn't work when using pem_bundle and partially works with pem_json. This now supports setting either a client certificate & private key, a custom CA, or both.

How does this work?

The certutil.ParsePEMBundle function makes assumptions about the PEM data it is parsing:

  1. It assumes the PEM bundle will be used for mTLS and therefore assumes the presence of both a client certificate and a custom CA, though it doesn't error if only the CA is provided. If only a CA is provided to this function, the CA certificate ends up in the Certificate field rather than CAChain which ends up not being interpreted correctly when converting to a tls.Config.
  2. It assumes the PEM data is in a specific order (client certificate before CA chain)

A new function has been written (pemBundleToTLSConfig) that parses the PEM bundle very similarly to certutil.ParsePEMBundle but does not make either of those assumptions.

Similarly, the certutil.ParsePKIJSON function ends up having some similar assumptions, however it does parse the CA correctly and thus the tls.Config works when connecting to Cassandra. However, the CA is also included as a client certificate when it shouldn't be. However, it appears that Cassandra will ignore a provided client certificate if not configured to use mTLS.

A new function was created (jsonBundleToTLSConfig) that parses the certificate data into a tls.Config object.

Testing

Additional automated tests were added in connection_producer_test.go that start up a Cassandra instance using TLS with a custom CA and connects to it using the same custom CA.

Also this was tested with a real Cassandra instance (also configured with a custom CA) and a real Vault instance configured to use just the custom CA when talking to Cassandra.

* Refactor TLS parsing

The ParsePEMBundle and ParsePKIJSON functions in the certutil package assumes
both a client certificate and a custom CA are specified. Cassandra needs to
allow for either a client certificate, a custom CA, or both. This revamps the
parsing of pem_json and pem_bundle to accomodate for any of these configurations
@pcman312 pcman312 added bug Used to indicate a potential bug secret/database/cassandra labels Jun 22, 2021
Copy link
Member

@austingebauer austingebauer left a comment

Choose a reason for hiding this comment

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

LGTM

@pcman312 pcman312 merged commit 6a403a6 into release/1.7.x Jun 23, 2021
@pcman312 pcman312 deleted the backport-cassandra-tls-custom-ca-1.7.x branch June 23, 2021 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug secret/database/cassandra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants