Blob Blame History Raw
From 95928ee1e268d242d8132c7bfefc60eb555afd9e Mon Sep 17 00:00:00 2001
From: "Endi S. Dewata" <edewata@redhat.com>
Date: Thu, 10 May 2018 02:19:01 +0200
Subject: [PATCH 01/11] Added CA signing cert validations

The configuration.py has been modified to validate the presence of
CA signing cert for existing/external CA installation.

The CertificateAuthority.getCACert() has been modified to validate
the content of ca.signing.cert property in CS.cfg.

https://pagure.io/dogtagpki/issue/2999

Change-Id: I56f5649b16ea98463bfa5e770b0c1dd7f00b7fcd
(cherry picked from commit 313c701957bedfd59f7f6368d0c37d2928d1a4a1)
---
 .../src/com/netscape/ca/CertificateAuthority.java  | 28 +++++++++++++---------
 .../server/deployment/scriptlets/configuration.py  | 16 ++++++++-----
 2 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/base/ca/src/com/netscape/ca/CertificateAuthority.java b/base/ca/src/com/netscape/ca/CertificateAuthority.java
index 9aaa9cb..90a8fba 100644
--- a/base/ca/src/com/netscape/ca/CertificateAuthority.java
+++ b/base/ca/src/com/netscape/ca/CertificateAuthority.java
@@ -52,6 +52,7 @@ import java.util.concurrent.CountDownLatch;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpSession;
 
+import org.apache.commons.lang.StringUtils;
 import org.dogtagpki.legacy.ca.CAPolicy;
 import org.dogtagpki.legacy.policy.IPolicyProcessor;
 import org.mozilla.jss.CryptoManager;
