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

Generate new demo certs with IPv6 loopback added to SAN in node certificate #3268

Conversation

DarshitChanpura
Copy link
Member

@DarshitChanpura DarshitChanpura commented Aug 29, 2023

Description

This PR solves bug where backwards compatibility tests would fail for IPv6 loopback address (::1) with:
No subject alternative names matching IP address ::1 found

  • Category: Maintenance

Exit criteria:

  • BWC tests should no longer fail with the above error.

Issues Resolved

Testing

Manual testing

Check List

  • New functionality includes testing
  • New functionality has been documented
  • 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: Darshit Chanpura <[email protected]>
…um calculator tool to allow easier generation of checksums whenever certificates are rotated

Signed-off-by: Darshit Chanpura <[email protected]>
@codecov
Copy link

codecov bot commented Aug 29, 2023

Codecov Report

Merging #3268 (804f377) into main (47a047c) will increase coverage by 0.74%.
Report is 2 commits behind head on main.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #3268      +/-   ##
============================================
+ Coverage     62.49%   63.23%   +0.74%     
- Complexity     3399     3448      +49     
============================================
  Files           259      263       +4     
  Lines         20055    20029      -26     
  Branches       3370     3341      -29     
============================================
+ Hits          12533    12665     +132     
+ Misses         5872     5737     -135     
+ Partials       1650     1627      -23     
Files Changed Coverage Δ
.../opensearch/security/OpenSearchSecurityPlugin.java 84.53% <100.00%> (+0.11%) ⬆️

... and 35 files with indirect coverage changes

@DarshitChanpura
Copy link
Member Author

Looking into plugin-install failures now. Seems like we need to generate a new JKS file for sanity tests since the certificates were updated.

