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" <[email protected]> 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 location + + X509Certificate clientCert = null; + for (X509Certificate cert : certs) { + + CMS.debug("RenewalProcessor: cert " + cert.getSubjectDN()); + clientCert = cert; + + 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("RenewalProcessor: found leaf cert"); + break; } - if (clientCert == null) { - CMS.debug("RenewalSubmitter: renewal: no client cert in chain"); - return null; - } - // convert to java X509 cert interface + + CMS.debug("RenewalProcessor: 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 { - byte[] certEncoded = clientCert.getEncoded(); - clientCert = new X509CertImpl(certEncoded); + BasicConstraintsExtension bce = new BasicConstraintsExtension(true, extBytes); + if (!(Boolean) bce.get("is_ca")) { + CMS.debug("RenewalProcessor: found CA cert in chain"); + break; + } // else found a ca cert, continue + } catch (Exception e) { - e.printStackTrace(); - CMS.debug("RenewalSubmitter: renewal: exception:" + e.toString()); - return null; + CMS.debug("RenewalProcessor: Invalid certificate extension:" + e); + throw new BadRequestException("Invalid certificate extension: " + e.getMessage(), e); } - - certSerial = clientCert.getSerialNumber(); } - return certSerial; + + // clientCert cannot be null here + + return clientCert.getSerialNumber(); } /* -- 2.4.11
>From d2adfc0e6c593bc4cf6483f8a0ed1b8144923b2f Mon Sep 17 00:00:00 2001 From: "Endi S. Dewata" <[email protected]> Date: Tue, 24 May 2016 09:41:58 +0200 Subject: [PATCH] Fixed problem submitting renewal request. The RenewalProcessor.processRenewal() has been modified to get the serial number of the certificate to renew from the profile input in addition to the <SerialNumber> attribute and client certificate. The serialNum field in CertEnrollmentRequest has been modified to use CertId which accepts both decimal and hexadecimal value. https://fedorahosted.org/pki/ticket/999 --- .../server/ca/rest/CertRequestService.java | 1 - .../certsrv/cert/CertEnrollmentRequest.java | 14 ++-- .../cms/servlet/cert/RenewalProcessor.java | 89 ++++++++++++++++++---- .../cms/servlet/profile/ProfileSubmitServlet.java | 4 +- 4 files changed, 86 insertions(+), 22 deletions(-) diff --git a/base/ca/src/org/dogtagpki/server/ca/rest/CertRequestService.java b/base/ca/src/org/dogtagpki/server/ca/rest/CertRequestService.java index 80aaf6f7899d92675c15c6f944b7a3a491784145..8498ac984bc70471c396d1092bdfbd2d59d81a86 100644 --- a/base/ca/src/org/dogtagpki/server/ca/rest/CertRequestService.java +++ b/base/ca/src/org/dogtagpki/server/ca/rest/CertRequestService.java @@ -387,7 +387,6 @@ public class CertRequestService extends PKIService implements CertRequestResourc request.setRenewal(Boolean.parseBoolean(profile.isRenewal())); request.setRemoteAddr(""); request.setRemoteHost(""); - request.setSerialNum(""); // populate inputs Enumeration<String> inputIds = profile.getProfileInputIds(); diff --git a/base/common/src/com/netscape/certsrv/cert/CertEnrollmentRequest.java b/base/common/src/com/netscape/certsrv/cert/CertEnrollmentRequest.java index 2b914e85667dc525947f7357ceaf6bbe464a2480..e3ea69c24e75de0dba87ff07cd7805107e1d7479 100644 --- a/base/common/src/com/netscape/certsrv/cert/CertEnrollmentRequest.java +++ b/base/common/src/com/netscape/certsrv/cert/CertEnrollmentRequest.java @@ -36,8 +36,11 @@ import javax.xml.bind.annotation.XmlAccessType; import javax.xml.bind.annotation.XmlAccessorType; import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlRootElement; +import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter; import com.netscape.certsrv.base.ResourceMessage; +import com.netscape.certsrv.dbs.certdb.CertId; +import com.netscape.certsrv.dbs.certdb.CertIdAdapter; import com.netscape.certsrv.profile.ProfileAttribute; import com.netscape.certsrv.profile.ProfileInput; import com.netscape.certsrv.profile.ProfileOutput; @@ -62,7 +65,8 @@ public class CertEnrollmentRequest extends ResourceMessage { protected boolean renewal; @XmlElement(name="SerialNumber") - protected String serialNum; // used for one type of renewal + @XmlJavaTypeAdapter(CertIdAdapter.class) + protected CertId serialNum; // used for one type of renewal @XmlElement(name="RemoteHost") protected String remoteHost; @@ -83,7 +87,7 @@ public class CertEnrollmentRequest extends ResourceMessage { public CertEnrollmentRequest(MultivaluedMap<String, String> form) { profileId = form.getFirst(PROFILE_ID); String renewalStr = form.getFirst(RENEWAL); - serialNum = form.getFirst(SERIAL_NUM); + serialNum = new CertId(form.getFirst(SERIAL_NUM)); renewal = new Boolean(renewalStr); } @@ -206,7 +210,7 @@ public class CertEnrollmentRequest extends ResourceMessage { HashMap<String, String> ret = new HashMap<String, String>(); ret.put("isRenewal", Boolean.valueOf(renewal).toString()); if (profileId != null) ret.put(PROFILE_ID, profileId); - if (serialNum != null) ret.put(SERIAL_NUM, serialNum); + if (serialNum != null) ret.put(SERIAL_NUM, serialNum.toHexString()); if (remoteHost != null) ret.put("remoteHost", remoteHost); if (remoteAddr != null) ret.put("remoteAddr", remoteAddr); @@ -219,11 +223,11 @@ public class CertEnrollmentRequest extends ResourceMessage { return ret; } - public String getSerialNum() { + public CertId getSerialNum() { return serialNum; } - public void setSerialNum(String serialNum) { + public void setSerialNum(CertId serialNum) { this.serialNum = serialNum; } 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 eaf230b03993b193513f4fd39d4b09f65fefd352..b22cc1ce4c28d89bc5380d1ab27d90775245abd5 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 @@ -38,13 +38,19 @@ import com.netscape.certsrv.base.EBaseException; import com.netscape.certsrv.base.EPropertyNotFound; import com.netscape.certsrv.base.SessionContext; import com.netscape.certsrv.cert.CertEnrollmentRequest; +import com.netscape.certsrv.dbs.certdb.CertId; import com.netscape.certsrv.dbs.certdb.ICertRecord; import com.netscape.certsrv.profile.IEnrollProfile; import com.netscape.certsrv.profile.IProfile; import com.netscape.certsrv.profile.IProfileAuthenticator; import com.netscape.certsrv.profile.IProfileContext; import com.netscape.certsrv.profile.IProfileInput; +import com.netscape.certsrv.profile.ProfileAttribute; +import com.netscape.certsrv.profile.ProfileInput; +import com.netscape.certsrv.registry.IPluginInfo; +import com.netscape.certsrv.registry.IPluginRegistry; import com.netscape.certsrv.request.IRequest; +import com.netscape.cms.profile.input.SerialNumRenewInput; import com.netscape.cms.servlet.common.AuthCredentials; import com.netscape.cms.servlet.common.CMSTemplate; import com.netscape.cms.servlet.profile.SSLClientCertProvider; @@ -77,7 +83,8 @@ public class RenewalProcessor extends CertProcessor { HashMap<String, String> params = data.toParams(); printParameterValues(params); } - CMS.debug("RenewalSubmitter: isRenewal true"); + + CMS.debug("RenewalProcessor: processRenewal()"); startTiming("enrollment"); request.setAttribute("reqType", "renewal"); @@ -85,7 +92,7 @@ public class RenewalProcessor extends CertProcessor { // in case of renew, "profile" is the orig profile // while "renewProfile" is the current profile used for renewal String renewProfileId = (this.profileID == null) ? data.getProfileId() : this.profileID; - CMS.debug("processRenewal: renewProfileId " + renewProfileId); + CMS.debug("RenewalProcessor: profile: " + renewProfileId); IProfile renewProfile = ps.getProfile(renewProfileId); if (renewProfile == null) { @@ -94,27 +101,79 @@ public class RenewalProcessor extends CertProcessor { throw new BadRequestDataException(CMS.getUserMessage(locale, "CMS_PROFILE_NOT_FOUND",CMSTemplate.escapeJavaScriptStringHTML(renewProfileId))); } if (!ps.isProfileEnable(renewProfileId)) { - CMS.debug("RenewalSubmitter: Profile " + renewProfileId + " not enabled"); + CMS.debug("RenewalProcessor: Profile " + renewProfileId + " not enabled"); throw new BadRequestDataException("Profile " + renewProfileId + " not enabled"); } - String serial = data.getSerialNum(); BigInteger certSerial = null; - if (StringUtils.isNotEmpty(serial)) { - // if serial number is sent with request, then the authentication - // method is not ssl client auth. In this case, an alternative - // authentication method is used (default: ldap based) - // usr_origreq evaluator should be used to authorize ownership - // of the cert - CMS.debug("RenewalSubmitter: renewal: serial number: " + serial); - certSerial = new BigInteger(serial); - - } else { + // get serial number from <SerialNumber> element (no auth required) + CertId serial = data.getSerialNum(); + if (serial != null) { + CMS.debug("RenewalProcessor: serial number: " + serial); + certSerial = serial.toBigInteger(); + } + + // if not found, get serial number from profile input (no auth required) + if (certSerial == null) { + + IPluginRegistry registry = (IPluginRegistry) CMS.getSubsystem(CMS.SUBSYSTEM_REGISTRY); + + // find SerialNumRenewInput + for (ProfileInput input : data.getInputs()) { + + String inputId = input.getId(); + if (inputId == null) { + throw new BadRequestException("Missing input ID"); + } + + String classId = input.getClassId(); + if (classId == null) { + throw new BadRequestException("Missing class ID in input " + inputId); + } + + IPluginInfo pluginInfo = registry.getPluginInfo("profileInput", classId); + if (pluginInfo == null) { + throw new BadRequestException("Unregistered class ID " + classId + " in input " + inputId); + } + + String className = pluginInfo.getClassName(); + if (!SerialNumRenewInput.class.getName().equals(className)) { + // check the next input + continue; + } + + CMS.debug("RenewalProcessor: found SerialNumRenewInput"); + ProfileAttribute attribute = input.getAttribute(SerialNumRenewInput.SERIAL_NUM); + + if (attribute == null) { + throw new BadRequestException("Missing attribute " + SerialNumRenewInput.SERIAL_NUM + " in input " + inputId); + } + + String value = attribute.getValue(); + CMS.debug("RenewalProcessor: profile input " + SerialNumRenewInput.SERIAL_NUM + " value: " + value); + + if (StringUtils.isEmpty(value)) { + throw new BadRequestException("Missing attribute value for " + SerialNumRenewInput.SERIAL_NUM + " in input " + inputId); + } + + serial = new CertId(value); + certSerial = serial.toBigInteger(); + break; + } + } + + // if still not found, get serial number from client certificate (if provided) + if (certSerial == null) { + + if (!request.isSecure()) { + throw new BadRequestException("Missing serial number"); + } + // ssl client auth is to be used // this is not authentication. Just use the cert to search // for orig request and find the right profile - CMS.debug("RenewalSubmitter: renewal: serial_num not found, must do ssl client auth"); + CMS.debug("RenewalProcessor: get serial number from client certificate"); certSerial = getSerialNumberFromCert(request); } diff --git a/base/server/cms/src/com/netscape/cms/servlet/profile/ProfileSubmitServlet.java b/base/server/cms/src/com/netscape/cms/servlet/profile/ProfileSubmitServlet.java index 7cced7c47727b2f71b9a5523b5a0c3d3ca0e8af1..d74fcd4216d62638dfc10c2bf4c6e63edd4981c6 100644 --- a/base/server/cms/src/com/netscape/cms/servlet/profile/ProfileSubmitServlet.java +++ b/base/server/cms/src/com/netscape/cms/servlet/profile/ProfileSubmitServlet.java @@ -37,6 +37,7 @@ import com.netscape.certsrv.ca.AuthorityID; import com.netscape.certsrv.ca.CANotFoundException; import com.netscape.certsrv.ca.ICertificateAuthority; import com.netscape.certsrv.cert.CertEnrollmentRequest; +import com.netscape.certsrv.dbs.certdb.CertId; import com.netscape.certsrv.profile.EProfileException; import com.netscape.certsrv.profile.IEnrollProfile; import com.netscape.certsrv.profile.IProfile; @@ -264,7 +265,8 @@ public class ProfileSubmitServlet extends ProfileServlet { CertEnrollmentRequest data = CertEnrollmentRequestFactory.create(cmsReq, profile, locale); //only used in renewal - data.setSerialNum(request.getParameter("serial_num")); + String serialNumber = request.getParameter("serial_num"); + data.setSerialNum(new CertId(serialNumber)); return processor.processRenewal(data, request, null); } -- 2.4.11
_______________________________________________ Pki-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/pki-devel
