[ 
https://issues.apache.org/jira/browse/HDDS-16936?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng resolved HDDS-16936.
-------------------------------
    Fix Version/s: 2.3.0
       Resolution: Fixed

> SCM issues SCM role certificates without the SCM admin check and copies the 
> CA basic constraint for every role
> --------------------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-16936
>                 URL: https://issues.apache.org/jira/browse/HDDS-16936
>             Project: Apache Ozone
>          Issue Type: Bug
>            Reporter: Siyao Meng
>            Assignee: Siyao Meng
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 2.3.0
>
>         Attachments: CR-3-require-scm-admin-for-scm-role-certificates.patch
>
>
> h3. Mechanism
> {{SCMSecurityProtocolServer.getEncodedCertToString}} signs every CSR for the 
> node type the request names. For {{NodeType.SCM}} it uses the root CA server 
> when the SCM holds one and the sub CA server otherwise, and 
> {{DefaultCAServer.getDuration}} gives that role {{hdds.x509.max.duration}}; 
> the result is an SCM sub CA certificate, stored with role SCM.
> Two RPCs reach it with that role. 
> {{SCMSecurityProtocolServer.getSCMCertificate}} compares the cluster id of 
> the request with its own and calls {{ScmUtils.checkIfCertSignRequestAllowed}} 
> (the root CA rotation window). {{SCMSecurityProtocolServer.getCertificate}} 
> (the {{NodeDetailsProto}} form, added for Recon in HDDS-6212) forwards 
> {{nodeDetails.getNodeType()}} and checks nothing else. Neither consults 
> {{StorageContainerManager.checkAdminAccess}}; in this server only 
> {{removeExpiredCertificates}} does, while {{SCMBlockProtocolServer.addSCM}}, 
> the other operation an SCM peer performs when it joins, does.
> Separately, the sub CA server that issues OM, datanode and Recon certificates 
> is built with {{DefaultCAProfile}} ({{StorageContainerManager}}, HDDS-5052), 
> whose extension map accepts the basic constraints extension, and 
> {{DefaultApprover.sign}} copies every extension the profile lists from the 
> CSR into the certificate. So a basic constraints extension in a CSR is copied 
> into the certificate for every role, not only SCM. 
> {{DefaultCAServer.requestCertificate}} also completes its future 
> exceptionally when {{DefaultApprover.inspectCSR}} rejects the CSR and then 
> continues into {{signAndStoreCertificate}}, so a rejected CSR is still signed 
> and persisted.
> h3. Trigger
> # An SCM role signing request ({{GetSCMCertificate}}, or {{GetCertificate}} 
> with node type {{SCM}}) whose remote user is not in the SCM admin set.
> # A CSR for a role other than SCM that carries the basic constraints 
> extension, handled by a {{DefaultCAServer}} built with {{DefaultCAProfile}}.
> # A CSR that {{DefaultApprover.inspectCSR}} rejects.
> By history, not run on older code: {{getSCMCertificate}} and the 
> {{DefaultCAProfile}} sub CA arrived with HDDS-4861 and HDDS-5052 (1.2.0), the 
> {{NodeDetailsProto}} form with HDDS-6212 (1.3.0). The three core files are 
> byte identical between the pin and the 2.2.1 tag.
> h3. Impact
> * The SCM admin check that gates the other SCM peer operation ({{addSCM}}) 
> does not gate the issuance of SCM role certificates, which are CA 
> certificates under the cluster root with the maximum duration, and the CA 
> basic constraint is not limited to the SCM role.
> * A CSR the approver rejected is still persisted in the SCM certificate store 
> and returned by {{listCertificate}}.
> h3. Reproduction
> The regression tests in the attached patch cover it and fail on unmodified 
> source. There is no standalone reproduction.
> * {{TestDefaultCAServer.testBasicConstraintsAllowedForScmRoleOnly}} 
> ({{hadoop-hdds/framework}}): a basic constraints CSR for the OM role is 
> rejected and not stored, the same CSR for the SCM role is issued with the 
> constraint and stored, and a CSR the profile rejects is neither signed nor 
> stored ({{MockCAStore}} now records what is stored). Without the change it 
> fails with {{Expected java.util.concurrent.ExecutionException to be thrown, 
> but nothing was thrown}}.
> * 
> {{TestSecureOzoneCluster.assertScmSecurityProtocolRequiresAdminForScmRoleCertificate}}
>  ({{hadoop-ozone/integration-test}}, inside 
> {{testSecureScmAndOmStartupAndAccessControl}} next to the existing SCM admin 
> protocol checks): both SCM role signing RPCs answer {{Access denied}} for a 
> remote user outside the SCM admin set. Without the change it fails with 
> {{Expected java.io.IOException to be thrown, but nothing was thrown}}.
> h3. Patch
> [^CR-3-require-scm-admin-for-scm-role-certificates.patch], against 
> b9b33851b20fa11baf4b07982de9afb8ea44f85a. It also applies to master at 
> 084c60d4fb9.
> {{SCMSecurityProtocolServer.getEncodedCertToString}} calls 
> {{checkAdminAccess}} with the RPC remote user when the role is SCM, so both 
> RPCs are covered in one place. {{DefaultCAServer.requestCertificate}} rejects 
> a CSR that carries the basic constraints extension for any role other than 
> SCM before signing, and returns without signing when the approver rejects the 
> CSR. {{MockCAStore}} records stored certificates for the test.
> No wire or protocol change and no configuration change. A bootstrapping or 
> renewing SCM requests its certificate under its own login user 
> ({{SCMCertificateClient}}), and the SCM admin set always contains the starter 
> user plus {{ozone.administrators}}, the same membership {{addSCM}} already 
> requires of it, so no new deployment requirement. OM, datanode and Recon CSRs 
> never carry the basic constraints extension (only {{SCMCertificateClient}} 
> sets it), so the second change rejects no legitimate CSR. In a mixed version 
> cluster an older SCM against a patched leader passes the same check it 
> already passes for {{addSCM}}, and a patched SCM against an older primary 
> meets no new check. Outside RPC, with no remote user, the check is a no op, 
> as for {{removeExpiredCertificates}}; with {{ozone.authorization.enabled}} 
> false it returns early, as every SCM admin check does.
> With the patch {{TestDefaultCAServer}}, {{TestDefaultProfile}} 
> ({{hadoop-hdds/framework}}) and {{TestRootCARotationManager}} 
> ({{hadoop-hdds/server-scm}}) pass, the SCM part of 
> {{TestSecureOzoneCluster.testSecureScmAndOmStartupAndAccessControl}} passes, 
> and checkstyle on {{hadoop-hdds/framework}}, {{hadoop-hdds/server-scm}} and 
> {{hadoop-ozone/integration-test}} is clean. Not run: a secure multi SCM 
> bootstrap or certificate renewal with the check in place 
> ({{TestSecretKeysApi}} did not start on the test machine for a host name 
> resolution reason unrelated to the change), and the OM part of 
> {{testSecureScmAndOmStartupAndAccessControl}} times out on that machine with 
> and without the patch for the same reason. That bootstrap and renewal keep 
> working follows from the shared login user by code reading and should be 
> confirmed on CI before merge.
> Found by code review of the SCM root CA rotation paths, as part of the TLA+ 
> verification effort under HDDS-15926, on commit 
> b9b33851b20fa11baf4b07982de9afb8ea44f85a. 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 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]

Reply via email to