[Pki-devel] [PATCH] 0072 Weaken PKIPrincipal to superclass in several places
Fraser Tweedale
ftweedal at redhat.com
Thu Jan 14 06:10:23 UTC 2016
Hi all,
The attached patch (part of the GSS-API effort) weakens several
soon-to-be-unsafe casts of the user principal object. It also adds
some commentary (in the form of TODOs) to replace hardcoded role
names with appropriate checks against authzManagers.
Thanks,
Fraser
-------------- next part --------------
From 368d81be5866739b4729952de2edeb4d5da14f43 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at redhat.com>
Date: Thu, 14 Jan 2016 16:13:26 +1100
Subject: [PATCH] Weaken PKIPrincipal to superclass in several places
In several places we are casting a `Principal' to `PKIPrincpal',
when `GenericPrincpal' or even no cast will suffice. In upcoming
external authentication support externally authenticated principals
will not be instances of `PKIPrincipal', so weaken assumptions about
type of the principal where possible.
Part of: https://fedorahosted.org/pki/ticket/1359
---
.../org/dogtagpki/server/ca/rest/CertService.java | 8 ++++---
.../dogtagpki/server/ca/rest/ProfileService.java | 28 +++++++++++++---------
.../org/dogtagpki/server/rest/AccountService.java | 5 +++-
.../com/netscape/cmscore/dbs/CSCfgDatabase.java | 9 +++----
.../server/tks/rest/TPSConnectorService.java | 4 ++--
5 files changed, 33 insertions(+), 21 deletions(-)
diff --git a/base/ca/src/org/dogtagpki/server/ca/rest/CertService.java b/base/ca/src/org/dogtagpki/server/ca/rest/CertService.java
index 440f756dee79904556f9f1671f2638cf22199e22..f219db63e4e1132f1fe166f5e753c650baa9344d 100644
--- a/base/ca/src/org/dogtagpki/server/ca/rest/CertService.java
+++ b/base/ca/src/org/dogtagpki/server/ca/rest/CertService.java
@@ -50,6 +50,7 @@ import netscape.security.x509.RevocationReason;
import netscape.security.x509.X509CertImpl;
import netscape.security.x509.X509Key;
+import org.apache.catalina.realm.GenericPrincipal;
import org.jboss.resteasy.plugins.providers.atom.Link;
import com.netscape.certsrv.apps.CMS;
@@ -75,7 +76,6 @@ import com.netscape.certsrv.dbs.certdb.ICertificateRepository;
import com.netscape.certsrv.logging.AuditFormat;
import com.netscape.certsrv.logging.ILogger;
import com.netscape.certsrv.request.IRequest;
-import com.netscape.cms.realm.PKIPrincipal;
import com.netscape.cms.servlet.base.PKIService;
import com.netscape.cms.servlet.cert.CertRequestDAO;
import com.netscape.cms.servlet.cert.FilterBuilder;
@@ -242,8 +242,10 @@ public class CertService extends PKIService implements CertResource {
processor.createCRLExtension();
- PKIPrincipal principal = (PKIPrincipal)servletRequest.getUserPrincipal();
- // TODO: do not hard-code role name
+ // TODO remove hardcoded role names and consult authzmgr
+ // (so that we can handle externally-authenticated principals)
+ GenericPrincipal principal =
+ (GenericPrincipal) servletRequest.getUserPrincipal();
String subjectDN = principal.hasRole("Certificate Manager Agents") ?
null : clientSubjectDN;
diff --git a/base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java b/base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java
index 488dd5ab960fd45fa3dad0dd83398b4317f2cb4f..28c50774148ec2e83bfaab15ed10adbb8dca0380 100644
--- a/base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java
+++ b/base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java
@@ -41,6 +41,7 @@ import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriBuilder;
import javax.ws.rs.core.UriInfo;
+import org.apache.catalina.realm.GenericPrincipal;
import org.apache.commons.lang.StringUtils;
import org.jboss.resteasy.plugins.providers.atom.Link;
@@ -76,7 +77,6 @@ import com.netscape.certsrv.profile.ProfileResource;
import com.netscape.certsrv.property.EPropertyException;
import com.netscape.certsrv.registry.IPluginInfo;
import com.netscape.certsrv.registry.IPluginRegistry;
-import com.netscape.cms.realm.PKIPrincipal;
import com.netscape.cms.servlet.base.PKIService;
import com.netscape.cms.servlet.profile.PolicyConstraintFactory;
import com.netscape.cms.servlet.profile.PolicyDefaultFactory;
@@ -124,11 +124,14 @@ public class ProfileService extends PKIService implements ProfileResource {
throw new PKIException("Error listing profiles. Profile Service not available");
}
- PKIPrincipal principal = (PKIPrincipal) servletRequest.getUserPrincipal();
- if ((principal != null) &&
- (principal.hasRole("Certificate Manager Agents") ||
- principal.hasRole("Certificate Manager Administrators"))) {
- visibleOnly = false;
+ // TODO remove hardcoded role names and consult authzmgr
+ // (so that we can handle externally-authenticated principals)
+ Principal principal = servletRequest.getUserPrincipal();
+ if (principal != null && principal instanceof GenericPrincipal) {
+ GenericPrincipal genPrincipal = (GenericPrincipal) principal;
+ if (genPrincipal.hasRole("Certificate Manager Agents") ||
+ genPrincipal.hasRole("Certificate Manager Administrators"))
+ visibleOnly = false;
}
Enumeration<String> e = ps.getProfileIds();
@@ -181,11 +184,14 @@ public class ProfileService extends PKIService implements ProfileResource {
throw new PKIException("Error retrieving profile. Profile Service not available");
}
- PKIPrincipal principal = (PKIPrincipal) servletRequest.getUserPrincipal();
- if ((principal != null) &&
- (principal.hasRole("Certificate Manager Agents") ||
- principal.hasRole("Certificate Manager Administrators"))) {
- visibleOnly = false;
+ // TODO remove hardcoded role names and consult authzmgr
+ // (so that we can handle externally-authenticated principals)
+ Principal principal = servletRequest.getUserPrincipal();
+ if (principal != null && principal instanceof GenericPrincipal) {
+ GenericPrincipal genPrincipal = (GenericPrincipal) principal;
+ if (genPrincipal.hasRole("Certificate Manager Agents") ||
+ genPrincipal.hasRole("Certificate Manager Administrators"))
+ visibleOnly = false;
}
IProfile profile;
diff --git a/base/server/cms/src/org/dogtagpki/server/rest/AccountService.java b/base/server/cms/src/org/dogtagpki/server/rest/AccountService.java
index 4e8e6e6f89fc065e2ad0c5fa616165de929142db..827e99e076585d0732bfde8ae795d6ae63648d5f 100644
--- a/base/server/cms/src/org/dogtagpki/server/rest/AccountService.java
+++ b/base/server/cms/src/org/dogtagpki/server/rest/AccountService.java
@@ -29,6 +29,7 @@ import javax.ws.rs.core.Request;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriInfo;
+import org.apache.catalina.realm.GenericPrincipal;
import org.apache.commons.lang.StringUtils;
import com.netscape.certsrv.account.AccountInfo;
@@ -75,8 +76,10 @@ public class AccountService extends PKIService implements AccountResource {
String email = user.getEmail();
if (!StringUtils.isEmpty(email)) response.setEmail(email);
+ }
- String[] roles = pkiPrincipal.getRoles();
+ if (principal instanceof GenericPrincipal) {
+ String[] roles = ((GenericPrincipal) principal).getRoles();
response.setRoles(Arrays.asList(roles));
}
diff --git a/base/server/cmscore/src/com/netscape/cmscore/dbs/CSCfgDatabase.java b/base/server/cmscore/src/com/netscape/cmscore/dbs/CSCfgDatabase.java
index 38f542ffb879ed8ed87f8673c810da05ebed9917..38b174859383adbef7c044955355009f08a36cb7 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/dbs/CSCfgDatabase.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/dbs/CSCfgDatabase.java
@@ -21,13 +21,13 @@ package com.netscape.cmscore.dbs;
import java.security.Principal;
import java.util.Arrays;
+import org.apache.catalina.realm.GenericPrincipal;
import org.apache.commons.lang.StringUtils;
import com.netscape.certsrv.apps.CMS;
import com.netscape.certsrv.base.EBaseException;
import com.netscape.certsrv.base.IConfigStore;
import com.netscape.certsrv.common.Constants;
-import com.netscape.cms.realm.PKIPrincipal;
/**
@@ -51,12 +51,13 @@ public class CSCfgDatabase<E extends CSCfgRecord> extends Database<E> {
}
public boolean canApprove(Principal principal) {
- if (!(principal instanceof PKIPrincipal)) {
+ if (!(principal instanceof GenericPrincipal)) {
return false;
}
- PKIPrincipal pkiPrincipal = (PKIPrincipal)principal;
- return pkiPrincipal.hasRole("TPS Agents");
+ // TODO remove hardcoded role name and consult authzmgr
+ // (so that we can handle externally-authenticated principals)
+ return ((GenericPrincipal) principal).hasRole("TPS Agents");
}
public String getRecordStatus(String recordID) throws EBaseException {
diff --git a/base/tks/src/org/dogtagpki/server/tks/rest/TPSConnectorService.java b/base/tks/src/org/dogtagpki/server/tks/rest/TPSConnectorService.java
index 93cd411cb6403ef68a9d9a0766cbaf6df30d40e1..bc655d6d0763ac9149a70e36703467b702da637d 100644
--- a/base/tks/src/org/dogtagpki/server/tks/rest/TPSConnectorService.java
+++ b/base/tks/src/org/dogtagpki/server/tks/rest/TPSConnectorService.java
@@ -5,6 +5,7 @@ import java.net.URI;
import java.security.InvalidAlgorithmParameterException;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
+import java.security.Principal;
import java.security.cert.X509Certificate;
import java.util.Arrays;
import java.util.Collection;
@@ -37,7 +38,6 @@ import com.netscape.certsrv.system.TPSConnectorResource;
import com.netscape.certsrv.tps.cert.TPSCertResource;
import com.netscape.certsrv.usrgrp.IUGSubsystem;
import com.netscape.certsrv.usrgrp.IUser;
-import com.netscape.cms.realm.PKIPrincipal;
import com.netscape.cms.servlet.base.PKIService;
import com.netscape.cmsutil.crypto.CryptoUtil;
import com.netscape.cmsutil.util.Utils;
@@ -326,7 +326,7 @@ public class TPSConnectorService extends PKIService implements TPSConnectorResou
throw new PKIException("Bad TPS connection configuration: userid not defined");
}
- PKIPrincipal principal = (PKIPrincipal) servletRequest.getUserPrincipal();
+ Principal principal = servletRequest.getUserPrincipal();
if (principal == null) {
throw new UnauthorizedException("User credentials not provided");
}
--
2.5.0
More information about the Pki-devel
mailing list