Siyao Meng created HDDS-16964:
---------------------------------
Summary: 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
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]
