171 lines
8.4 KiB
Diff
171 lines
8.4 KiB
Diff
From 608e9bbe537aba314b124ceef70f9b606ab7e121 Mon Sep 17 00:00:00 2001
|
|
From: Fraser Tweedale <ftweedal@redhat.com>
|
|
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 <ftweedal@redhat.com>
|
|
Signed-off-by: Alexander Scheel <ascheel@redhat.com>
|
|
---
|
|
.../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
|
|
|