@@ -1603,25 +1604,30 @@ public class CertificateAuthority
     }
 
     public X509CertImpl getCACert() throws EBaseException {
+
         if (mCaCert != null) {
             return mCaCert;
         }
-        // during configuration
-        try {
-            String cert = mConfig.getString("signing.cert", null);
-            if (cert != null) {
-                return new X509CertImpl(Utils.base64decode(cert));
-            }
 
-        } catch (EBaseException e) {
-            CMS.debug(e);
-            throw e;
+        String cert = mConfig.getString("signing.cert");
+        CMS.debug("CertificateAuthority: CA signing cert: " + cert);
+
+        if (StringUtils.isEmpty(cert)) {
+            CMS.debug("CertificateAuthority: Missing CA signing certificate");
+            throw new EBaseException("Missing CA signing certificate");
+        }
+
+        byte[] bytes = Utils.base64decode(cert);
+        CMS.debug("CertificateAuthority: size: " + bytes.length + " bytes");
+
+        try {
+            return new X509CertImpl(bytes);
 
         } catch (CertificateException e) {
+            CMS.debug("Unable to parse CA signing cert: " + e.getMessage());
+            CMS.debug(e);
             throw new EBaseException(e);
         }
-
-        return null;
     }
 
     public org.mozilla.jss.crypto.X509Certificate getCaX509Cert() {
diff --git a/base/server/python/pki/server/deployment/scriptlets/configuration.py b/base/server/python/pki/server/deployment/scriptlets/configuration.py
index 2cda5e0..fd043a8 100644
--- a/base/server/python/pki/server/deployment/scriptlets/configuration.py
+++ b/base/server/python/pki/server/deployment/scriptlets/configuration.py
@@ -395,11 +395,15 @@ class PkiScriptlet(pkiscriptlet.AbstractBasePkiScriptlet):
             self.import_system_cert_request(deployer, subsystem, 'subsystem')
             self.import_system_cert_request(deployer, subsystem, 'sslserver')
 
-    def import_ca_signing_cert(self, deployer, nssdb):
+    def import_ca_signing_cert(self, deployer, nssdb, subsystem):
 
-        cert_file = deployer.mdict.get('pki_ca_signing_cert_path')
+        param = 'pki_ca_signing_cert_path'
+        cert_file = deployer.mdict.get(param)
         if not cert_file or not os.path.exists(cert_file):
-            return
+            if subsystem.name == 'ca':
+                raise Exception('Invalid certificate path: %s=%s' % (param, cert_file))
+            else:
+                return
 
         nickname = deployer.mdict['pki_ca_signing_nickname']
 
@@ -589,14 +593,14 @@ class PkiScriptlet(pkiscriptlet.AbstractBasePkiScriptlet):
     def import_system_certs(self, deployer, nssdb, subsystem):
 
         if subsystem.name == 'ca':
-            self.import_ca_signing_cert(deployer, nssdb)
+            self.import_ca_signing_cert(deployer, nssdb, subsystem)
             self.import_ca_ocsp_signing_cert(deployer, nssdb)
 
         if subsystem.name == 'kra':
             # Always import cert chain into internal token.
             internal_nssdb = subsystem.instance.open_nssdb()
             try:
-                self.import_ca_signing_cert(deployer, internal_nssdb)
+                self.import_ca_signing_cert(deployer, internal_nssdb, subsystem)
             finally:
                 internal_nssdb.close()
 
@@ -608,7 +612,7 @@ class PkiScriptlet(pkiscriptlet.AbstractBasePkiScriptlet):
             # Always import cert chain into internal token.
             internal_nssdb = subsystem.instance.open_nssdb()
             try:
-                self.import_ca_signing_cert(deployer, internal_nssdb)
+                self.import_ca_signing_cert(deployer, internal_nssdb, subsystem)
             finally:
                 internal_nssdb.close()
 
-- 
1.8.3.1


From 177a51b8f51c9beaf3dab6ba06174a07fdf9e3ca Mon Sep 17 00:00:00 2001
From: Christina Fu <cfu@redhat.com>
Date: Mon, 4 Jun 2018 10:53:12 -0700
Subject: [PATCH 04/11] Ticket 3028 CMC CRMF request results in
 InvalidKeyFormatException when signing algorithm is ECC

This patch fixes the issue where in case of CRMF request with ECC keys the
public key was encoded incorrectly previously.

The fix was done in a way that RSA portion is unaffected.

Fixes https://pagure.io/dogtagpki/issue/3028

Change-Id: I3eb62638f2970dc7a9df37abb19015bd287b383d
(cherry picked from commit f8da5db790777ab4c0bd8ab08c5d4932e2f25349)
---
 .../src/com/netscape/cms/authentication/CMCUserSignedAuth.java |  9 +++++++--
 base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java      | 10 +++++++++-
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/base/server/cms/src/com/netscape/cms/authentication/CMCUserSignedAuth.java b/base/server/cms/src/com/netscape/cms/authentication/CMCUserSignedAuth.java
index d92b33b..d5f6c34 100644
--- a/base/server/cms/src/com/netscape/cms/authentication/CMCUserSignedAuth.java
+++ b/base/server/cms/src/com/netscape/cms/authentication/CMCUserSignedAuth.java
@@ -697,15 +697,20 @@ public class CMCUserSignedAuth implements IAuthManager, IExtendedPluginInfo,
                                     SubjectPublicKeyInfo pkinfo = template.getPublicKey();
                                     PrivateKey.Type keyType = null;
                                     String alg = pkinfo.getAlgorithm();
-                                    BIT_STRING bitString = pkinfo.getSubjectPublicKey();
-                                    byte[] publicKeyData = bitString.getBits();
+                                    byte[] publicKeyData = null;
+
                                     if (alg.equals("RSA")) {
+                                        BIT_STRING bitString = pkinfo.getSubjectPublicKey();
+                                        publicKeyData = bitString.getBits();
                                         CMS.debug(method + "signing key alg=RSA");
                                         keyType = PrivateKey.RSA;
                                         selfsign_pubK = PK11PubKey.fromRaw(keyType, publicKeyData);
                                     } else if (alg.equals("EC")) {
                                         CMS.debug(method + "signing key alg=EC");
                                         keyType = PrivateKey.EC;
+                                        X509Key pubKey = CryptoUtil.getX509KeyFromCRMFMsg(crm);
+                                        CMS.debug(method + "got X509Key ");
+                                        publicKeyData = (pubKey).getEncoded();
                                         selfsign_pubK = PK11ECPublicKey.fromSPKI(/*keyType,*/ publicKeyData);
                                     } else {
                                         msg = "unsupported signature algorithm: " + alg;
diff --git a/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java b/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
index 0742f8e..7f8f11e 100644
--- a/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
+++ b/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
@@ -1398,7 +1398,13 @@ public class CryptoUtil {
             throw new IOException("invalid certificate requests");
         }
         CertReqMsg msg = (CertReqMsg) crmfMsgs.elementAt(0);
-        CertRequest certreq = msg.getCertReq();
+        return getX509KeyFromCRMFMsg(msg);
+    }
+
+    public static X509Key getX509KeyFromCRMFMsg(CertReqMsg crmfMsg)
+              throws IOException, NoSuchAlgorithmException,
+                  InvalidKeyException, InvalidKeyFormatException {
+        CertRequest certreq = crmfMsg.getCertReq();
         CertTemplate certTemplate = certreq.getCertTemplate();
         SubjectPublicKeyInfo spkinfo = certTemplate.getPublicKey();
         PublicKey pkey = spkinfo.toPublicKey();
@@ -1904,9 +1910,11 @@ public class CryptoUtil {
                      System.out.println(method + "extension found");
                      try {
                        if (jssOID.equals(SKIoid)) {
+                         System.out.println(method + "SKIoid == jssOID");
                          extn =
                              new SubjectKeyIdentifierExtension(false, jssext.getExtnValue().toByteArray());
                        } else {
+                         System.out.println(method + "SKIoid != jssOID");
                          extn =
                              new netscape.security.x509.Extension(csOID, false, jssext.getExtnValue().toByteArray());
                        }
-- 
1.8.3.1


From 8f695ca9808f9060072b38b7d9b5bc79a6df4ab5 Mon Sep 17 00:00:00 2001
From: Christina Fu <cfu@redhat.com>
Date: Mon, 4 Jun 2018 11:03:20 -0700
Subject: [PATCH 05/11] Ticket 3028 additional error checking

Change-Id: If660fabd21b9992416dd1d5463b6ffd68fa1bf43
(cherry picked from commit d7eca28b1d72804e1cfabeb6851aa393fafe39c7)
---
 base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java b/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
index 7f8f11e..d3036f3 100644
--- a/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
+++ b/base/util/src/com/netscape/cmsutil/crypto/CryptoUtil.java
@@ -1382,6 +1382,9 @@ public class CryptoUtil {
 
     public static SEQUENCE parseCRMFMsgs(byte cert_request[])
                throws IOException, InvalidBERException {
+        if (cert_request == null) {
+            throw new IOException("invalid certificate requests: cert_request null");
+        }
         ByteArrayInputStream crmfBlobIn =
                 new ByteArrayInputStream(cert_request);
         SEQUENCE crmfMsgs = (SEQUENCE)
@@ -1393,6 +1396,9 @@ public class CryptoUtil {
     public static X509Key getX509KeyFromCRMFMsgs(SEQUENCE crmfMsgs)
               throws IOException, NoSuchAlgorithmException,
                   InvalidKeyException, InvalidKeyFormatException {
+        if (crmfMsgs == null) {
+            throw new IOException("invalid certificate requests: crmfMsgs null");
+        }
         int nummsgs = crmfMsgs.size();
         if (nummsgs <= 0) {
             throw new IOException("invalid certificate requests");
-- 
1.8.3.1


From 63035adc06628b4ce2be20457e6c569186e1832f Mon Sep 17 00:00:00 2001
From: gkapoor <gkapoor@redhat.com>
Date: Tue, 29 May 2018 19:52:15 +0530
Subject: [PATCH 06/11] Fix for
 https://bugzilla.redhat.com/show_bug.cgi?id=1544843

Change-Id: Id8d45bfc804a9f26a1a475cb928cf184975a8f5f
Signed-off-by: gkapoor <gkapoor@redhat.com>
(cherry picked from commit b0f9a67f4ee61c5ca1f020b0a6accefceb9bbe0b)
(cherry picked from commit 0619c9e71cc0b98885739335f6c580f6b883fec2)
---
 base/common/python/pki/nssdb.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/base/common/python/pki/nssdb.py b/base/common/python/pki/nssdb.py
index 0f3c97a..f350255 100644
--- a/base/common/python/pki/nssdb.py
+++ b/base/common/python/pki/nssdb.py
@@ -322,6 +322,13 @@ class NSSDatabase(object):
             if key_type:
                 cmd.extend(['-k', key_type])
 
+            if key_type.lower() == 'ec':
+                # This is fix for Bugzilla 1544843
+                cmd.extend([
+                    '--keyOpFlagsOn', 'sign',
+                    '--keyOpFlagsOff', 'derive'
+                ])
+
             if key_size:
                 cmd.extend(['-g', str(key_size)])
 
-- 
1.8.3.1


From 3b6edbcfd86ac0ca407f59f784a6f99dc6259504 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal@redhat.com>
Date: Wed, 14 Mar 2018 22:26:34 +1100
Subject: [PATCH 07/11] Fix ACL evaluation in allow,deny mode

When `authz.evaluateOrder=allow,deny', ACL evaluation returns the
wrong result: matching allow rules deny access, and matching deny
rules allow access.

Fix the problem and improve type safety and readability by
introducing a couple of enums for ACLEntry.Type and EvaluationOrder.

CVE-2018-1080

Fixes: https://pagure.io/freeipa/issue/7453
Change-Id: Ic076ed4b90c305cda9da2c56ec90fc77b4dac039
(cherry picked from commit b917819285bd03f5979f053f7d2bd43a2bb88e95)
---
 .../src/com/netscape/certsrv/acls/ACLEntry.java    |  28 ++---
 .../com/netscape/cms/authorization/AAclAuthz.java  | 124 ++++++++-------------
 2 files changed, 60 insertions(+), 92 deletions(-)

diff --git a/base/common/src/com/netscape/certsrv/acls/ACLEntry.java b/base/common/src/com/netscape/certsrv/acls/ACLEntry.java
index 5cca230..cab3f68 100644
--- a/base/common/src/com/netscape/certsrv/acls/ACLEntry.java
+++ b/base/common/src/com/netscape/certsrv/acls/ACLEntry.java
@@ -33,9 +33,11 @@ public class ACLEntry implements IACLEntry, java.io.Serializable {
     */
     private static final long serialVersionUID = 422656406529200393L;
 
+    public enum Type { Allow , Deny };
+
     protected Hashtable<String, String> mPerms = new Hashtable<String, String>();
     protected String expressions = null;
-    protected boolean negative = false;
+    protected Type type = Type.Deny;
     protected String aclEntryString = null;
 
     /**
@@ -45,20 +47,12 @@ public class ACLEntry implements IACLEntry, java.io.Serializable {
     }
 
     /**
-     * Checks if this ACL entry is set to negative.
+     * Get the Type of the ACL entry.
      *
-     * @return true if this ACL entry expression is for "deny";
-     *         false if this ACL entry expression is for "allow"
-     */
-    public boolean isNegative() {
-        return negative;
-    }
-
-    /**
-     * Sets this ACL entry negative. This ACL entry expression is for "deny".
+     * @return Allow or Deny
      */
-    public void setNegative() {
-        negative = true;
+    public Type getType() {
+        return type;
     }
 
     /**
@@ -160,7 +154,7 @@ public class ACLEntry implements IACLEntry, java.io.Serializable {
         //           don't grant permission
         if (mPerms.get(permission) == null)
             return false;
-        if (isNegative()) {
+        if (type == Type.Deny) {
             return false;
         } else {
             return true;
@@ -195,9 +189,9 @@ public class ACLEntry implements IACLEntry, java.io.Serializable {
         ACLEntry entry = new ACLEntry();
 
         if (prefix.equals("allow")) {
-            // do nothing
+            entry.type = Type.Allow;
         } else if (prefix.equals("deny")) {
-            entry.setNegative();
+            entry.type = Type.Deny;
         } else {
             return null;
         }
@@ -230,7 +224,7 @@ public class ACLEntry implements IACLEntry, java.io.Serializable {
     public String toString() {
         StringBuffer entry = new StringBuffer();
 
-        if (isNegative()) {
+        if (type == Type.Deny) {
             entry.append("deny (");
         } else {
             entry.append("allow (");
diff --git a/base/server/cms/src/com/netscape/cms/authorization/AAclAuthz.java b/base/server/cms/src/com/netscape/cms/authorization/AAclAuthz.java
index 7b69ec4..2bef101 100644
--- a/base/server/cms/src/com/netscape/cms/authorization/AAclAuthz.java
+++ b/base/server/cms/src/com/netscape/cms/authorization/AAclAuthz.java
@@ -66,6 +66,8 @@ import com.netscape.cmsutil.util.Utils;
  */
 public abstract class AAclAuthz implements IAuthzManager {
 
+    public enum EvaluationOrder { DenyAllow , AllowDeny };
+
     protected static final String PROP_CLASS = "class";
     protected static final String PROP_IMPL = "impl";
     protected static final String PROP_EVAL = "accessEvaluator";
@@ -375,7 +377,7 @@ public abstract class AAclAuthz implements IAuthzManager {
                         log(ILogger.LL_SECURITY, " checkACLs(): permission denied");
                         throw new EACLsException(CMS.getUserMessage("CMS_ACL_PERMISSION_DENIED"));
                     }
-                } else if (!entry.isNegative()) {
+                } else if (entry.getType() == ACLEntry.Type.Allow) {
                     // didn't meet the access expression for "allow", failed
                     log(ILogger.LL_SECURITY, "checkACLs(): permission denied");
                     throw new EACLsException(CMS.getUserMessage("CMS_ACL_PERMISSION_DENIED"));
@@ -503,46 +505,18 @@ public abstract class AAclAuthz implements IAuthzManager {
 
         CMS.debug("AAclAuthz.checkPermission(" + name + ", " + perm + ")");
 
-        Vector<String> nodev = getNodes(name);
-        Enumeration<String> nodes = nodev.elements();
-        String order = getOrder();
-        Enumeration<ACLEntry> entries = null;
-
-        if (order.equals("deny")) {
-            entries = getDenyEntries(nodes, perm);
-        } else {
-            entries = getAllowEntries(nodes, perm);
-        }
-
-        while (entries.hasMoreElements()) {
-            ACLEntry entry = entries.nextElement();
-
-            CMS.debug("checkPermission(): expressions: " + entry.getAttributeExpressions());
-            if (evaluateExpressions(authToken, entry.getAttributeExpressions())) {
-                log(ILogger.LL_SECURITY, "checkPermission(): permission denied");
-                throw new EACLsException(CMS.getUserMessage("CMS_ACL_PERMISSION_DENIED"));
-            }
-        }
-
-        nodes = nodev.elements();
-        if (order.equals("deny")) {
-            entries = getAllowEntries(nodes, perm);
-        } else {
-            entries = getDenyEntries(nodes, perm);
-        }
+        Vector<String> nodes = getNodes(name);
+        EvaluationOrder order = getOrder();
 
         boolean permitted = false;
-
-        while (entries.hasMoreElements()) {
-            ACLEntry entry = entries.nextElement();
-
-            CMS.debug("checkPermission(): expressions: " + entry.getAttributeExpressions());
-            if (evaluateExpressions(authToken, entry.getAttributeExpressions())) {
-                permitted = true;
-            }
+        if (order == EvaluationOrder.DenyAllow) {
+            checkDenyEntries(authToken, nodes, perm);
+            permitted = checkAllowEntries(authToken, nodes, perm);
+        } else if (order == EvaluationOrder.AllowDeny) {
+            permitted = checkAllowEntries(authToken, nodes, perm);
+            checkDenyEntries(authToken, nodes, perm);
         }
 
-        nodev = null;
         if (!permitted) {
             String[] params = new String[2];
             params[0] = name;
@@ -560,54 +534,57 @@ public abstract class AAclAuthz implements IAuthzManager {
         log(ILogger.LL_INFO, infoMsg);
     }
 
-    protected Enumeration<ACLEntry> getAllowEntries(Enumeration<String> nodes, String operation) {
-        String name = "";
-        ACL acl = null;
-        Enumeration<ACLEntry> e = null;
-        Vector<ACLEntry> v = new Vector<ACLEntry>();
-
-        while (nodes.hasMoreElements()) {
-            name = nodes.nextElement();
-            acl = mACLs.get(name);
-            if (acl == null)
-                continue;
-            e = acl.entries();
-            while (e.hasMoreElements()) {
-                ACLEntry entry = e.nextElement();
-
-                if (!entry.isNegative() &&
-                        entry.containPermission(operation)) {
-                    v.addElement(entry);
-                }
+    protected boolean checkAllowEntries(
+            IAuthToken authToken,
+            Iterable<String> nodes,
+            String perm) {
+        for (ACLEntry entry : getEntries(ACLEntry.Type.Allow, nodes, perm)) {
+            CMS.debug("checkAllowEntries(): expressions: " + entry.getAttributeExpressions());
+            if (evaluateExpressions(authToken, entry.getAttributeExpressions())) {
+                return true;
             }
         }
+        return false;
+    }
 
-        return v.elements();
+    /** throw EACLsException if a deny entry is matched */
+    protected void checkDenyEntries(
+            IAuthToken authToken,
+            Iterable<String> nodes,
+            String perm)
+            throws EACLsException {
+        for (ACLEntry entry : getEntries(ACLEntry.Type.Deny, nodes, perm)) {
+            CMS.debug("checkDenyEntries(): expressions: " + entry.getAttributeExpressions());
+            if (evaluateExpressions(authToken, entry.getAttributeExpressions())) {
+                log(ILogger.LL_SECURITY, "checkPermission(): permission denied");
+                throw new EACLsException(CMS.getUserMessage("CMS_ACL_PERMISSION_DENIED"));
+            }
+        }
     }
 
-    protected Enumeration<ACLEntry> getDenyEntries(Enumeration<String> nodes, String operation) {
-        String name = "";
-        ACL acl = null;
-        Enumeration<ACLEntry> e = null;
+    protected Iterable<ACLEntry> getEntries(
+            ACLEntry.Type entryType,
+            Iterable<String> nodes,
+            String operation
+    ) {
         Vector<ACLEntry> v = new Vector<ACLEntry>();
 
-        while (nodes.hasMoreElements()) {
-            name = nodes.nextElement();
-            acl = mACLs.get(name);
+        for (String name : nodes) {
+            ACL acl = mACLs.get(name);
             if (acl == null)
                 continue;
-            e = acl.entries();
+            Enumeration<ACLEntry> e = acl.entries();
             while (e.hasMoreElements()) {
                 ACLEntry entry = e.nextElement();
 
-                if (entry.isNegative() &&
+                if (entry.getType() == entryType &&
                         entry.containPermission(operation)) {
                     v.addElement(entry);
                 }
             }
         }
 
-        return v.elements();
+        return v;
     }
 
     /**
@@ -897,19 +874,16 @@ public abstract class AAclAuthz implements IAuthzManager {
         }
     }
 
-    public String getOrder() {
-        IConfigStore mainConfig = CMS.getConfigStore();
-        String order = "";
-
+    public static EvaluationOrder getOrder() {
         try {
-            order = mainConfig.getString("authz.evaluateOrder", "");
+            String order = CMS.getConfigStore().getString("authz.evaluateOrder", "");
             if (order.startsWith("allow"))
-                return "allow";
+                return EvaluationOrder.AllowDeny;
             else
-                return "deny";
+                return EvaluationOrder.DenyAllow;
         } catch (Exception e) {
+            return EvaluationOrder.DenyAllow;
         }
-        return "deny";
     }
 
     public boolean evaluateACLs(IAuthToken authToken, String exp) {
-- 
1.8.3.1


From 71d74c61e841fbe82aac7293de28f4fc0ed05258 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal@redhat.com>
Date: Tue, 29 May 2018 15:07:30 +1000
Subject: [PATCH 08/11] Handle empty NameConstraints subtrees when reading
 extension

When reading stored NameConstraints extension data on a request, if
includedSubtrees or excludedSubtrees is empty, an exception is
thrown.  But these are valid cases, so do not thrown an exception.

Also perform some minor drive-by refactors and add the 'static'
qualifier to a few methods to improve readability.

Part of: https://pagure.io/dogtagpki/issue/2922

Change-Id: I925d8a64b96dd0f45b0548ceb11dbee4223cd64c
(cherry picked from commit adb1810ddbeb30014b9ad192118bbf7ee1efd595)
---
 .../netscape/cms/profile/def/EnrollDefault.java    |  7 +++---
 .../cms/profile/def/NameConstraintsExtDefault.java | 29 +++++++---------------
 2 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/base/server/cms/src/com/netscape/cms/profile/def/EnrollDefault.java b/base/server/cms/src/com/netscape/cms/profile/def/EnrollDefault.java
index 173ff83..f4ed98b 100644
--- a/base/server/cms/src/com/netscape/cms/profile/def/EnrollDefault.java
+++ b/base/server/cms/src/com/netscape/cms/profile/def/EnrollDefault.java
@@ -672,7 +672,7 @@ public abstract class EnrollDefault implements IPolicyDefault, ICertInfoPolicyDe
         return true;
     }
 
-    protected String buildRecords(Vector<NameValuePairs> recs) throws EPropertyException {
+    protected static String buildRecords(Vector<NameValuePairs> recs) {
         StringBuffer sb = new StringBuffer();
 
         for (int i = 0; i < recs.size(); i++) {
@@ -739,7 +739,7 @@ public abstract class EnrollDefault implements IPolicyDefault, ICertInfoPolicyDe
         return v;
     }
 
-    protected String getGeneralNameType(GeneralName gn)
+    protected static String getGeneralNameType(GeneralName gn)
             throws EPropertyException {
         int type = gn.getType();
 
@@ -763,7 +763,8 @@ public abstract class EnrollDefault implements IPolicyDefault, ICertInfoPolicyDe
         throw new EPropertyException("Unsupported type: " + type);
     }
 
-    protected String getGeneralNameValue(GeneralName gn) throws EPropertyException {
+    protected static String getGeneralNameValue(GeneralName gn)
+            throws EPropertyException {
         String s = gn.toString();
         int type = gn.getType();
 
diff --git a/base/server/cms/src/com/netscape/cms/profile/def/NameConstraintsExtDefault.java b/base/server/cms/src/com/netscape/cms/profile/def/NameConstraintsExtDefault.java
index a3d41b7..eb87d1c 100644
--- a/base/server/cms/src/com/netscape/cms/profile/def/NameConstraintsExtDefault.java
+++ b/base/server/cms/src/com/netscape/cms/profile/def/NameConstraintsExtDefault.java
@@ -478,12 +478,7 @@ public class NameConstraintsExtDefault extends EnrollExtDefault {
                 CMS.debug("NameConstraintExtDefault: getValue " + e.toString());
             }
 
-            if (subtrees == null) {
-                CMS.debug("NameConstraintsExtDefault::getValue() VAL_PERMITTED_SUBTREES is null!");
-                throw new EPropertyException("subtrees is null");
-            }
-
-            return getSubtreesInfo(ext, subtrees);
+            return getSubtreesInfo(subtrees);
         } else if (name.equals(VAL_EXCLUDED_SUBTREES)) {
             ext = (NameConstraintsExtension)
                     getExtension(PKIXExtensions.NameConstraints_Id.toString(), info);
@@ -500,28 +495,22 @@ public class NameConstraintsExtDefault extends EnrollExtDefault {
                 CMS.debug("NameConstraintExtDefault: getValue " + e.toString());
             }
 
-            if (subtrees == null) {
-                CMS.debug("NameConstraintsExtDefault::getValue() VAL_EXCLUDED_SUBTREES is null!");
-                throw new EPropertyException("subtrees is null");
-            }
-
-            return getSubtreesInfo(ext, subtrees);
+            return getSubtreesInfo(subtrees);
         } else {
             throw new EPropertyException(CMS.getUserMessage(
                         locale, "CMS_INVALID_PROPERTY", name));
         }
     }
 
-    private String getSubtreesInfo(NameConstraintsExtension ext,
-            GeneralSubtrees subtrees) throws EPropertyException {
-        Vector<GeneralSubtree> trees = subtrees.getSubtrees();
-        int size = trees.size();
-
-        Vector<NameValuePairs> recs = new Vector<NameValuePairs>();
+    private static String getSubtreesInfo(GeneralSubtrees subtrees)
+            throws EPropertyException {
+        if (subtrees == null)
+            return "";
 
-        for (int i = 0; i < size; i++) {
-            GeneralSubtree tree = trees.elementAt(i);
+        Vector<GeneralSubtree> trees = subtrees.getSubtrees();
+        Vector<NameValuePairs> recs = new Vector<>();
 
+        for (GeneralSubtree tree : trees) {
             GeneralName gn = tree.getGeneralName();
             String type = getGeneralNameType(gn);
             int max = tree.getMaxValue();
-- 
1.8.3.1


From a0cca30d5b42355e559d38cfe11b03bfb00c8b4c Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal@redhat.com>
Date: Tue, 29 May 2018 15:39:48 +1000
Subject: [PATCH 09/11] IPAddressName: fix toString method

IPAddressName.toString() is invoked when saving
NameConstraintDefault configurations.  Its implementation was wrong;
it produced bogus output for the netmasked variants used for
NameConstraints.  This resulted in issuance failures.  Update the
method to produce correct output for both netmasked and
non-netmasked addresses.

Fixes: https://pagure.io/dogtagpki/issue/2922
Change-Id: I3012565379961add5ac8286043f55c8e30520ddd
(cherry picked from commit a796f490b4c8aeea228195dacc3843cabe56b3ac)
---
 .../src/netscape/security/x509/IPAddressName.java  | 60 ++++++++++++++--------
 1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/base/util/src/netscape/security/x509/IPAddressName.java b/base/util/src/netscape/security/x509/IPAddressName.java
index 1c01f58..a343a5f 100644
--- a/base/util/src/netscape/security/x509/IPAddressName.java
+++ b/base/util/src/netscape/security/x509/IPAddressName.java
@@ -156,30 +156,46 @@ public class IPAddressName implements GeneralNameInterface {
      * Return a printable string of IPaddress
      */
     public String toString() {
-        if (address.length == 4) {
-            return ("IPAddress: " + (address[0] & 0xff) + "."
-                    + (address[1] & 0xff) + "."
-                    + (address[2] & 0xff) + "." + (address[3] & 0xff));
+        StringBuilder r = new StringBuilder("IPAddress: ");
+        ByteBuffer buf = ByteBuffer.wrap(address);
+        if (address.length == IPv4_LEN) {
+            writeIPv4(r, buf);
+        } else if (address.length == IPv4_LEN * 2) {
+            writeIPv4(r, buf);
+            r.append(",");
+            writeIPv4(r, buf);
+        } else if (address.length == IPv6_LEN) {
+            writeIPv6(r, buf);
+        } else if (address.length == IPv6_LEN * 2) {
+            writeIPv6(r, buf);
+            r.append(",");
+            writeIPv6(r, buf);
         } else {
-            StringBuffer r = new StringBuffer("IPAddress: " + Integer.toHexString(address[0] & 0xff));
-            String hexString = Integer.toHexString(address[1] & 0xff);
-            if (hexString.length() == 1) {
-                r.append("0" + hexString);
-            } else {
-                r.append(hexString);
-            }
-            for (int i = 2; i < address.length;) {
-                r.append(":" + Integer.toHexString(address[i] & 0xff));
-                hexString = Integer.toHexString(address[i + 1] & 0xff);
-                if (hexString.length() == 1) {
-                    r.append("0" + hexString);
-                } else {
-                    r.append(hexString);
-                }
-                i += 2;
-            }
-            return r.toString();
+            // shouldn't be possible
+            r.append("0.0.0.0");
         }
+        return r.toString();
+    }
+
+    private static void writeIPv4(StringBuilder r, ByteBuffer buf) {
+        for (int i = 0; i < 4; i++) {
+            if (i > 0) r.append(".");
+            r.append(buf.get() & 0xff);
+        }
+    }
+
+    private static void writeIPv6(StringBuilder r, ByteBuffer buf) {
+        for (int i = 0; i < 8; i++) {
+            if (i > 0) r.append(":");
+            r.append(Integer.toHexString(read16BitInt(buf)));
+        }
+    }
+
+    /**
+     * Read big-endian 16-bit int from buffer (advancing cursor)
+     */
+    private static int read16BitInt(ByteBuffer buf) {
+        return ((buf.get() & 0xff) << 8) + (buf.get() & 0xff);
     }
 
     /**
-- 
1.8.3.1


From 943bc3e77f630465906cfcd11812b917f06d1478 Mon Sep 17 00:00:00 2001
From: Christina Fu <cfu@redhat.com>
Date: Mon, 4 Jun 2018 16:47:57 -0700
Subject: [PATCH 10/11] Ticket 3033 CRMFPopClient tool - should allow option to
 do no key archival

This patch allows key transport cert file to not be specified, which would
then not include key archive option in the CRMF request.

fixes https://pagure.io/dogtagpki/issue/3033

Change-Id: Ib8c585c15057684aa049632d8eb67c2827d7e774
(cherry picked from commit 8cf6b5b2ac6da169f1c63341159faebc09580798)
---
 .../src/com/netscape/cmstools/CRMFPopClient.java   | 83 +++++++++++++---------
 1 file changed, 48 insertions(+), 35 deletions(-)

diff --git a/base/java-tools/src/com/netscape/cmstools/CRMFPopClient.java b/base/java-tools/src/com/netscape/cmstools/CRMFPopClient.java
index bc95983..747b7d6 100644
--- a/base/java-tools/src/com/netscape/cmstools/CRMFPopClient.java
+++ b/base/java-tools/src/com/netscape/cmstools/CRMFPopClient.java
@@ -309,7 +309,8 @@ public class CRMFPopClient {
         String subjectDN = cmd.getOptionValue("n");
         boolean encodingEnabled = Boolean.parseBoolean(cmd.getOptionValue("k", "false"));
 
-        String transportCertFilename = cmd.getOptionValue("b", "transport.txt");
+        // if transportCertFilename is not specified then assume no key archival
+        String transportCertFilename = cmd.getOptionValue("b");
 
         String popOption = cmd.getOptionValue("q", "POP_SUCCESS");
 
@@ -444,11 +445,18 @@ public class CRMFPopClient {
             CRMFPopClient client = new CRMFPopClient();
             client.setVerbose(verbose);
 
-            if (verbose) System.out.println("Loading transport certificate");
-            String encoded = new String(Files.readAllBytes(Paths.get(transportCertFilename)));
-            byte[] transportCertData = Cert.parseCertificate(encoded);
+            String encoded = null;
+            X509Certificate transportCert = null;
+            if (transportCertFilename != null) {
+                if (verbose) System.out.println("archival option enabled");
+                if (verbose) System.out.println("Loading transport certificate");
+                encoded = new String(Files.readAllBytes(Paths.get(transportCertFilename)));
+                byte[] transportCertData = Cert.parseCertificate(encoded);
+                transportCert = manager.importCACertPackage(transportCertData);
+            } else {
+                if (verbose) System.out.println("archival option not enabled");
+            }
 
-            X509Certificate transportCert = manager.importCACertPackage(transportCertData);
 
             if (verbose) System.out.println("Parsing subject DN");
             Name subject = client.createName(subjectDN, encodingEnabled);
@@ -478,7 +486,7 @@ public class CRMFPopClient {
             String kid = CryptoUtil.encodeKeyID(id);
             System.out.println("Keypair private key id: " + kid);
 
-            if (hostPort != null) {
+            if ((transportCert != null) && (hostPort != null)) {
                 // check the CA for the required key wrap algorithm
                 // if found, override whatever has been set by the command line
                 // options for the key wrap algorithm
@@ -492,8 +500,10 @@ public class CRMFPopClient {
                 kwAlg = getKeyWrapAlgotihm(pkiclient);
             }
 
-            if (verbose) System.out.println("Using key wrap algorithm: " + kwAlg);
-            keyWrapAlgorithm = KeyWrapAlgorithm.fromString(kwAlg);
+            if (verbose && (transportCert != null)) System.out.println("Using key wrap algorithm: " + kwAlg);
+            if (transportCert != null) {
+                keyWrapAlgorithm = KeyWrapAlgorithm.fromString(kwAlg);
+            }
 
             if (verbose) System.out.println("Creating certificate request");
             CertRequest certRequest = client.createCertRequest(
@@ -652,36 +662,39 @@ public class CRMFPopClient {
             KeyPair keyPair,
             Name subject,
             KeyWrapAlgorithm keyWrapAlgorithm) throws Exception {
-        byte[] iv = CryptoUtil.getNonceData(keyWrapAlgorithm.getBlockSize());
-        OBJECT_IDENTIFIER kwOID = CryptoUtil.getOID(keyWrapAlgorithm);
-
-        /* TODO(alee)
-         *
-         * HACK HACK!
-         * algorithms like AES KeyWrap do not require an IV, but we need to include one
-         * in the AlgorithmIdentifier above, or the creation and parsing of the
-         * PKIArchiveOptions options will fail.  So we include an IV in aid, but null it
-         * later to correctly encrypt the data
-         */
-        AlgorithmIdentifier aid = new AlgorithmIdentifier(kwOID, new OCTET_STRING(iv));
-
-        Class[] iv_classes = keyWrapAlgorithm.getParameterClasses();
-        if (iv_classes == null || iv_classes.length == 0)
-            iv = null;
-
-        WrappingParams params = getWrappingParams(keyWrapAlgorithm, iv);
-
-        PKIArchiveOptions opts = CryptoUtil.createPKIArchiveOptions(
-                token,
-                transportCert.getPublicKey(),
-                (PrivateKey) keyPair.getPrivate(),
-                params,
-                aid);
 
         CertTemplate certTemplate = createCertTemplate(subject, keyPair.getPublic());
-
         SEQUENCE seq = new SEQUENCE();
-        seq.addElement(new AVA(new OBJECT_IDENTIFIER("1.3.6.1.5.5.7.5.1.4"), opts));
+
+        if (transportCert != null) { // add key archive Option
+            byte[] iv = CryptoUtil.getNonceData(keyWrapAlgorithm.getBlockSize());
+            OBJECT_IDENTIFIER kwOID = CryptoUtil.getOID(keyWrapAlgorithm);
+
+            /* TODO(alee)
+             *
+             * HACK HACK!
+             * algorithms like AES KeyWrap do not require an IV, but we need to include one
+             * in the AlgorithmIdentifier above, or the creation and parsing of the
+             * PKIArchiveOptions options will fail.  So we include an IV in aid, but null it
+             * later to correctly encrypt the data
+             */
+            AlgorithmIdentifier aid = new AlgorithmIdentifier(kwOID, new OCTET_STRING(iv));
+
+            Class[] iv_classes = keyWrapAlgorithm.getParameterClasses();
+            if (iv_classes == null || iv_classes.length == 0)
+                iv = null;
+
+            WrappingParams params = getWrappingParams(keyWrapAlgorithm, iv);
+
+            PKIArchiveOptions opts = CryptoUtil.createPKIArchiveOptions(
+                    token,
+                    transportCert.getPublicKey(),
+                    (PrivateKey) keyPair.getPrivate(),
+                    params,
+                    aid);
+
+            seq.addElement(new AVA(new OBJECT_IDENTIFIER("1.3.6.1.5.5.7.5.1.4"), opts));
+        } // key archival option
 
         /*
         OCTET_STRING ostr = createIDPOPLinkWitness();
-- 
1.8.3.1


From 13f571329219958d573ba2194e58adce1239a14f Mon Sep 17 00:00:00 2001
From: Christina Fu <cfu@redhat.com>
Date: Wed, 6 Jun 2018 11:28:55 -0700
Subject: [PATCH 11/11] Bugzilla #1580527 CMCAuth Authorization for agents.

This patch adds proper authz entries to enrollment profiles using CMCAuth;
It also adds proper acl check inside ProfileSubmitCMCServlet for CMCAuth.

Fixes 2nd part of Bugzilla #1580527

Change-Id: I61fa1613f752c5bc203ab18d6a073eb7a13c966b
(cherry picked from commit 405b31bbbc8940354da22e2ab90215d8a19ff86e)
---
 base/ca/shared/profiles/ca/caECFullCMCUserCert.cfg |  1 +
 base/ca/shared/profiles/ca/caFullCMCUserCert.cfg   |  1 +
 .../servlet/profile/ProfileSubmitCMCServlet.java   | 23 ++++++++++++++++++----
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/base/ca/shared/profiles/ca/caECFullCMCUserCert.cfg b/base/ca/shared/profiles/ca/caECFullCMCUserCert.cfg
index 469dbb0..b24cb03 100644
--- a/base/ca/shared/profiles/ca/caECFullCMCUserCert.cfg
+++ b/base/ca/shared/profiles/ca/caECFullCMCUserCert.cfg
@@ -4,6 +4,7 @@ enableBy=admin
 name=Agent-Signed CMC-Authenticated User Certificate Enrollment
 visible=false
 auth.instance_id=CMCAuth
+authz.acl=group="Certificate Manager Agents"
 input.list=i1
 input.i1.class_id=cmcCertReqInputImpl
 output.list=o1
diff --git a/base/ca/shared/profiles/ca/caFullCMCUserCert.cfg b/base/ca/shared/profiles/ca/caFullCMCUserCert.cfg
index dd336ad..c15b002 100644
--- a/base/ca/shared/profiles/ca/caFullCMCUserCert.cfg
+++ b/base/ca/shared/profiles/ca/caFullCMCUserCert.cfg
@@ -4,6 +4,7 @@ enableBy=admin
 name=Agent-Signed CMC-Authenticated User Certificate Enrollment
 visible=false
 auth.instance_id=CMCAuth
+authz.acl=group="Certificate Manager Agents"
 input.list=i1
 input.i1.class_id=cmcCertReqInputImpl
 output.list=o1
diff --git a/base/server/cms/src/com/netscape/cms/servlet/profile/ProfileSubmitCMCServlet.java b/base/server/cms/src/com/netscape/cms/servlet/profile/ProfileSubmitCMCServlet.java
index a0bcfb5..7d75e31 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/profile/ProfileSubmitCMCServlet.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/profile/ProfileSubmitCMCServlet.java
@@ -438,10 +438,15 @@ public class ProfileSubmitCMCServlet extends ProfileServlet {
         context.put("sslClientCertProvider",
                 new SSLClientCertProvider(request));
         CMS.debug("ProfileSubmitCMCServlet: set sslClientCertProvider");
+
+        String auditSubjectID = auditSubjectID();
         if (authenticator != null) {
             try {
                 authToken = authenticate(authenticator, request);
                 // authentication success
+                if (authToken != null) {
+                    auditSubjectID = authToken.getInString(IAuthToken.USER_ID);
+                }
             } catch (EBaseException e) {
                 CMCOutputTemplate template = new CMCOutputTemplate();
                 SEQUENCE seq = new SEQUENCE();
@@ -468,6 +473,20 @@ public class ProfileSubmitCMCServlet extends ProfileServlet {
                 } catch (Exception e) {
                     CMS.debug("ProfileSubmitCMCServlet authorization failure: " + e.toString());
                 }
+
+                // CMCAuth should pair with additional authz check as it counts
+                // as pre-approved
+                String authMgrID = authenticator.getName();
+                if (authMgrID.equals("CMCAuth")) {
+                    authzToken = null; // reset authzToken
+                    CMS.debug("ProfileSubmitCMCServlet CMCAuth requires additional authz check");
+                    try {
+                        authzToken = authorize(mAclMethod, authToken,
+                                "certServer.ca.certrequests", "execute");
+                    } catch (Exception e) {
+                        CMS.debug("ProfileSubmitCMCServlet authorization failure: " + e.toString());
+                    }
+                }
             }
 
             if (authzToken == null) {
@@ -486,10 +505,6 @@ public class ProfileSubmitCMCServlet extends ProfileServlet {
             }
         }
 
-        String auditSubjectID = auditSubjectID();
-        if (authToken != null) {
-            auditSubjectID = authToken.getInString(IAuthToken.USER_ID);
-        }
         String auditMessage = CMS.getLogMessage(
                 AuditEvent.CMC_REQUEST_RECEIVED,
                 auditSubjectID,
-- 
1.8.3.1