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

[Endi Sukma Dewata]

On 5/24/2016 11:55 AM, Endi Sukma Dewata wrote:

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

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


This was conditionally ACKed by jmagne (thanks!). It's been tested to 
work with the UI and CLI with a minor revision. 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] 754-755 Fixed problem submitting renewal request.

[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
+