Re: [Pki-devel] [PATCH] 303-306 Various issues

2016-05-24 Thread Ade Lee 
Patches 303, 305 and 306 have been modified as discussed and checked
in.

Patch 304 has been revised as discussed on IRC.  Please review.

Ade

On Fri, 2016-05-20 at 17:00 -0500, Endi Sukma Dewata wrote:
> On 5/20/2016 2:20 PM, Ade Lee wrote:
> > Please review:
> > 
> > Patches listed in reverse order (306 -> 303)
> > 
> > Ade
> 
> Some comments/questions:
> 
> Patch #303:
> 1. Instead of using underscores (i.e. ca.publishing.cert_enable and 
> ca.publishing.crl_enable) it would be more consistent to use dots
> (i.e. 
> ca.publishing.cert.enable and ca.publishing.crl.enable) in the
> parameter 
> names.
> 
> 2. The PublisherProcessor.isCertPublishingEnabled() and 
> isCRLPublishingEnabled() currently swallow the exception thrown by 
> getBoolean() and interpret it as disabled. I think since "this should
> never happen" the exception should (if not too much additional work)
> be 
> allowed to bubble up.
> 
> Patch #304:
> 1. I think the default maxAge and maxFiles should not be unlimited 
> because most people probably will use the default values until
> something 
> goes wrong (e.g. disk full), and we want to avoid that. It would be 
> better to pick something reasonable, for example 1 year and 100
> files, 
> respectively.
> 
> 2. Currently the unit for maxAge is hour. How long do people usually 
> want to retain old files? Should we use day instead?
> 
> 3. In purgeExcessFiles() the files are sorted by last modified 
> timestamp. It's kind of risky since someone could accidentally do 
> something that updates the timestamp, then code will be deleting a
> file 
> that's not supposed to be purged yet. Can the files be sorted by
> their 
> names? In Tomcat the log files can be sorted by their names since
> they 
> contain a timestamp or sequence number.
> 
> 4. Also in purgeExcessFiles() the last loop calls dir.listFiles() in 
> each iteration. For efficiency it might be better to use a counter
> since 
> the number of excess files can be computed before the loop.
> 
> 5. The exception thrown by getInteger() should not be swallowed
> either. 
> If there's a configuration problem we want to know that.
> 
> Patch #305:
> 1. It might be better to check for invalid revocation reason in the 
> RevocationReason.valueOf() itself so any code using it is guaranteed
> to 
> get a valid value.
> 
> Patch #306 will follow later.
> From 63f6047c3e535eb336689082101ca60e61a67f29 Mon Sep 17 00:00:00 2001
From: Ade Lee 
Date: Thu, 19 May 2016 00:08:20 -0400
Subject: [PATCH] Add parameters to purge old published files

Ticket 2254
---
 base/server/cms/src/CMakeLists.txt |   9 +-
 .../cms/publish/publishers/FileBasedPublisher.java | 151 +++--
 2 files changed, 150 insertions(+), 10 deletions(-)

diff --git a/base/server/cms/src/CMakeLists.txt b/base/server/cms/src/CMakeLists.txt
index d414d2859ba2512d1184b973a54e5807ff0c92fe..33b1cd3baf8d321c7f1a2f50e5f3e8360c515695 100644
--- a/base/server/cms/src/CMakeLists.txt
+++ b/base/server/cms/src/CMakeLists.txt
@@ -30,6 +30,13 @@ find_file(COMMONS_HTTPCLIENT_JAR
 /usr/share/java
 )
 
+find_file(COMMONS_IO_JAR
+NAMES
+commons-io.jar
+PATHS
+/usr/share/java
+)
+
 find_file(COMMONS_LANG_JAR
 NAMES
 commons-lang.jar
@@ -124,7 +131,7 @@ javac(pki-cms-classes
 com/netscape/cms/*.java
 org/dogtagpki/server/*.java
 CLASSPATH
-${COMMONS_CODEC_JAR} ${COMMONS_LANG_JAR} ${COMMONS_HTTPCLIENT_JAR}
+${COMMONS_CODEC_JAR} ${COMMONS_IO_JAR} ${COMMONS_LANG_JAR} ${COMMONS_HTTPCLIENT_JAR}
 ${HTTPCLIENT_JAR} ${HTTPCORE_JAR}
 ${XALAN_JAR} ${XERCES_JAR}
 ${JSS_JAR} ${SYMKEY_JAR}
diff --git a/base/server/cms/src/com/netscape/cms/publish/publishers/FileBasedPublisher.java b/base/server/cms/src/com/netscape/cms/publish/publishers/FileBasedPublisher.java
index c48aa2db44163850d34f99e146ba6505926d2389..f3fa7eceed33697823baf89f7dcc751e0b43494f 100644
--- a/base/server/cms/src/com/netscape/cms/publish/publishers/FileBasedPublisher.java
+++ b/base/server/cms/src/com/netscape/cms/publish/publishers/FileBasedPublisher.java
@@ -19,6 +19,7 @@ package com.netscape.cms.publish.publishers;
 
 import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.io.FileFilter;
 import java.io.FileOutputStream;
 import java.io.FilterOutputStream;
 import java.io.IOException;
@@ -28,14 +29,17 @@ import java.security.cert.CRLException;
 import java.security.cert.CertificateEncodingException;
 import java.security.cert.X509CRL;
 import java.security.cert.X509Certificate;
+import java.text.ParseException;
+import java.util.Arrays;
 import java.util.Locale;
 import java.util.TimeZone;
 import java.util.Vector;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipOutputStream;
 
-import netscape.ldap.LDAPConnection;
-
+import org.apache.commons.io.filefilter.RegexFileFilter;
 import org.mozilla.jss.util.Base64OutputStream;

[Pki-devel] [PATCH] pki-cfu-0123-Ticket-1665-Cert-Revocation-Reasons-not-being-update.patch

2016-05-24 Thread Christina Fu 
https://fedorahosted.org/pki/ticket/1665 Certificate Revocation Reasons 
not being updated in some cases


Ticket 1665 - Cert Revocation
Reasons not being updated when on-hold
This patch fixes the following areas:
* In the CA, when revokeCert is called, make it possible to move 
from on_hold

to revoke.
* In the servlet that handles TPS revoke (DoRevokeTPS), make sure 
it allows

the on_hold cert to be put in the bucket to be revoked.
* there are a few minor fixes such as typos and one have to do with the
populate method in SubjectDNInput.java needs better handling of 
subject in

case it's null.
Note: This patch does not make attempt to allow agents to revoke 
certs that
are on_hold from agent interface.  The search filter needs to be 
modified to

allow that.

thanks,
Christina
>From b3e34f912c7aa5a695710fdbf9ff8faf5f5a47af Mon Sep 17 00:00:00 2001
From: Christina Fu 
Date: Mon, 23 May 2016 16:22:54 -0700
Subject: [PATCH] Ticket 1665 - Cert Revocation Reasons not being updated when
 on-hold This patch fixes the following areas: * In the CA, when revokeCert is
 called, make it possible to move from on_hold to revoke. * In the servlet
 that handles TPS revoke (DoRevokeTPS), make sure it allows the on_hold cert
 to be put in the bucket to be revoked. * there are a few minor fixes such as
 typos and one have to do with the populate method in SubjectDNInput.java
 needs better handling of subject in case it's null. Note: This patch does not
 make attempt to allow agents to revoke certs that are on_hold from agent
 interface.  The search filter needs to be modified to allow that.

---
 base/ca/src/com/netscape/ca/CAService.java | 17 --
 .../netscape/certsrv/dbs/certdb/ICertRecord.java   |  5 +++
 .../certsrv/dbs/certdb/ICertificateRepository.java |  3 ++
 .../netscape/cms/profile/common/EnrollProfile.java |  4 +--
 .../netscape/cms/profile/input/SubjectDNInput.java |  2 +-
 .../com/netscape/cms/servlet/cert/DoRevokeTPS.java | 12 ---
 .../src/com/netscape/cmscore/dbs/CertRecord.java   | 28 
 .../cmscore/dbs/CertificateRepository.java | 38 ++
 .../src/org/dogtagpki/server/tps/TPSTokendb.java   |  2 ++
 9 files changed, 95 insertions(+), 16 deletions(-)

diff --git a/base/ca/src/com/netscape/ca/CAService.java b/base/ca/src/com/netscape/ca/CAService.java
index 2b5d5f732b36f5cf6e1e5aa54552a5e4dcc5afaa..7fdce06136ba5cd27e65b48aeb3ed3e3cb3566e8 100644
--- a/base/ca/src/com/netscape/ca/CAService.java
+++ b/base/ca/src/com/netscape/ca/CAService.java
@@ -1001,9 +1001,11 @@ public class CAService implements ICAService, IService {
 Date revdate = crlentry.getRevocationDate();
 CRLExtensions crlentryexts = crlentry.getExtensions();
 
+CMS.debug("CAService.revokeCert: revokeCert begins");
 CertRecord certRec = (CertRecord) mCA.getCertificateRepository().readCertificateRecord(serialno);
 
 if (certRec == null) {
+CMS.debug("CAService.revokeCert: cert record not found");
 mCA.log(ILogger.LL_FAILURE, CMS.getLogMessage("CMSCORE_CA_CERT_NOT_FOUND", serialno.toString(16)));
 throw new ECAException(
 CMS.getUserMessage("CMS_CA_CANT_FIND_CERT_SERIAL",
@@ -1013,14 +1015,24 @@ public class CAService implements ICAService, IService {
 // allow revoking certs that are on hold.
 String certStatus = certRec.getStatus();
 
-if (certStatus.equals(ICertRecord.STATUS_REVOKED) ||
+if ((certStatus.equals(ICertRecord.STATUS_REVOKED) &&
+!certRec.isCertOnHold()) ||
 certStatus.equals(ICertRecord.STATUS_REVOKED_EXPIRED)) {
+CMS.debug("CAService.revokeCert: cert already revoked:" +
+serialno.toString());
 throw new ECAException(CMS.getUserMessage("CMS_CA_CERT_ALREADY_REVOKED",
 "0x" + Long.toHexString(serialno.longValue(;
 }
 try {
-mCA.getCertificateRepository().markAsRevoked(serialno,
+CMS.debug("CAService.revokeCert: about to call markAsRevoked");
+if (certRec.isCertOnHold()) {
+mCA.getCertificateRepository().markAsRevoked(serialno,
+new RevocationInfo(revdate, crlentryexts), true /*isOnHold*/);
+} else {
+mCA.getCertificateRepository().markAsRevoked(serialno,
 new RevocationInfo(revdate, crlentryexts));
+}
+CMS.debug("CAService.revokeCert: cert revoked");
 mCA.log(ILogger.LL_INFO, CMS.getLogMessage("CMSCORE_CA_CERT_REVOKED",
 serialno.toString(16)));
 // inform all CRLIssuingPoints about revoked certificate
