Skip to content

Conversation

@xiaoyuyao
Copy link
Contributor

@xiaoyuyao xiaoyuyao commented Oct 1, 2020

What changes were proposed in this pull request?

Fix the encoding of KeyUsage in the CSR to ensure it can be used in FIPS environment.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-4301

How was this patch tested?

FIPS enabled secure ozone cluster and verify that the CA certificate can be properly generated and used.

@elek elek changed the title HDDS-4301. SCM CA certificate does not encode KeyUsage extension prop… HDDS-4301. SCM CA certificate does not encode KeyUsage extension properly Oct 9, 2020
Copy link
Member

@elek elek left a comment

Choose a reason for hiding this comment

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

Thanks to fix this @xiaoyuyao

I don't fully understand it (yet), and I have no FIPS certificate suite off-the-shelf, so I am asking to learn and to make it possible to merge it ;-)

  1. Is it a backward compatible change? If I understood well both the old method and new method is good enough for validation, but the new version is more standard.

  2. There is a new DEROctetString(keyUsage) call in SelfSignedCertificate which is very similar to the one which is replaced in CertificateSigningRequest.

Do we need to replace it? Is it always better to use keyUsage.getEncoded() instead of DEROctetString?

@xiaoyuyao
Copy link
Contributor Author

bq. Is it a backward compatible change? If I understood well both the old method and new method is good enough for validation, but the new version is more standard.

Yes. The new version is the standard way to encode the keyUsage.

bq. There is a new DEROctetString(keyUsage) call in SelfSignedCertificate which is very similar to the one which is replaced in CertificateSigningRequest.
There are 4 "new DEROctetString" but only two of them are for keyUsage. This PR is to fix the encoding of the keyUsage. I don't see find issue of using DEROctetString for for constraints and subject alternate names extension.

Copy link
Contributor

@arp7 arp7 left a comment

Choose a reason for hiding this comment

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

LGTM

@xiaoyuyao xiaoyuyao merged commit 049793d into apache:master Oct 19, 2020
@xiaoyuyao
Copy link
Contributor Author

Thanks @elek and @arp7 for the review.

errose28 added a commit to errose28/ozone that referenced this pull request Oct 19, 2020
* master:
  HDDS-4301. SCM CA certificate does not encode KeyUsage extension properly (apache#1468)
  HDDS-4158. Provide a class type for Java based configuration (apache#1407)
  HDDS-4297. Allow multiple transactions per container to be sent for deletion by SCM.
  HDDS-2922. Balance ratis leader distribution in datanodes (apache#1371)
  HDDS-4269. Ozone DataNode thinks a volume is failed if an unexpected file is in the HDDS root directory. (apache#1490)
  HDDS-4327. Potential resource leakage using BatchOperation. (apache#1493)
  HDDS-3995. Fix s3g met NPE exception while write file by multiPartUpload (apache#1499)
  HDDS-4343. ReplicationManager.handleOverReplicatedContainer() does not handle unhealthyReplicas properly. (apache#1495)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants