Siyao Meng created HDDS-16963:
---------------------------------
Summary: SCMSecurityProtocolServer.checkAndRotate does not call
checkAdminAccess
Key: HDDS-16963
URL: https://issues.apache.org/jira/browse/HDDS-16963
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: CR-6-require-scm-admin-for-secret-key-rotation.patch
h3. Mechanism
{{SCMSecurityProtocolServer.checkAndRotate}} serves the {{CheckAndRotate}}
request of the secret key protocol, the request behind {{ozone admin scm
rotate}} (HDDS-8659). With {{force}} set it replaces the current secret key.
The method only calls {{validateSecretKeyStatus}} (secret keys enabled and
initialized) before it calls {{SecretKeyManager.checkAndRotate}}.
The other administrative operation of the same class,
{{removeExpiredCertificates}}, starts with
{{storageContainerManager.checkAdminAccess(getRpcRemoteUser(), false)}}, and so
do the administrative operations of {{SCMClientProtocolServer}}.
{{checkAndRotate}} has no such call, so the SCM admin check is not applied to
this operation.
h3. Trigger
{{SCMSecurityProtocolServer.checkAndRotate}} does not call
{{StorageContainerManager.checkAdminAccess}} before it rotates.
h3. Impact
Forced secret key rotation is an administrative operation, but it is not
restricted to SCM administrators the way the other SCM admin operations are.
h3. Patch
[^CR-6-require-scm-admin-for-secret-key-rotation.patch], against
025266b52a6dcaf39508607cad736799c40907e8. It also applies to master at
4389803780f.
{{checkAndRotate}} now starts with
{{storageContainerManager.checkAdminAccess(getRpcRemoteUser(), false)}}, the
same call {{removeExpiredCertificates}} makes. A caller that is not an SCM
administrator gets "Access denied for user ... SCM superuser privilege is
required." The check admits the same identities as every other SCM admin
command. The periodic rotation by {{SecretKeyManagerService}} does not go
through this method and is not affected.
Covered by a new assertion in the existing
{{TestSecureOzoneCluster.testSecureScmAndOmStartupAndAccessControl}}, next to
the existing SCM admin checks: {{checkAndRotate(true)}} is denied for the non
admin test user and returns true for the SCM user. Without the change the new
assertion fails. With it {{TestSecureOzoneCluster}} (9 tests) and
{{TestBlockTokensCLI}} (3 tests, it runs {{ozone admin scm rotate}} with and
without the force option) pass and checkstyle is clean. This issue has no
standalone reproduction, the regression test in the patch covers the change.
Related: HDDS-16936 (open) adds the SCM admin check to another operation of the
same class. Both patches add a line at the same place in
{{testSecureScmAndOmStartupAndAccessControl}}, so the second to land needs a
trivial rebase.
Found by code review of the SCM secret key rotation and distribution paths, as
part of the TLA+ verification effort under HDDS-15926, on commit
025266b52a6dcaf39508607cad736799c40907e8. Checked against HDDS issues and
apache/ozone pull requests for duplicates before filing. The attached patch is
a proposal for review. Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
