[Pki-devel] [PATCH] 649 Refactored SecurityDomainProcessor.
Endi Sukma Dewata
edewata at redhat.com
Tue Oct 13 15:55:25 UTC 2015
On 10/8/2015 4:34 PM, Ade Lee wrote:
> Looks like the only behavioral change is that the exception when a
> hostname cannot be resolved to an IP is thrown, rather than being
> swallowed up. So two issues:
>
> 1) you've taken what was an optional parameter (because the IP test is
> optional) and turned it into a required one. That could break some
> cases
>
> 2) when the breakage occurs, we do not know if its a client or server
> issue. So should this be a 400 or 500 error?
New patch attached.
Per IRC discussion the changes that added IP address validation have
been reverted. Further discussion is needed to see if the IP address
needs to be stored at all.
> On Tue, 2015-10-06 at 17:49 -0500, Endi Sukma Dewata wrote:
>> The SecurityDomainProcessor.getEnterpriseGroupName() has been
>> added to simplify ConfigurationUtils.getGroupName().
>>
>> The SecurityDomainProcessor.getInstallToken() has been modified
>> to validate the user role and the IP address, and to generate
>> safer session ID.
>>
>> https://fedorahosted.org/pki/ticket/1633
>> https://www.redhat.com/mailman/listinfo/pki-devel
--
Endi S. Dewata
-------------- next part --------------
From 131b9e2b1a17914f5f5f73d19fa1f012e44711d0 Mon Sep 17 00:00:00 2001
From: "Endi S. Dewata" <edewata at redhat.com>
Date: Fri, 11 Sep 2015 22:54:56 +0200
Subject: [PATCH] Refactored SecurityDomainProcessor.
The SecurityDomainProcessor.getEnterpriseGroupName() has been
added to simplify ConfigurationUtils.getGroupName().
The SecurityDomainProcessor.getInstallToken() has been modified
to validate the user role and to generate safer session ID.
https://fedorahosted.org/pki/ticket/1633
---
.../cms/servlet/csadmin/ConfigurationUtils.java | 18 -------
.../servlet/csadmin/SecurityDomainProcessor.java | 58 +++++++++++++---------
.../server/rest/SecurityDomainService.java | 16 ++++--
3 files changed, 47 insertions(+), 45 deletions(-)
diff --git a/base/server/cms/src/com/netscape/cms/servlet/csadmin/ConfigurationUtils.java b/base/server/cms/src/com/netscape/cms/servlet/csadmin/ConfigurationUtils.java
index 7b5bef567de773c0cd5b86ba6e36a1c16f444012..d3302949fe9145b2e3cab3f456a105537909fc66 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/csadmin/ConfigurationUtils.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/csadmin/ConfigurationUtils.java
@@ -441,24 +441,6 @@ public class ConfigurationUtils {
return null;
}
- public static String getGroupName(String uid, String subsystemname) {
- IUGSubsystem subsystem = (IUGSubsystem) (CMS.getSubsystem(IUGSubsystem.ID));
- if (subsystem.isMemberOf(uid, "Enterprise CA Administrators") && subsystemname.equals("CA")) {
- return "Enterprise CA Administrators";
- } else if (subsystem.isMemberOf(uid, "Enterprise KRA Administrators") && subsystemname.equals("KRA")) {
- return "Enterprise KRA Administrators";
- } else if (subsystem.isMemberOf(uid, "Enterprise OCSP Administrators") && subsystemname.equals("OCSP")) {
- return "Enterprise OCSP Administrators";
- } else if (subsystem.isMemberOf(uid, "Enterprise TKS Administrators") && subsystemname.equals("TKS")) {
- return "Enterprise TKS Administrators";
- } else if (subsystem.isMemberOf(uid, "Enterprise RA Administrators") && subsystemname.equals("RA")) {
- return "Enterprise RA Administrators";
- } else if (subsystem.isMemberOf(uid, "Enterprise TPS Administrators") && subsystemname.equals("TPS")) {
- return "Enterprise TPS Administrators";
- }
- return null;
- }
-
public static String getDomainXML(String hostname, int https_admin_port, boolean https)
throws IOException, SAXException, ParserConfigurationException {
CMS.debug("getDomainXML start");
diff --git a/base/server/cms/src/com/netscape/cms/servlet/csadmin/SecurityDomainProcessor.java b/base/server/cms/src/com/netscape/cms/servlet/csadmin/SecurityDomainProcessor.java
index 08b11c605ad3feb2efb30d3b754bf4dacc19a950..3a2b694e56c8fabe02c22a36fefb81e46f891654 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/csadmin/SecurityDomainProcessor.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/csadmin/SecurityDomainProcessor.java
@@ -31,13 +31,6 @@ import javax.xml.transform.TransformerFactory;
import javax.xml.transform.dom.DOMSource;
import javax.xml.transform.stream.StreamResult;
-import netscape.ldap.LDAPAttribute;
-import netscape.ldap.LDAPAttributeSet;
-import netscape.ldap.LDAPConnection;
-import netscape.ldap.LDAPEntry;
-import netscape.ldap.LDAPSearchConstraints;
-import netscape.ldap.LDAPSearchResults;
-
import org.w3c.dom.Document;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
@@ -55,9 +48,17 @@ import com.netscape.certsrv.system.DomainInfo;
import com.netscape.certsrv.system.InstallToken;
import com.netscape.certsrv.system.SecurityDomainHost;
import com.netscape.certsrv.system.SecurityDomainSubsystem;
+import com.netscape.certsrv.usrgrp.IUGSubsystem;
import com.netscape.cms.servlet.processors.CAProcessor;
import com.netscape.cmsutil.xml.XMLObject;
+import netscape.ldap.LDAPAttribute;
+import netscape.ldap.LDAPAttributeSet;
+import netscape.ldap.LDAPConnection;
+import netscape.ldap.LDAPEntry;
+import netscape.ldap.LDAPSearchConstraints;
+import netscape.ldap.LDAPSearchResults;
+
/**
* @author Endi S. Dewata
*/
@@ -74,47 +75,56 @@ public class SecurityDomainProcessor extends CAProcessor {
super("securitydomain", locale);
}
+ public static String getEnterpriseGroupName(String subsystemname) {
+ return "Enterprise " + subsystemname + " Administrators";
+ }
+
public InstallToken getInstallToken(
String user,
- String hostname,
- String subsystem) throws EBaseException {
+ String host,
+ String subsystem) throws Exception {
- String groupname = ConfigurationUtils.getGroupName(user, subsystem);
+ subsystem = subsystem.toUpperCase();
+ IUGSubsystem ugSubsystem = (IUGSubsystem) CMS.getSubsystem(IUGSubsystem.ID);
- if (groupname == null) {
+ String group = getEnterpriseGroupName(subsystem);
+ CMS.debug("SecurityDomainProcessor: group: " + group);
+
+ if (!ugSubsystem.isMemberOf(user, group)) {
String message = CMS.getLogMessage(
LOGGING_SIGNED_AUDIT_ROLE_ASSUME,
user,
ILogger.FAILURE,
- "Enterprise " + subsystem + " Administrators");
+ group);
audit(message);
- throw new UnauthorizedException("Access denied.");
+ throw new UnauthorizedException("User " + user + " is not a member of " + group + " group.");
}
String message = CMS.getLogMessage(
LOGGING_SIGNED_AUDIT_ROLE_ASSUME,
user,
ILogger.SUCCESS,
- groupname);
+ group);
audit(message);
String ip = "";
try {
- ip = InetAddress.getByName(hostname).getHostAddress();
+ ip = InetAddress.getByName(host).getHostAddress();
} catch (Exception e) {
- CMS.debug("Unable to determine IP address for "+hostname);
+ CMS.debug("Unable to determine IP address for " + host + ": " + e);
}
- // assign cookie
- Long num = random.nextLong();
- String cookie = num.toString();
+ // generate random session ID
+ // use positive number to avoid CLI issues
+ Long num = Math.abs(random.nextLong());
+ String sessionID = num.toString();
- String auditParams = "operation;;issue_token+token;;" + cookie + "+ip;;" + ip +
- "+uid;;" + user + "+groupname;;" + groupname;
+ String auditParams = "operation;;issue_token+token;;" + sessionID + "+ip;;" + ip +
+ "+uid;;" + user + "+groupname;;" + group;
ISecurityDomainSessionTable ctable = CMS.getSecurityDomainSessionTable();
- int status = ctable.addEntry(cookie, ip, user, groupname);
+ int status = ctable.addEntry(sessionID, ip, user, group);
if (status == ISecurityDomainSessionTable.SUCCESS) {
message = CMS.getLogMessage(
@@ -132,11 +142,11 @@ public class SecurityDomainProcessor extends CAProcessor {
auditParams);
audit(message);
- throw new PKIException("Failed to update security domain.");
+ throw new PKIException("Failed to create session.");
}
- return new InstallToken(cookie);
+ return new InstallToken(sessionID);
}
public DomainInfo getDomainInfo() throws EBaseException {
diff --git a/base/server/cms/src/org/dogtagpki/server/rest/SecurityDomainService.java b/base/server/cms/src/org/dogtagpki/server/rest/SecurityDomainService.java
index 23c439c7e4b58b5582dd67d3581898b4b23daabe..3d708ebb6de32235e9fbaaf8a6e8e87635c131ce 100644
--- a/base/server/cms/src/org/dogtagpki/server/rest/SecurityDomainService.java
+++ b/base/server/cms/src/org/dogtagpki/server/rest/SecurityDomainService.java
@@ -24,7 +24,7 @@ import javax.ws.rs.core.Request;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriInfo;
-import com.netscape.certsrv.base.EBaseException;
+import com.netscape.certsrv.apps.CMS;
import com.netscape.certsrv.base.PKIException;
import com.netscape.certsrv.system.DomainInfo;
import com.netscape.certsrv.system.InstallToken;
@@ -51,6 +51,7 @@ public class SecurityDomainService extends PKIService implements SecurityDomainR
@Override
public Response getInstallToken(String hostname, String subsystem) {
+ CMS.debug("SecurityDomainService.getInstallToken(" + hostname + ", " + subsystem + ")");
try {
// Get uid from realm authentication.
String user = servletRequest.getUserPrincipal().getName();
@@ -59,8 +60,12 @@ public class SecurityDomainService extends PKIService implements SecurityDomainR
InstallToken installToken = processor.getInstallToken(user, hostname, subsystem);
return createOKResponse(installToken);
+ } catch (PKIException e) {
+ CMS.debug("SecurityDomainService: " + e);
+ throw e;
- } catch (EBaseException e) {
+ } catch (Exception e) {
+ CMS.debug(e);
throw new PKIException(e.getMessage(), e);
}
}
@@ -72,7 +77,12 @@ public class SecurityDomainService extends PKIService implements SecurityDomainR
DomainInfo domainInfo = processor.getDomainInfo();
return createOKResponse(domainInfo);
- } catch (EBaseException e) {
+ } catch (PKIException e) {
+ CMS.debug("SecurityDomainService: " + e);
+ throw e;
+
+ } catch (Exception e) {
+ CMS.debug(e);
throw new PKIException(e.getMessage(), e);
}
}
--
2.4.3
More information about the Pki-devel
mailing list