From 608e9bbe537aba314b124ceef70f9b606ab7e121 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Wed, 13 Jan 2021 18:27:46 +1100 Subject: [PATCH] Fix renewal profile approval process Due to a recent change in PKI CLI, the CLI now passes along user authentication with submissions to the renewal endpoint. Unlike the EE pages, the REST API has passed along this authentication for a while. Due to a bug in the RenewalProcessor, requests with credentials against profiles with no authentication method and no ACLs result in the certificiate automatically being approved. This occurs because, when an earlier commit (cb9eb967b5e24f5fde8bbf8ae87aa615b7033db7) modified the code to allow Light-Weight SubCAs to issue certificates, validation wasn't done on the passed principal, to see if it was a trusted agent. Because profiles requring Agent approval have an empty ACL list (as, no user should be able to submit a certificate request and have it automatically signed without agent approval), authorize allows any user to approve this request and thus accepts the AuthToken. Critical analysis: the RenewalProcessor code interprets (authToken != null) as evidence that the authenticated user is /authorized/ to immediately issue the certificate. This mismatch of concerns (authn vs authz) resulted in a misunderstanding of system behaviour. The "latent" AuthToken (from the HTTP request) was assigned to authToken without realising that authorization needed to be performed. We fix this by splitting the logic on whether the profile defines an authenticator. If so, we (re)authenticate and authorize the user according to the profile configuration. If the profile does not define an authenticator but there is a principal in the HTTP request, if (and only if) the user has permission to approve certificate requests *and* the requested renewal profile is caManualRenewal (which is hardcoded to be used for LWCA renewal), then we issue the certificate immediately. This special case ensures that LWCA renewal keeps working. Otherwise, if there is no principal in the HTTP request or the principal does not have permission to approve certificate requests, we leave the authToken unset. The resulting renewal request will be created with status PENDING, i.e. enqueued for agent review. Signed-off-by: Fraser Tweedale Signed-off-by: Alexander Scheel --- .../com/netscape/ca/CertificateAuthority.java | 10 +++ .../cms/servlet/cert/RenewalProcessor.java | 75 +++++++++++++++++-- 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/base/ca/src/com/netscape/ca/CertificateAuthority.java b/base/ca/src/com/netscape/ca/CertificateAuthority.java index 560507168a..431ce9ff78 100644 --- a/base/ca/src/com/netscape/ca/CertificateAuthority.java +++ b/base/ca/src/com/netscape/ca/CertificateAuthority.java @@ -1929,6 +1929,16 @@ public class CertificateAuthority } ProfileSubsystem ps = engine.getProfileSubsystem(); + /* NOTE: hard-coding the profile to use for Lightweight CA renewal + * might be OK, but caManualRenewal was not the right one to use. + * As a consequence, we have an undesirable special case in + * RenewalProcessor.processRenewal(). + * + * We should introduce a new profile specifically for LWCA renewal, + * with an authenticator and ACLs to match the authz requirements + * for the renewAuthority REST resource itself. Then we can use + * it here, and remove the workaround from RenewalProcessor. + */ Profile profile = ps.getProfile("caManualRenewal"); CertEnrollmentRequest req = CertEnrollmentRequestFactory.create( new ArgBlock(), profile, httpReq.getLocale()); diff --git a/base/ca/src/com/netscape/cms/servlet/cert/RenewalProcessor.java b/base/ca/src/com/netscape/cms/servlet/cert/RenewalProcessor.java index 4293cdd064..fd20f48267 100644 --- a/base/ca/src/com/netscape/cms/servlet/cert/RenewalProcessor.java +++ b/base/ca/src/com/netscape/cms/servlet/cert/RenewalProcessor.java @@ -32,6 +32,7 @@ import javax.servlet.http.HttpServletRequest; import org.apache.commons.lang3.StringUtils; import org.dogtagpki.server.ca.CAEngine; +import org.dogtagpki.server.authorization.AuthzToken; import org.mozilla.jss.netscape.security.x509.BasicConstraintsExtension; import org.mozilla.jss.netscape.security.x509.X509CertImpl; @@ -267,16 +268,78 @@ public class RenewalProcessor extends CertProcessor { // before creating the request, authenticate the request IAuthToken authToken = null; - Principal principal = request.getUserPrincipal(); - if (principal instanceof PKIPrincipal) - authToken = ((PKIPrincipal) principal).getAuthToken(); - if (authToken == null && authenticator != null) { - authToken = authenticate(request, origReq, authenticator, context, true, credentials); + + if (authenticator != null) { + /* The profile specifies an authenticator. Use it to + * authenticate the user. Ignore the "latent" session + * principal (if any). + */ + authToken = authenticate( + request, + origReq, + authenticator, + context, + true /* isRenewal */, + credentials); + } else { + /* When authenticator is null, we expect manual agent + * review (leave authToken as null). + * + * But as a special case to ensure Lightweight CA (LWCA) + * renewal works, if there is a latent user in the HTTP + * request, we use that user (i.e. set authToken to the + * principal's IAuthToken) if and only if: + * + * - The renewal profile is caManualRenewal (LWCA renewal + * is hardcoded to use this profile); AND + * + * - The latent user is authorized to "execute" + * certificate requests (i.e. agent approval) + * + * See also CertificateAuthority.renewAuthority(). + */ + + Principal principal = request.getUserPrincipal(); + if ( + renewProfileId.equals("caManualRenewal") + && principal instanceof PKIPrincipal + ) { + IAuthToken latentToken = ((PKIPrincipal) principal).getAuthToken(); + AuthzToken authzToken = authorize( + "DirAclAuthz", latentToken, "certServer.ca.certrequests", "execute"); + if (authzToken != null) { + // Success (no exception); user is authorized to approve + // cert requests. Set the authToken. + // + // NOTE: This authz does not replace or subsume the + // profile-specific authz check below. + authToken = latentToken; + } else { + // leave authToken as null to enqueue a pending request. + } + } else { + // not caManualRenewal or no latent principal; + // leave authToken as null to enqueue a pending request. + } } - // authentication success, now authorize + /* Authorize the request. + * + * If authToken != null, it will be checked against ACLs specified + * in the profile (if any). If ACLs are defined and authToken does + * not match, throws an authorization exception. + * + * If authToken == null, no check is performed (even if the profile + * defines ACLs). This is fine, because null authToken will cause + * the request status to be 'pending' [agent approval]. + */ authorize(profileId, renewProfile, authToken); + /* At this point, the request will be created. If authToken + * is non-null, then the certificate will be issued + * immediately. Otherwise the request will be pending. */ + + /////////////////////////////////////////////// // create and populate requests /////////////////////////////////////////////// -- 2.26.2