[
https://issues.apache.org/jira/browse/HDDS-16964?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16964:
------------------------------
Attachment: CR-12-check-caller-identity-for-scm-secret-key-reads.patch
> SCMSecurityProtocolServer serves the secret keys without checking the caller
> for a service identity
> ---------------------------------------------------------------------------------------------------
>
> Key: HDDS-16964
> URL: https://issues.apache.org/jira/browse/HDDS-16964
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments:
> CR-12-check-caller-identity-for-scm-secret-key-reads.patch
>
>
> h3. Mechanism
> {{SCMSecurityProtocolServer}} serves the secret key protocols
> {{SecretKeyProtocolScm}}, {{SecretKeyProtocolOm}} and
> {{SecretKeyProtocolDatanode}}, each declared with
> {{@KerberosInfo(clientPrincipal = ...)}} for the SCM, OM or datanode
> principal (HDDS-8164). The read operations {{getCurrentSecretKey}},
> {{getSecretKey}} and {{getAllSecretKeys}} return the key material that signs
> block tokens, container tokens and OM delegation tokens.
> Today the only check of the caller for these operations is the service level
> authorization of the RPC layer, which applies when
> {{hadoop.security.authorization}} is enabled. The operations themselves call
> {{validateSecretKeyStatus}} and nothing else, so unlike
> {{removeExpiredCertificates}} in the same class, which starts with
> {{checkAdminAccess}}, they do not check who the caller is.
> h3. Trigger
> {{SCMSecurityProtocolServer.getCurrentSecretKey}}, {{getSecretKey}} and
> {{getAllSecretKeys}} do not check the identity of the caller.
> h3. Impact
> The three operations do not themselves verify that the caller is one of the
> service identities their protocols are declared for.
> h3. Patch
> [^CR-12-check-caller-identity-for-scm-secret-key-reads.patch], against
> 025266b52a6dcaf39508607cad736799c40907e8. It also applies to master at
> 4389803780f.
> With {{hadoop.security.authorization}} enabled, the three read operations now
> check the caller before they serve a key: the caller must match the client
> principal of one of the three secret key protocols, resolved with
> {{SecurityUtil.getServerPrincipal}} against the caller's address the same way
> the service ACL resolves it. The check sits at the operation, next to
> {{validateSecretKeyStatus}}, the way the administrative operations of SCM
> call {{checkAdminAccess}}. A caller with another identity gets "Access denied
> for user ... SCM, OM or datanode service identity is required." Every caller
> the service ACL admits passes the check, so a configuration that works today
> keeps working. With {{hadoop.security.authorization}} disabled nothing
> changes, which is the behaviour
> {{TestSecretKeysApi.testSecretKeyWithoutAuthorization}} expects. There is no
> wire or protocol change.
> Covered by a new assertion in the existing
> {{TestSecretKeysApi.testSecretKeyApi}}: the operations of the running SCM,
> called with the test user as the caller, throw {{AccessControlException}}
> with "Access denied", and called with the SCM principal they return the keys.
> Without the change the assertion fails with "Expected
> org.apache.hadoop.security.AccessControlException to be thrown, but nothing
> was thrown." With it {{TestSecretKeysApi}} (4 tests, service authorization
> enabled, with the OM and datanodes of the mini cluster fetching their keys),
> {{TestSecureOzoneCluster}} (9 tests) and {{TestBlockTokensCLI}} (3 tests)
> pass, and checkstyle on {{hadoop-hdds/server-scm}} and
> {{hadoop-ozone/integration-test}} is clean. This issue has no standalone
> reproduction, the regression test in the patch covers the change.
> Related: the rotation operation of the same class, {{checkAndRotate}}, is
> covered by HDDS-16963, which adds the SCM admin check to it. The two patches
> change different methods of {{SCMSecurityProtocolServer}} and different test
> classes, and apply together in either order.
> 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. Related but not
> duplicates: HDDS-8164 (introduced the per protocol service ACLs for the
> secret key APIs), HDDS-16366 and HDDS-16369 (testing and documentation of the
> internal protocol ACLs). The attached patch is a proposal for review.
> Generated with Specula (Claude Opus 5.5, Claude Fable 5.1).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
