-
Notifications
You must be signed in to change notification settings - Fork 622
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
Regenerate testdata/pki and include script for regenerating #1805
Conversation
subjectAltName = @alt_names | ||
|
||
[alt_names] | ||
URI = spiffe://test.cassandra.apache.org/cassandra-gocql-driver/integrationTest/gocql |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note, I opted to include a URI:spiffe in the SAN of both the gocql and cassandra certificates as I intend to write a test that utilizes the mutual TLS feature introduced in 5.0. I will create a follow on PR for this later. I've already tried the feature out locally and it works great with gocql!
@@ -1,83 +1,31 @@ | |||
Certificate: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't realize until later that the original certs embedded text; I can add this if people think its useful, but one can always extract this using openssl x509 -in testdata/pki/cassandra.crt -text -noout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is useful to immediately see expiration date :). Did not know that this was possible before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 agreed, was just a matter of including -text
so added that!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also a bit useful to see some subtle differences with the new cert, like the inclusion of SPIFFE based SANs.
# limitations under the License. | ||
# | ||
|
||
# This script generates the various certificates used for integration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered having integration.sh call this file and regenerate certs every time, figured that may introduce some fragility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also try not to introduce more moving parts in integration tests. What do you think about increasing validity to 100 years?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds reasonable, these are self signed certificates so not much of a risk of having a long validity here. Updated.
# requires additional bag attributes that openssl doesn't provide. | ||
echo "Generating truststore .truststore for Cassandra" | ||
rm -fv .truststore | ||
keytool -import -keystore .truststore -trustcacerts \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am thinking whether we should run GoCQL tests with the same JVM / C* matrix as for Java driver. We do run tests with JVM 8 and there we need also JKS format. On the other hand, would it make sense to test Golang driver with C* on JVM 8? JVM version will of course not impact the driver itself, but only server-side component.
Just pointing this out but this is out, but I am fine leaving the PKC12 format only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably for the best as I encountered compatibility problems even while testing. I've pushed an update to use JKS instead.
@@ -1,83 +1,31 @@ | |||
Certificate: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is useful to immediately see expiration date :). Did not know that this was possible before.
# limitations under the License. | ||
# | ||
|
||
# This script generates the various certificates used for integration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also try not to introduce more moving parts in integration tests. What do you think about increasing validity to 100 years?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, +1 from my side.
Found a tiny issue while testing on mac:
Fixed in 18bfc10 |
@tolbertam Would you mind squashing this down to a single commit and adding the usual "patch by" notation to it? I'd like to get this in so that we can see a good clean test run for #1817 before we merge that... and, well, before we merge pretty much any other PR. :) |
will do! |
The existing certificates in testdata/pki expire on September 16 2024. This commit includes a 'generate_certs.sh' script for regenerating private keys and certificates as needed. As I couldn't find the original steps used to generate these, it's possible these certificates are generated differently, but they are done in a nominal way. One slight derivation with the original certificates is that I have opted to use PKCS12 format instead of the propertiary java JKS format for the .truststore and .keystore file. The cassandra and gocql certificates also embed a spiffe in the SAN so they can eventually be used for mTLS authentication testing. patch by Andy Tolbert; reviewed by Bret McGuire for CASSANDRA-19862
18bfc10
to
4d37238
Compare
Squashed and updated the commit with the Patch by; reviewed by addendum. Hopefully this is good to go! If it passes please feel free to merge it @absurdfarce |
All right, we're all set here @tolbertam; thanks for the fix! I haven't done anything to CASSANDRA-19862; I'll leave that to you. |
The existing certificates in testdata/pki expire on September 16 2024.
This commit includes a 'generate_certs.sh' script for regenerating private keys and certificates as needed.
As I couldn't find the original steps used to generate these, it's possible these certificates are generated differently, but they are done in a nominal way.
One slight derivation with the original certificates is that the cassandra and gocql certificates also now embed a spiffe in the SAN so they can eventually be used for mTLS authentication testing.
patch by Andy Tolbert; reviewed by <> for CASSANDRA-19862