HDDS-7830. SCM API for OM and Datanode to get secret keys #4345

duongkame · 2023-03-03T18:19:52Z

What changes were proposed in this pull request?

SCM API to expose SecretKeys to OM and Datanodes. The APIs are implemented in the same SCMSecurityProtocol that is used by OM & Datanodes today to fetch Certificates. List of new APIs:

getCurrentSecretKey: used by OM to fetch the current key to sign block tokens.
getSecretKey: used by Datanodes to fetch a secret key with a given key id (encoded in the tokens sent to datanodes).
getAllSecretKeys: allow Datanodes to proactively prefetch secret keys from SCM.

These APIs will be used by OM and DN to sign and verify tokens. An early code review for that is here: duongkame#3.

What is the link to the Apache JIRA

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

How was this patch tested?

Integration test.

https://github.com/duongkame/ozone/actions/runs/4326226681

Galsza

Thanks @duongkame for working on this! I started running the new tests and after running them 30 times both testSecretKeyAfterSCMFailover and testSecretKeyApiNotEnabled failed at least once. Could they be flaky, or is this something that comes necessarily with MiniOzoneCluster tests?

Overall the change looks good, I have left 2 suggestions for white space changes.

...ds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMSecurityProtocolServer.java

Galsza · 2023-03-08T14:20:08Z

...ds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMSecurityProtocolServer.java

-      CertificateServer rootCertificateServer,
-      CertificateServer scmCertificateServer,
-      X509Certificate rootCACert, StorageContainerManager scm)
+                            CertificateServer rootCertificateServer,


Minor note and hint here: this is an unnecessary indentation change. You can disable it by editor/codestyle/java -> wrapping and braces tab and disabling "align when multiline" on method call arguments.

Thanks, that's helpful. :)

…m/server/SCMSecurityProtocolServer.java Co-authored-by: Galsza <109229906+Galsza@users.noreply.github.com>

kerneltime · 2023-03-09T07:10:38Z

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecretKeysApi.java

+    clusterId = UUID.randomUUID().toString();
+    scmId = UUID.randomUUID().toString();
+
+    startMiniKdc();


Is it possible to reuse one of the existing cluster spin + SCM tests? This will have some overhead on overall testing time and load. It could also be a robot test.

kerneltime · 2023-03-09T07:15:22Z

It would be good to capture the security model expected for which principals can make the API calls.

kerneltime · 2023-03-15T00:59:47Z

Merging this in since this is a dev branch, and we can refactor the code as needed.
This change looks.

duongkame · 2023-03-15T06:05:57Z

Thanks @duongkame for working on this! I started running the new tests and after running them 30 times both testSecretKeyAfterSCMFailover and testSecretKeyApiNotEnabled failed at least once. Could they be flaky, or is this something that comes necessarily with MiniOzoneCluster tests?

Overall the change looks good, I have left 2 suggestions for white space changes.

Thanks, @Galsza for the review. I could actually reproduce the failure after running the tests continuously multiple times. Think I found the problem with a short waiting time in testSecretKeySuccess that causes flakiness. Fixed.

duongkame · 2023-03-15T06:20:44Z

It would be good to capture the security model expected for which principals can make the API calls.

Thanks for the review @kerneltime. I created HDDS-8164 to address the authorization model for secret key APIs.