@@ -1052,6 +1064,7 @@ public class CAService implements ICAService, IService {
 }
 }
 } catch (EBaseException e) {
+CMS.debug("CAService.revokeCert: " + e.toString());

Re: [Pki-devel] [PATCH] 750 Fixed cert enrollment problem with empty rangeUnit in profile.

2016-05-24 Thread Endi Sukma Dewata 

On 5/19/2016 10:27 AM, Endi Sukma Dewata wrote:

Previously cert enrollment might fail after editing the profile
using the console. This is because the console added an empty
rangeUnit parameter, but the server rejected the empty value.

The convertRangeUnit() methods in several classes have been
modified to accept the empty value and convert it into the
default value (i.e. day).

https://fedorahosted.org/pki/ticket/2308


ACKed by alee (thanks!). Pushed to master.

--
Endi S. Dewata

___
Pki-devel mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel

Re: [Pki-devel] [PATCH] 749 Fixed support for generic CSR extensions.

2016-05-24 Thread Endi Sukma Dewata 

On 5/18/2016 10:44 AM, Endi Sukma Dewata wrote:

The deployment tool has been modified to support adding Subordinate
CA extension into the CSR for Microsoft CA, and also adding generic
extensions to any system certificate.

https://fedorahosted.org/pki/ticket/2312


ACKed by alee (thanks!). Pushed to master.

--
Endi S. Dewata

___
Pki-devel mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel

[Pki-devel] [PATCH] 756 Updated system certificate selftests.

2016-05-24 Thread Endi Sukma Dewata 

The CertUtils.verifySystemCertByNickname() has been modified to call
CryptoManager.verifyCertificate() to validate the system certificates
which will provide better information (i.e. NSS error message and
stack trace) to troubleshoot validation issues.

https://fedorahosted.org/pki/ticket/850

--
Endi S. Dewata
>From 197743bdabeee8890eb22005a799e80d51cc66f0 Mon Sep 17 00:00:00 2001
From: "Endi S. Dewata" 
Date: Tue, 8 Dec 2015 21:47:58 +0100
Subject: [PATCH] Updated system certificate selftests.

The CertUtils.verifySystemCertByNickname() has been modified to call
CryptoManager.verifyCertificate() to validate the system certificates
which will provide better information (i.e. NSS error message and
stack trace) to troubleshoot validation issues.

https://fedorahosted.org/pki/ticket/850
---
 .../cmscore/src/com/netscape/cmscore/cert/CertUtils.java | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/base/server/cmscore/src/com/netscape/cmscore/cert/CertUtils.java b/base/server/cmscore/src/com/netscape/cmscore/cert/CertUtils.java
index d780cba7375280e1b490415173f9aa00f62a557d..5b6382e00ac7d735dca17e0ff89efa8077eed875 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/cert/CertUtils.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/cert/CertUtils.java
@@ -834,18 +834,18 @@ public class CertUtils {
 if (certusage == null || certusage.equals(""))
 CMS.debug("CertUtils: verifySystemCertByNickname(): required certusage not defined, getting current certusage");
 
-CMS.debug("CertUtils: verifySystemCertByNickname(): calling isCertValid()");
 try {
 CryptoManager cm = CryptoManager.getInstance();
 if (cu.getUsage() != CryptoManager.CertificateUsage.CheckAllUsages.getUsage()) {
-if (cm.isCertValid(nickname, true, cu)) {
-CMS.debug("CertUtils: verifySystemCertByNickname() passed: " + nickname);
-} else {
-CMS.debug("CertUtils: verifySystemCertByNickname() failed: " + nickname);
-throw new Exception("Invalid certificate " + nickname);
+CMS.debug("CertUtils: verifySystemCertByNickname(): calling verifyCertificate(" + nickname + ", true, " + cu + ")");
+try {
+cm.verifyCertificate(nickname, true, cu);
+} catch (CertificateException e) {
+throw new Exception("Certificate " + nickname + " is invalid: " + e.getMessage(), e);
 }
 
 } else {
+CMS.debug("CertUtils: verifySystemCertByNickname(): calling isCertValid(" + nickname + ", true)");
 // find out about current cert usage
 ccu = cm.isCertValid(nickname, true);
 if (ccu == CertificateUsage.basicCertificateUsages) {
-- 
2.4.11

___
Pki-devel mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel

[Pki-devel] [PATCH] 754-755 Fixed problem submitting renewal request.

2016-05-24 Thread Endi Sukma Dewata 

Attached are patches to fix a problem with submitting renewal request.

https://fedorahosted.org/pki/ticket/999

--
Endi S. Dewata
>From baea3b89ad5c3866ee8e40059b1ca5774984e355 Mon Sep 17 00:00:00 2001
From: "Endi S. Dewata" 
Date: Tue, 24 May 2016 16:47:31 +0200
Subject: [PATCH] Fixed error reporting in
 RenewalProcessor.getSerialNumberFromCert().

The RenewalProcessor.getSerialNumberFromCert() has been modified
to throw an exception instead of returning null to pass the error
message to the client to help troubleshooting.

The code has also be modified to remove redundant null checking
and redundant decoding and re-encoding.

https://fedorahosted.org/pki/ticket/999
---
 .../cms/servlet/cert/RenewalProcessor.java | 98 ++
 1 file changed, 43 insertions(+), 55 deletions(-)

diff --git a/base/server/cms/src/com/netscape/cms/servlet/cert/RenewalProcessor.java b/base/server/cms/src/com/netscape/cms/servlet/cert/RenewalProcessor.java
index 87291a08bf915838c0347287f962bd4a6f591e96..eaf230b03993b193513f4fd39d4b09f65fefd352 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/cert/RenewalProcessor.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/cert/RenewalProcessor.java
@@ -33,6 +33,7 @@ import org.apache.commons.lang.StringUtils;
 import com.netscape.certsrv.apps.CMS;
 import com.netscape.certsrv.authentication.IAuthToken;
 import com.netscape.certsrv.base.BadRequestDataException;
+import com.netscape.certsrv.base.BadRequestException;
 import com.netscape.certsrv.base.EBaseException;
 import com.netscape.certsrv.base.EPropertyNotFound;
 import com.netscape.certsrv.base.SessionContext;
@@ -115,11 +116,6 @@ public class RenewalProcessor extends CertProcessor {
 // for orig request and find the right profile
 CMS.debug("RenewalSubmitter: renewal: serial_num not found, must do ssl client auth");
 certSerial = getSerialNumberFromCert(request);
-
-if (certSerial == null) {
-CMS.debug(CMS.getUserMessage(locale, "CMS_GW_MISSING_CERTS_RENEW_FROM_AUTHMGR"));
-throw new EBaseException(CMS.getUserMessage(locale, "CMS_GW_MISSING_CERTS_RENEW_FROM_AUTHMGR"));
-}
 }
 
 CMS.debug("processRenewal: serial number of cert to renew:" + certSerial.toString());
@@ -252,64 +248,56 @@ public class RenewalProcessor extends CertProcessor {
 }
 
 private BigInteger getSerialNumberFromCert(HttpServletRequest request) throws EBaseException {
-BigInteger certSerial;
+
 SSLClientCertProvider sslCCP = new SSLClientCertProvider(request);
 X509Certificate[] certs = sslCCP.getClientCertificateChain();
-certSerial = null;
+
 if (certs == null || certs.length == 0) {
-CMS.debug("RenewalSubmitter: renewal: no ssl client cert chain");
-return null;
-} else { // has ssl client cert
-CMS.debug("RenewalSubmitter: renewal: has ssl client cert chain");
-// shouldn't expect leaf cert to be always at the
-// same location
-X509Certificate clientCert = null;
-for (int i = 0; i < certs.length; i++) {
-clientCert = certs[i];
-byte[] extBytes = clientCert.getExtensionValue("2.5.29.19");
-// try to see if this is a leaf cert
-// look for BasicConstraint extension
-if (extBytes == null) {
-// found leaf cert
-CMS.debug("RenewalSubmitter: renewal: found leaf cert");
-break;
-} else {
-CMS.debug("RenewalSubmitter: renewal: found cert having BasicConstraints ext");
-// it's got BasicConstraints extension
-// so it's not likely to be a leaf cert,
-// however, check the isCA field regardless
-try {
-BasicConstraintsExtension bce =
-new BasicConstraintsExtension(true, extBytes);
-if (bce != null) {
-if (!(Boolean) bce.get("is_ca")) {
-CMS.debug("RenewalSubmitter: renewal: found CA cert in chain");
-break;
-} // else found a ca cert, continue
-}
-} catch (Exception e) {
-CMS.debug("RenewalSubmitter: renewal: exception:" + e.toString());
-return null;
-}
-}
+CMS.debug("RenewalProcessor: missing SSL client certificate chain");
+throw new BadRequestException("Missing SSL client certificate chain");
+}
+
+CMS.debug("RenewalProcessor: has SSL client cert chain");
+// shouldn't expect leaf cert to be always at the
+// same locati

[Pki-devel] Announcing the Release of Dogtag 10.3.1

2016-05-24 Thread Matthew Harmsen 

The Dogtag team is proud to announce the release of Dogtag 10.3.1.

Builds are available for Fedora 24.

== Build Versions ==

 *

   dogtag-pki-10.3.1-1
   

 *

   dogtag-pki-theme-10.3.1-1
   

 *

   pki-console-10.3.1-1
   

 *

   pki-core-10.3.1-1
   

== Upgrade Notes ==

Simply use dnf to update existing packages.

___
Pki-devel mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel