You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "Galsza (via GitHub)" <gi...@apache.org> on 2023/09/18 15:49:31 UTC
[GitHub] [ozone] Galsza opened a new pull request, #5313: HDDS-8960. Hold the rootCA's private key only in memory for the time of initialization/rotation, then forget it
Galsza opened a new pull request, #5313:
URL: https://github.com/apache/ozone/pull/5313
## What changes were proposed in this pull request?
Saving the root ca certificate public/private key can be avoided by only storing it in memory.
This also involves changing the certificate structure for the system as the only certificate signed by the root ca certificate is the first scm's sub-ca and later we use that certificate to sign other scm's sub-cas. This results in the full trust chain becoming 1 certificate longer.
Lot of code was removed around building a CA list, which no longer needs to be an actual list. This method's goal was providing the basis for the trust chain, but as we are using certificate paths now only the root ca certificate is needed to for verifying a full chain.
[HDDS-8960](https://issues.apache.org/jira/browse/HDDS-8960)
Testing was done using CI as well as observing the root ca keys missing. Here is a green CI run:
https://github.com/Galsza/ozone/actions/runs/6221002097
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] Galsza commented on pull request #5313: HDDS-8960. Hold the rootCA's private key only in memory for the time of initialization/rotation, then forget it
Posted by "Galsza (via GitHub)" <gi...@apache.org>.
Galsza commented on PR #5313:
URL: https://github.com/apache/ozone/pull/5313#issuecomment-1735788651
@fapifta I've updated this patch with the changes we have discussed, please take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ChenSammi commented on pull request #5313: HDDS-8960. Hold the rootCA's private key only in memory for the time of initialization/rotation, then forget it
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on PR #5313:
URL: https://github.com/apache/ozone/pull/5313#issuecomment-1725060018
Hi @Galsza , can you extract the buildCAX509List part into a new patch? I think it's not related with the goal of this one.
Also, I'm kind of understand the new logic of DefaultCAServer why rootCaKeyPair is needed. But it's not that straight forward. I'm think if we can do some refactor of this DefaultCAServer. Currently it's mixed with root CA function with sub CA function. Is it possible that we separate it into two class, one for root CA and another for sub CA in this patch, so that it will be natural that root CA doesn't persist key materials, while sub CA do?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-8960. Hold the rootCA's private key only in memory for the time of initialization/rotation, then forget it [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5313:
URL: https://github.com/apache/ozone/pull/5313#issuecomment-1926006500
Thank you very much for the patch. I am closing this PR __temporarily__ as there was no activity recently and it is waiting for response from its author.
It doesn't mean that this PR is not important or ignored: feel free to reopen the PR at any time.
It only means that attention of committers is not required. We prefer to keep the review queue clean. This ensures PRs in need of review are more visible, which results in faster feedback for all PRs.
If you need ANY help to finish this PR, please [contact the community](https://github.com/apache/hadoop-ozone#contact) on the mailing list or the slack channel."
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] Galsza commented on pull request #5313: HDDS-8960. Hold the rootCA's private key only in memory for the time of initialization/rotation, then forget it
Posted by "Galsza (via GitHub)" <gi...@apache.org>.
Galsza commented on PR #5313:
URL: https://github.com/apache/ozone/pull/5313#issuecomment-1726106166
Hi @ChenSammi , some changes to the buildCAList are necessary, but I can extract it later. I have reverted back the changes to a basic version now.
Regarding the root CA key generation I'm not sure how this separation can be done better. I've inlined the generateKeys function to make it clearer that it's not invoked from any other place. The SUB-CA certificate keys are generated and stored in an entirely different manner, during signing and storing the certificates. Do you have any advice on how to improve this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ChenSammi commented on pull request #5313: HDDS-8960. Hold the rootCA's private key only in memory for the time of initialization/rotation, then forget it
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on PR #5313:
URL: https://github.com/apache/ozone/pull/5313#issuecomment-1727744231
> Hi @ChenSammi , some changes to the buildCAList are necessary, but I can extract it later. I have reverted back the changes to a basic version now.
I know it's necessary. I just say we can submit these part of changes with a new JIRA of a proper title.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] fapifta commented on a diff in pull request #5313: HDDS-8960. Hold the rootCA's private key only in memory for the time of initialization/rotation, then forget it
Posted by "fapifta (via GitHub)" <gi...@apache.org>.
fapifta commented on code in PR #5313:
URL: https://github.com/apache/ozone/pull/5313#discussion_r1337751952
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java:
##########
@@ -871,38 +869,11 @@ certificateStore, null, new DefaultCAProfile(),
CAType.SUBORDINATE);
}
- // If primary SCM node Id is set it means this is a cluster which has
- // performed init with SCM HA version code.
- if (scmStorageConfig.checkPrimarySCMIdInitialized()) {
Review Comment:
I believe we should not remove this code piece.
Ozone 1.2 introduced SCM HA, but we should enable the upgrade from Ozone 1.1 with a single SCM node to Ozone 1.4 probably, and this code part deals with the things required to recoginize that we are after such an upgrade, set the cluster up that way, and make it possible to switch to SCM HA after the upgrade.
I believe this is something which we should keep at least until we do a major version change, as this is an incompatible change we should not do it in a minor release.
Also in such an upgrade scenario, we already have the keys on disk, and we need the rootCAServer as we create the sub-CA certificates during the first steps after the upgrade happened, and for that we need the rootCA server.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java:
##########
@@ -389,18 +391,25 @@ public static List<String> buildCAList(CertificateClient certClient,
if (!SCMHAUtils.isSCMHAEnabled(configuration)) {
return generateCAList(certClient);
} else {
+ Set<X509Certificate> rootCAs = certClient.getAllRootCaCerts();
Review Comment:
With the addition of this, I believe the current code will never pass through after the return statement in 400.
We should definitely spend some time clearing out this code with compatibility needs in mind. Can you please file an additional JIRA for this under HDDS-7335 as a sub-task please?
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java:
##########
@@ -389,18 +391,25 @@ public static List<String> buildCAList(CertificateClient certClient,
if (!SCMHAUtils.isSCMHAEnabled(configuration)) {
return generateCAList(certClient);
} else {
+ Set<X509Certificate> rootCAs = certClient.getAllRootCaCerts();
+ if (!rootCAs.isEmpty()) {
+ List<String> rootCaCertPems = new ArrayList<>();
+ for (X509Certificate rootCA : rootCAs) {
+ rootCaCertPems.add(CertificateCodec.getPEMEncodedString(rootCA));
+ }
+ return rootCaCertPems;
+ }
Collection<String> scmNodes = SCMHAUtils.getSCMNodeIds(configuration);
- int expectedCount = scmNodes.size() + 1;
Review Comment:
Thinking this through a bit, this change seems to be an interesting one.
This check was useful, as the old trust model was based on getting all the sub-CA certificate and the rootCA certificate, and then validating single certificates based on that. In our new system, we rely only on the rootCA certificate.
But when a current client talks to an older cluster, we still need to get all the certificates. And in this case the certClient.getAllRootCaCerts() method is unknown on the server side, and I guess we should get an exception to handle, and then we should fallback to the old behaviour with the old check...
We discussed about this change, but since then I realized that it is not enough to just simplify the code here, but we need to keep the possibility to talk to an older cluster where we have the old system.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-8960. Hold the rootCA's private key only in memory for the time of initialization/rotation, then forget it [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #5313:
URL: https://github.com/apache/ozone/pull/5313#issuecomment-1890979767
/pending
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-8960. Hold the rootCA's private key only in memory for the time of initialization/rotation, then forget it [ozone]
Posted by "Galsza (via GitHub)" <gi...@apache.org>.
Galsza commented on PR #5313:
URL: https://github.com/apache/ozone/pull/5313#issuecomment-1742838264
Converted to draft because compatibility issues require further examination.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-8960. Hold the rootCA's private key only in memory for the time of initialization/rotation, then forget it [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #5313: HDDS-8960. Hold the rootCA's private key only in memory for the time of initialization/rotation, then forget it
URL: https://github.com/apache/ozone/pull/5313
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org