* master: (73 commits) HDDS-8587. Test that CertificateClient can store multiple rootCA certificates (apache#4852) HDDS-8801. ReplicationManager: Add metric to count how often replication is throttled (apache#4864) HDDS-8477. Unit test for Snapdiff using tombstone entries (apache#4678) HDDS-7507. [Snapshot] Implement List Snapshot API Pagination (apache#4065) (apache#4861) HDDS-8373. Document that setquota doesn't accept decimals (apache#4856) HDDS-8779. Recon - Expose flag for enable/disable of heatmap. (apache#4845) HDDS-8677. Ozone admin OM CLI command for block tokens (apache#4760) HDDS-8164. Authorize secret key APIs (apache#4597) HDDS-7945. Integrate secret keys to SCM snapshot (apache#4549) HDDS-8003. E2E integration test cases for block tokens (apache#4547) HDDS-7831. Use symmetric secret key to sign and verify token (apache#4417) HDDS-7830. SCM API for OM and Datanode to get secret keys (apache#4345) HDDS-7734. Implement symmetric SecretKeys lifescycle management in SCM (apache#4194) HDDS-8679. Add dedicated, configurable thread pool for OM gRPC server (apache#4771) HDDS-8790. Split EC acceptance tests (apache#4855) HDDS-8714. TestScmHAFinalization: mark testFinalizationWithRestart as flaky, enable other test cases HDDS-8787. Reduce ozone sh calls in robot tests (apache#4854) HDDS-8774. Log allocation stack trace for leaked CodecBuffer (apache#4840) HDDS-8729. Add metric for count of blocks pending deletion on datanode (apache#4800) HDDS-8780. Leak of ManagedChannel in HASecurityUtils (apache#4850) ...

* tmp-dir-refactor: (73 commits) HDDS-8587. Test that CertificateClient can store multiple rootCA certificates (apache#4852) HDDS-8801. ReplicationManager: Add metric to count how often replication is throttled (apache#4864) HDDS-8477. Unit test for Snapdiff using tombstone entries (apache#4678) HDDS-7507. [Snapshot] Implement List Snapshot API Pagination (apache#4065) (apache#4861) HDDS-8373. Document that setquota doesn't accept decimals (apache#4856) HDDS-8779. Recon - Expose flag for enable/disable of heatmap. (apache#4845) HDDS-8677. Ozone admin OM CLI command for block tokens (apache#4760) HDDS-8164. Authorize secret key APIs (apache#4597) HDDS-7945. Integrate secret keys to SCM snapshot (apache#4549) HDDS-8003. E2E integration test cases for block tokens (apache#4547) HDDS-7831. Use symmetric secret key to sign and verify token (apache#4417) HDDS-7830. SCM API for OM and Datanode to get secret keys (apache#4345) HDDS-7734. Implement symmetric SecretKeys lifescycle management in SCM (apache#4194) HDDS-8679. Add dedicated, configurable thread pool for OM gRPC server (apache#4771) HDDS-8790. Split EC acceptance tests (apache#4855) HDDS-8714. TestScmHAFinalization: mark testFinalizationWithRestart as flaky, enable other test cases HDDS-8787. Reduce ozone sh calls in robot tests (apache#4854) HDDS-8774. Log allocation stack trace for leaked CodecBuffer (apache#4840) HDDS-8729. Add metric for count of blocks pending deletion on datanode (apache#4800) HDDS-8780. Leak of ManagedChannel in HASecurityUtils (apache#4850) ...

duongkame added 3 commits March 3, 2023 09:45

HDDS-7830. SCM API for OM and Datanode to get secret keys

c71daaa

Cleanup.

1b27f14

Fix check-style.

c34d2d4

duongkame changed the title ~~Hdds 7830~~ HDDS-7830. SCM API for OM and Datanode to get secret keys Mar 3, 2023

duongkame requested review from kerneltime and fapifta March 3, 2023 18:20

Fix check-style.

15d8686

duongkame marked this pull request as ready for review March 3, 2023 18:29

Galsza reviewed Mar 8, 2023

View reviewed changes

duongkame and others added 3 commits March 8, 2023 09:20

Update hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/sc…

e2c045a

…m/server/SCMSecurityProtocolServer.java Co-authored-by: Galsza <109229906+Galsza@users.noreply.github.com>

Make integ test unflaky.

a4c1a27

revert unnecessary space changes.

0537c0c

kerneltime requested a review from ChenSammi March 9, 2023 06:53

kerneltime reviewed Mar 9, 2023

View reviewed changes

fix checkstyle.

940d989

kerneltime merged this pull request into apache:HDDS-7733-Symmetric-Tokens Mar 15, 2023

duongkame added a commit to duongkame/ozone that referenced this pull request Mar 16, 2023

HDDS-7830. SCM API for OM and Datanode to get secret keys (apache#4345)

3f6ef59

duongkame added a commit to duongkame/ozone that referenced this pull request May 9, 2023

HDDS-7830. SCM API for OM and Datanode to get secret keys (apache#4345)

42e2cb3

duongkame added a commit to duongkame/ozone that referenced this pull request Jun 8, 2023

HDDS-7830. SCM API for OM and Datanode to get secret keys (apache#4345)

e2709f8

duongkame added a commit that referenced this pull request Jun 8, 2023

HDDS-7830. SCM API for OM and Datanode to get secret keys (#4345)

aa24a63

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDDS-7830. SCM API for OM and Datanode to get secret keys #4345

HDDS-7830. SCM API for OM and Datanode to get secret keys #4345

duongkame commented Mar 3, 2023 •

edited

Loading

Galsza left a comment

Galsza Mar 8, 2023

duongkame Mar 9, 2023

kerneltime Mar 9, 2023

kerneltime commented Mar 9, 2023

kerneltime commented Mar 15, 2023

duongkame commented Mar 15, 2023

duongkame commented Mar 15, 2023

HDDS-7830. SCM API for OM and Datanode to get secret keys #4345

HDDS-7830. SCM API for OM and Datanode to get secret keys #4345

Conversation

duongkame commented Mar 3, 2023 • edited Loading

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Galsza left a comment

Choose a reason for hiding this comment

Galsza Mar 8, 2023

Choose a reason for hiding this comment

duongkame Mar 9, 2023

Choose a reason for hiding this comment

kerneltime Mar 9, 2023

Choose a reason for hiding this comment

kerneltime commented Mar 9, 2023

kerneltime commented Mar 15, 2023

duongkame commented Mar 15, 2023

duongkame commented Mar 15, 2023

duongkame commented Mar 3, 2023 •

edited

Loading