@@ -0,0 +1,71 @@
/*
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need a tool for this? There are many command line tools available for this kind of purpose, e.g. https://stackoverflow.com/questions/3358420/generating-a-sha-256-hash-from-the-linux-command-line

Copy link
Member Author

Choose a reason for hiding this comment

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

Thought it'd be of value to add capability to generate shas of all files in a folder with a given extension. Otherwise they'd have to cat each file manually and run sha256sum. i.e. cd <cert-folder>; cat cert.pem | sha256sum

But the main purpose was to seek feedback on PR whether we should ship with this or not. I felt it might be useful down the line so I added it.

Copy link
Member

Choose a reason for hiding this comment

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

If instead this tool that generated the certs and then updated all of the relevant locations (including creating the checksums) - I would be onboard.

@DarshitChanpura DarshitChanpura force-pushed the add-ipv6-loopback-demo-certs branch 2 times, most recently from ef512ed to 2bd4eae Compare August 30, 2023 19:34
@DarshitChanpura DarshitChanpura force-pushed the add-ipv6-loopback-demo-certs branch from 2bd4eae to 0e4d427 Compare August 30, 2023 20:49
Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

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

We should not ship ChecksumCalcuator as part of the plugin distribution, its purpose is to automate making changes in the java code, not something a consumer of the plugin would need.

@DarshitChanpura DarshitChanpura force-pushed the add-ipv6-loopback-demo-certs branch from 2fa9757 to 6aacfc0 Compare August 31, 2023 17:04
@DarshitChanpura DarshitChanpura force-pushed the add-ipv6-loopback-demo-certs branch from 6ab4ae4 to 804f377 Compare August 31, 2023 17:47
@DarshitChanpura
Copy link
Member Author

We should not ship ChecksumCalcuator as part of the plugin distribution, its purpose is to automate making changes in the java code, not something a consumer of the plugin would need.

Agreed. I have added a section in Developer guide on steps to refresh demo certs.

@cwperks
Copy link
Member

cwperks commented Aug 31, 2023

@DarshitChanpura Is there any way to 'diff' the certs?

i.e. I'd like to verify that the only differences are:

  1. Issue date
  2. Expiry date
  3. the SAN in the updated certs is mostly the same, but in the new certs it contains the IPv6 loopback address (::1)

@DarshitChanpura
Copy link
Member Author

@DarshitChanpura Is there any way to 'diff' the certs?

i.e. I'd like to verify that the only differences are:

  1. Issue date
  2. Expiry date
  3. the SAN in the updated certs is mostly the same, but in the new certs it contains the IPv6 loopback address (::1)

You can compare certs by running openssl x509 -noout -text -in '<cert>.pem' side-by-side in two terminals and then compare the old and new ones.

@cwperks
Copy link
Member

cwperks commented Aug 31, 2023

This is the diff I created locally for reference. Most of the differences look ok to me:

(Cleaned up the diff to make it easier to read)

Expand to see the diff:
diff esnode.txt esnode-new.txt
diff1
<             Not Before: Apr 22 03:43:47 2018 GMT
<             Not After : Apr 19 03:43:47 2028 GMT
<         Subject: DC = de, L = test, O = node, OU = node, CN = node-0.example.com
---
>             Not Before: Aug 29 19:44:42 2023 GMT
>             Not After : Aug 26 19:44:42 2033 GMT
>         Subject: C = de, L = test, O = node, OU = node, CN = node-0.example.com
diff2
<             X509v3 Authority Key Identifier:
<                 keyid:92:35:0C:E0:0F:1E:2B:45:F6:4D:39:F3:7B:5F:A2:E6:12:97:40:73
<                 DirName:/DC=com/DC=example/O=Example Com Inc./OU=Example Com Inc. Root CA/CN=Example Com Inc. Root CA
<                 serial:01
---
>             X509v3 Authority Key Identifier:
>                 17:87:DF:A0:5A:EB:66:12:A7:D5:D0:F8:BA:12:45:3C:B7:2B:00:9C
diff3
<             X509v3 Key Usage: critical
<             X509v3 Extended Key Usage: critical
---
>             X509v3 Key Usage:
>             X509v3 Extended Key Usage:
diff4
<             X509v3 Subject Alternative Name:
<                 Registered ID:1.2.3.4.5.5, DNS:node-0.example.com, DNS:localhost, IP Address:127.0.0.1
Address:127.0.0.1
---
>             X509v3 Subject Alternative Name:
>                 Registered ID:1.2.3.4.5.5, DNS:node-0.example.com, DNS:localhost, IP Address:0:0:0:0:0:0:0:1, IP 

@peternied
Copy link
Member

@cwperks What do you think is missing? By virtue of BWC tests passing, and Plugin Install passing - I am assuming they work.

@cwperks
Copy link
Member

cwperks commented Aug 31, 2023

@peternied Just trying to cover all the bases. This PR looks good to me.

@DarshitChanpura DarshitChanpura merged commit a4f8f03 into opensearch-project:main Aug 31, 2023
@DarshitChanpura DarshitChanpura added the backport 2.x backport to 2.x branch label Aug 31, 2023
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x 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.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/security/backport-2.x
# Create a new branch
git switch --create backport/backport-3268-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 a4f8f0399a41b11a52e7d3a8f00dca49cae7be4f
# Push it to GitHub
git push --set-upstream origin backport/backport-3268-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/security/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-3268-to-2.x.

DarshitChanpura added a commit to DarshitChanpura/security that referenced this pull request Aug 31, 2023
…ficate (opensearch-project#3268)

Solves bug where backwards compatibility tests would fail for
IPv6 loopback address (`::1`) with:
`No subject alternative names matching IP address ::1 found`

Signed-off-by: Darshit Chanpura <[email protected]>
(cherry picked from commit a4f8f03)
willyborankin pushed a commit that referenced this pull request Aug 31, 2023
…N in node certificate (#3268) (#3277)

Backports #3268 by cherry-picking
a4f8f03

Manual backport was required to address conflicts in DEVELOPER_GUIDE.md
@xfournet
Copy link

Unfortunately this has generated an already expired root CA certificate : valid from 29/08/2023 to 28/09/2023

DarshitChanpura added a commit that referenced this pull request Feb 22, 2024
…expired root ca certificate (#4061)

### Description

During the last renewal of certs
#3268, the option
`-days 3650` was missed for root-ca.pem cert causing it to set the
default expiry of 30 days. This PR regenerates the public cert
root-ca.pem, using the same private-key, and it also regenerate public
certs `es-node.pem` and `kirk.pem` so that they can be verified with
this new certificate.
* Category : Bug fix
* Why these changes are required?
    - To ensure the expiry is in 10 years from now
* What is the old behavior before changes and new behavior after
changes?
- root-ca is currently expired, and this change will set expiry to 2034

### Issues Resolved
- Resolves #4047


### Testing
- Automated testing + [Manual
Testing](#4061 (comment))



---------

Signed-off-by: Darshit Chanpura <[email protected]>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 22, 2024
…expired root ca certificate (#4061)

### Description

During the last renewal of certs
#3268, the option
`-days 3650` was missed for root-ca.pem cert causing it to set the
default expiry of 30 days. This PR regenerates the public cert
root-ca.pem, using the same private-key, and it also regenerate public
certs `es-node.pem` and `kirk.pem` so that they can be verified with
this new certificate.
* Category : Bug fix
* Why these changes are required?
    - To ensure the expiry is in 10 years from now
* What is the old behavior before changes and new behavior after
changes?
- root-ca is currently expired, and this change will set expiry to 2034

### Issues Resolved
- Resolves #4047

### Testing
- Automated testing + [Manual
Testing](#4061 (comment))

---------

Signed-off-by: Darshit Chanpura <[email protected]>
(cherry picked from commit 9a6a018)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
dlin2028 pushed a commit to dlin2028/security that referenced this pull request May 1, 2024
…expired root ca certificate (opensearch-project#4061)

### Description

During the last renewal of certs
opensearch-project#3268, the option
`-days 3650` was missed for root-ca.pem cert causing it to set the
default expiry of 30 days. This PR regenerates the public cert
root-ca.pem, using the same private-key, and it also regenerate public
certs `es-node.pem` and `kirk.pem` so that they can be verified with
this new certificate.
* Category : Bug fix
* Why these changes are required?
    - To ensure the expiry is in 10 years from now
* What is the old behavior before changes and new behavior after
changes?
- root-ca is currently expired, and this change will set expiry to 2034

### Issues Resolved
- Resolves opensearch-project#4047


### Testing
- Automated testing + [Manual
Testing](opensearch-project#4061 (comment))



---------

Signed-off-by: Darshit Chanpura <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] The node certificate in the demo certificates does not include the IPv6 loopback address ::1
4 participants