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

[SPARK-45374][CORE] Add test keys for SSL functionality #43163

Closed
wants to merge 1 commit into from

Conversation

hasnain-db
Copy link
Contributor

What changes were proposed in this pull request?

This PR introduces test keys for SSL functionality. They keys were generated using something like the following:

openssl genpkey -algorithm RSA -out key.pem -hexseed deadbeef
openssl pkcs8 -topk8 -in key.pem -out key.pem.out
openssl req -new -key key.pem -out csr.csr -days 3650 -subj "/CN=test/ST=California/L=San Francisco/OU=Org1/O=Org2/C=US"
openssl x509 -req -in csr.csr -signkey key.pem -out certchain.pem -days 3650
rm key.pem csr.csr
mv key.pem.enc key.pem

And then copied to all the relevant folders. I also copied over the keystore and trustore files (did not regenerate those).

Why are the changes needed?

We need these test files to run tests using PEM keys for SSL connections.

Does this PR introduce any user-facing change?

No

How was this patch tested?

These test files will be used by follow up PRs. This was tested as part of #42685, whic is being split.

Was this patch authored or co-authored using generative AI tooling?

No

Copy link
Contributor

@mridulm mridulm left a comment

Choose a reason for hiding this comment

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

+CC @srowen as well ... these are meant to be for tests, want to make sure there are no concerns with potential 'security audit' related user reports, etc.

@srowen
Copy link
Member

srowen commented Sep 28, 2023

Where are these used though?

@hasnain-db
Copy link
Contributor Author

@srowen these were used in #42685, I am splitting up that PR into parts to make it easier to review, per @mridulm 's sugestion. The tests using these will be added soon (once this and a few other foundational PRs are merged)

@srowen
Copy link
Member

srowen commented Sep 28, 2023

OK, hm, I don't really see the value in splitting this part out, but it doesn't hurt. @mridulm has the ball.

Copy link
Contributor

@JoshRosen JoshRosen left a comment

Choose a reason for hiding this comment

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

these are meant to be for tests, want to make sure there are no concerns with potential 'security audit' related user reports, etc.

I think this will be okay because:

  1. These are only in test JARs
  2. Some of the test JARs already contain other forms of test key files, such as https://github.com/apache/spark/blob/master/sql/hive/src/test/resources/data/files/keystore.jks

@mridulm mridulm closed this in d52561c Sep 28, 2023
@mridulm
Copy link
Contributor

mridulm commented Sep 28, 2023

Merged to master.
Thanks for adding this @hasnain-db.
Thanks @srowen and @JoshRosen for review :-)

@mridulm
Copy link
Contributor

mridulm commented Sep 28, 2023

Just noticed that we are using the same jira for all of the PR's.
Can you create an umbrella jira and have individual tasks for each of the PR's please ? Makes it easier to track.

We can manually relink the merged PR's in the jira for now.

@hasnain-db
Copy link
Contributor Author

hasnain-db commented Sep 28, 2023

happy to do so, I'll update the inflight PRs that haven't merged yet and let you know if I need help re-linking.

EDIT: Updated the other PRs that have not been merged, and I will file more tasks for the upcoming PRs once I get to putting them up. Not sure how to relink the PRs in jira unfortunately.

@hasnain-db hasnain-db changed the title [SPARK-44937][CORE] Add test keys for SSL functionality [SPARK-45374][CORE] Add test keys for SSL functionality Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants