Skip to content

Only check validity of certs in the chain of the node certificates#4979

Merged
cwperks merged 9 commits into
opensearch-project:mainfrom
cwperks:validate-certs-in-chain
Apr 14, 2025
Merged

Only check validity of certs in the chain of the node certificates#4979
cwperks merged 9 commits into
opensearch-project:mainfrom
cwperks:validate-certs-in-chain

Conversation

@cwperks

@cwperks cwperks commented Dec 20, 2024

Copy link
Copy Markdown
Member

Description

This PR updates the certificate validation checks on bootup to limit validation to only the certificates within the chain of the node certificates. In 2.18.0 there was a change that validated all certificates contained in a bundle even if they were not part of the chain from the node certificates. Since those certs are not pertinent to OpenSearch, the validation does not need to occur.

Opening this in Draft as POC to start soliciting some feedback. Automated tests need to be added for different scenarios.

  • Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)

Bug fix

Issues Resolved

Related issue: #4949

See discussion on forum: https://forum.opensearch.org/t/is-this-an-issue-with-opensearch-or-the-security-plugin-upgrading-from-2-17-1-to-2-18-0/22395

Check List

  • New functionality includes testing
  • New functionality has been documented
  • New Roles/Permissions have a corresponding security dashboards plugin PR
  • API changes companion pull request created
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Craig Perkins <cwperx@amazon.com>
@cwperks cwperks changed the title Only check validate of certs in the chain of the node certificates Only check validity of certs in the chain of the node certificates Dec 20, 2024
cwperks added 3 commits March 19, 2025 09:54
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
@codecov

codecov Bot commented Mar 19, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 67.30769% with 17 lines in your changes missing coverage. Please review.

Project coverage is 72.01%. Comparing base (37d259c) to head (21be4b8).
Report is 34 commits behind head on main.

Files with missing lines Patch % Lines
.../opensearch/security/ssl/config/KeyStoreUtils.java 51.72% 11 Missing and 3 partials ⚠️
.../org/opensearch/security/ssl/SslConfiguration.java 81.25% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4979      +/-   ##
==========================================
- Coverage   72.07%   72.01%   -0.06%     
==========================================
  Files         336      336              
  Lines       22614    22648      +34     
  Branches     3554     3560       +6     
==========================================
+ Hits        16298    16309      +11     
- Misses       4543     4567      +24     
+ Partials     1773     1772       -1     
Files with missing lines Coverage Δ
...rch/security/ssl/config/KeyStoreConfiguration.java 67.53% <100.00%> (+2.74%) ⬆️
...h/security/ssl/config/TrustStoreConfiguration.java 63.15% <100.00%> (ø)
.../org/opensearch/security/ssl/SslConfiguration.java 77.19% <81.25%> (-1.66%) ⬇️
.../opensearch/security/ssl/config/KeyStoreUtils.java 54.54% <51.72%> (-1.78%) ⬇️

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/test/resources/ssl/root-ca-invalid.pem
Comment thread src/test/java/org/opensearch/security/ssl/SSLTest.java
Signed-off-by: Craig Perkins <cwperx@amazon.com>
@cwperks cwperks marked this pull request as ready for review April 9, 2025 13:32
@cwperks

cwperks commented Apr 9, 2025

Copy link
Copy Markdown
Member Author

To generate new certs to add to the truststore I use commands like:

openssl genrsa -out root-ca2-key.pem 2048
openssl req -new -x509 -sha256 -days 1 -key root-ca2-key.pem -subj "/DC=com/DC=example/O=Example Com Inc./OU=Example Com Inc. Invalid CA/CN=Example Com Inc. Invalid CA" -addext "basicConstraints = critical,CA:TRUE" -addext "keyUsage = critical, digitalSignature, keyCertSign, cRLSign" -addext "subjectKeyIdentifier = hash" -addext "authorityKeyIdentifier = keyid:always,issuer:always" -out root-ca-invalid.pem

To import the new cert to the truststore:

keytool -import -trustcacerts -file root-ca-invalid.pem -alias root-ca-invalid -keystore truststore_invalid.jks

The password for the test truststore is changeit

willyborankin
willyborankin previously approved these changes Apr 9, 2025

@DarshitChanpura DarshitChanpura left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @cwperks left couple of comments and couple of nits.

Comment thread src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java Outdated
Comment thread src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java Outdated
Comment thread src/test/java/org/opensearch/security/ssl/SSLTest.java Outdated
Comment thread src/main/java/org/opensearch/security/ssl/config/KeyStoreUtils.java
Signed-off-by: Craig Perkins <cwperx@amazon.com>
@cwperks cwperks merged commit 280d8e5 into opensearch-project:main Apr 14, 2025
DarshitChanpura added a commit to DarshitChanpura/security that referenced this pull request Apr 21, 2025
…pensearch-project#4979)

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Co-authored-by: Darshit Chanpura <dchanp@amazon.com>
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
@cwperks cwperks added the backport 2.19 backport to 2.19 branch label May 14, 2025
@opensearch-trigger-bot

Copy link
Copy Markdown
Contributor

The backport to 2.19 failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/security/backport-2.19 2.19
# Navigate to the new working tree
pushd ../.worktrees/security/backport-2.19
# Create a new branch
git switch --create backport/backport-4979-to-2.19
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 280d8e5fb80e7c0732a162ea9f682a75040593d3
# Push it to GitHub
git push --set-upstream origin backport/backport-4979-to-2.19
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/security/backport-2.19

Then, create a pull request where the base branch is 2.19 and the compare/head branch is backport/backport-4979-to-2.19.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.19 backport to 2.19 branch backport-failed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants