[
https://issues.apache.org/jira/browse/HDDS-16963?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16963:
------------------------------
Attachment: CR-6-require-scm-admin-for-secret-key-rotation.patch
> 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
> Priority: Major
> 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]
