date:20160301

Re: [Pki-devel] [PATCH] 0051 Lightweight CAs: lookup correct issuer for OCSP responses

2016-03-01 Thread Fraser Tweedale

On Mon, Feb 22, 2016 at 12:02:49PM -0500, Ade Lee wrote:
> Couple of comments ..
> 
> 1. First off, there is a typo in the comments on the method.  I think
> you mean ..  
> 
> 3. Either we WERE the issuing CA, or we .. rather than "were not"
> 
> 2. We can go with the heuristic of taking the first CA, but I do not
> think we should leak information about other certs if the CA is
> incorrect.  The way the code is now, we will still return data on
> whether a particular cert serial number is valid -- even if that cert
> was not issued on that CA.
> 
> A simple solution is to simply pass code to processRequest() to ignore
> the request if the issuer is not correct and not return a response for
> that request.
> 
RFC 6960 says:

The response MUST include a SingleResponse for each certificate
in the request.

So the best we can do is return 'unknown' status in this case.

I've attached updated patch 0051-2 - the only change is the comment
fixup - and two new patches: 0074 refactors digest lookup and adds
support for SHA-2 algos, and 0075 changes the OCSP behaviour to
return 'unknown' cert status for certs that from a different issuer.

Cheers,
Fraser
From f24c753bde6f744d64a06fbe6c07a55575085691 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Thu, 1 Oct 2015 08:26:01 -0400
Subject: [PATCH] Lightweight CAs: lookup correct issuer for OCSP responses

---
 .../src/com/netscape/ca/CertificateAuthority.java  | 39 +-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/base/ca/src/com/netscape/ca/CertificateAuthority.java 
b/base/ca/src/com/netscape/ca/CertificateAuthority.java
index 
889e7e3f7595a0f831c61cecec6ee2c8fa462d44..cbb155a3bedd1517256b89ab8d2803d6ccbbb8c5
 100644
--- a/base/ca/src/com/netscape/ca/CertificateAuthority.java
+++ b/base/ca/src/com/netscape/ca/CertificateAuthority.java
@@ -2121,12 +2121,49 @@ public class CertificateAuthority implements 
ICertificateAuthority, ICertAuthori
 return null;
 }
 
+TBSRequest tbsReq = request.getTBSRequest();
+
+/* An OCSP request can contain CertIDs for certificates
+ * issued by different CAs, but each SingleResponse is valid
+ * only if the combined response was signed by its issuer or
+ * an authorised OCSP signing delegate.
+ *
+ * Even though it is silly to send an OCSP request
+ * asking about certs issued by different CAs, we must
+ * employ some heuristic to deal with this case. Our
+ * heuristic is:
+ *
+ * 1. Find the issuer of the cert identified by the first
+ *CertID in the request.
+ *
+ * 2. If this CA is *not* the issuer, look up the issuer
+ *by its DN in the caMap.  If not found, fail.  If
+ *found, dispatch to its 'validate' method.  Otherwise
+ *continue.
+ *
+ * 3. If this CA is NOT the issuing CA, we locate the
+ *issuing CA and dispatch to its 'validate' method.
+ *Otherwise, we move forward to generate and sign the
+ *aggregate OCSP response.
+ */
+ICertificateAuthority ocspCA = this;
+if (tbsReq.getRequestCount() > 0) {
+com.netscape.cmsutil.ocsp.Request req = tbsReq.getRequestAt(0);
+BigInteger serialNo = req.getCertID().getSerialNumber();
+X509CertImpl cert = mCertRepot.getX509Certificate(serialNo);
+X500Name certIssuerDN = (X500Name) cert.getIssuerDN();
+ocspCA = getCA(certIssuerDN);
+}
+if (ocspCA == null)
+throw new CANotFoundException("Could not locate issuing CA");
+if (ocspCA != this)
+return ((IOCSPService) ocspCA).validate(request);
+
 mNumOCSPRequest++;
 IStatsSubsystem statsSub = (IStatsSubsystem) CMS.getSubsystem("stats");
 long startTime = CMS.getCurrentDate().getTime();
 try {
 //log(ILogger.LL_INFO, "start OCSP request");
-TBSRequest tbsReq = request.getTBSRequest();
 
 // (3) look into database to check the
 // certificate's status
-- 
2.5.0

From bc2461f2051247f59b68ba39d0eba9796b3ddfe0 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Tue, 1 Mar 2016 20:46:49 -0500
Subject: [PATCH 74/75] Move OCSP digest name lookup to CertID class

The OCSP digest name lookup is currently defined in IOCSPAuthority
and implemented by OCSPAuthority, but /any/ code that deals with
CertID might need to know the digest, so move the lookup there.

Also refactor the lookup to use a HashMap, and add mappings for SHA2
algorithms.
---
 .../com/netscape/certsrv/ocsp/IOCSPAuthority.java   |  9 -
 base/ocsp/src/com/netscape/ocsp/OCSPAuthority.java  | 21 -
 .../cms/src/com/netscape/cms/ocsp/DefStore.java |  3 +--
 .../cms/src/com/netscape/cms/ocsp/LDAPStore.java|  3 +--
 base/util/src/com/netscape/cmsutil/ocsp/CertID.java | 19

Re: [Pki-devel] [PATCH] 278 - handle external certs

2016-03-01 Thread Ade Lee

Found an error .. lets try again ..

Ade
On Tue, 2016-03-01 at 17:20 -0500, Ade Lee wrote:
> Based on feedback, revamped the internals to make it look much nicer.
> 
> Ade
> On Tue, 2016-03-01 at 13:19 -0500, Ade Lee wrote:
> > On Mon, 2016-02-29 at 20:36 -0500, John Magne wrote:
> > > Review Notes:
> > > 
> > > Looks like good stuff.
> > > 
> > > Just a couple of small picky questions.
> > > 
> > > We can delay the final ACK to after we either decide that the
> > > questions
> > > are not that important or we decide to make any minor changes.
> > > 
> > > 
> > > thanks,
> > > jack
> > > 
> > > 
> > > This method:
> > > 
> > > in __init__.py :
> > > 
> > > 
> > > +def load_external_certs(self, conf_file):
> > > +# load external certs data
> > > +if os.path.exists(conf_file):
> > > +lines = open(conf_file).read().splitlines()
> > > +for line in lines:
> > > +parts = line.split('=', 1)
> > > +name = parts[0]
> > > +value = parts[1]
> > > +self.external_certs[name] = value
> > > +else:
> > > +self.external_certs['num_certs'] = 0
> > > +
> > > 
> > > I see if there is no file, the value of num_certs is 0.
> > > 
> > > What about in the loop that reads the file when it does exist?
> > > What
> > > if it's empty?
> > > Also when doing the splitting for the values, do we need some
> > > simple
> > > sanity checking?
> > > 
> > How about this -- 
> > 
> > +def load_external_certs(self, conf_file):
> > +# load external certs data
> > +if os.path.exists(conf_file) and
> > os.stat(conf_file).st_size
> > > 0:
> > +lines = open(conf_file).read().splitlines()
> > +for line in lines:
> > +parts = line.split('=', 1)
> >  if len(parts) != 2:
> >  // throw some exception
> > +name = parts[0]
> > +value = parts[1]
> > +self.external_certs[name] = value
> > +else:
> > +self.external_certs['num_certs'] = 0
> > 
> > +if 'num_certs' not in self.external_certs:
> >  // throw some exception
> > 
> > > Same Q's for method:   def update_external_cert_conf(self,
> > > external_path, deployer):
> > > 
> > > 
> > > def delete_external_cert(self, nickname, token):
> > > +match_found = False
> > > +num_certs = int(self.external_certs['num_certs'])
> > > +if num_certs > 0:
> > > +for num in range(0, num_certs):
> > > +current_nick = self.external_certs[str(num) +
> > > ".nickname"]
> > > +current_token = self.external_certs[str(num) +
> > > ".token"]
> > > +if current_nick == nickname and current_token ==
> > > token:
> > > +del self.external_certs[str(num) +
> > > ".nickname"]
> > > +del self.external_certs[str(num) + ".token"]
> > > +match_found = True
> > > +continue
> > > +if match_found:
> > > +self.external_certs[str(num - 1) +
> > > ".nickname"] = current_nick
> > > +self.external_certs[str(num - 1) + ".token"]
> > > =
> > > current_token
> > > +
> > > +self.external_certs['num_certs'] = num_certs - 1
> > > +self.save_external_cert_data()
> > > 
> > > I may have this wrong, but if you find a match and then continue?
> > > Will the "match_found if, ever get executed?
> > > 
> > It will get executed on the next -- and every subsequent value.
> > The purpose is to reorder subsequent values.
> > 
> > So lets say we have values 0,1,2,3,4, and we have a match on 1. 
> >  Then
> > we will remove 1 and set 2-> 1, 3-> 2, 4->3.  So the result will be
> > 0,1,2,3
> > 
> > > Also you could if you wanted pre-calculate those hash indexes
> > > into
> > > a
> > > var and use them.
> > > 
> > yeah - but it doesn't really help as much -- see comment above.
> > > 
> > > Question: Here:
> > > 
> > > nicks = self.import_certs(
> > > +instance, cert_file, nickname, token, trust_args)
> > > +self.update_instance_config(instance, nicks, token)
> > > +
> > > +self.print_message('Certificate imported for instance
> > > %s.'
> > > %
> > > +   instance_name)
> > > 
> > > Question is is there somewhere in that call chain that throws an
> > > exception if something goes wrong?
> > 
> > yes.  I'm tested a number of cases where that does happen.  The
> > exception comes all the way from nss out.
> > 
> > > Just checking to see if those late to lines get called (along
> > > with
> > > the print, even if there is an error importing certs) ??
> > > 
> > No they don't.  The exceptions cause the code to break out.
> > > 
> > > Method:
> > > 
> > > class InstanceExternalCertDeleteCLI(pki.cli.CLI):
> > > 
> > > Would there be any value in allowing multiple nicknames to be

Re: [Pki-devel] [PATCH] 0054 Lightweight CAs: add audit events

2016-03-01 Thread Fraser Tweedale

On Wed, Mar 02, 2016 at 10:05:56AM +1000, Fraser Tweedale wrote:
> On Mon, Feb 22, 2016 at 02:21:58PM -0500, Ade Lee wrote:
> > This patch needs to be rebased.
> > 
> > Tt was possible, however, to review the contents.  In general,
> > everything looks good.  It would be be useful though, to be able to
> > distinguish the many failure cases.  For instance --
> > 
> >  try {
> >  ca.modifyAuthority(data.getEnabled(), data.getDescription());
> > +audit(ILogger.SUCCESS, OpDef.OP_MODIFY, 
> > ca.getAuthorityID().toString(), auditParams);
> >  return createOKResponse(readAuthorityData(ca));
> >  } catch (CATypeException e) {
> > +audit(ILogger.FAILURE, OpDef.OP_MODIFY, 
> > ca.getAuthorityID().toString(), auditParams);
> >  throw new ForbiddenException(e.toString());
> >  } catch (IssuerUnavailableException e) {
> > +audit(ILogger.FAILURE, OpDef.OP_MODIFY, 
> > ca.getAuthorityID().toString(), auditParams);
> >  throw new ConflictingOperationException(e.toString());
> >  } catch (EBaseException e) {
> >  CMS.debug(e);
> > +audit(ILogger.FAILURE, OpDef.OP_MODIFY, 
> > ca.getAuthorityID().toString(), auditParams);
> >  throw new PKIException("Error modifying authority: " + 
> > e.toString());
> >  }
> > 
> > It would be nice to be able to determine if the modify failed because of 
> > permissions or otherwise.
> > Can we add the exception error message to the auditParams?
> > 
> > Ade 
> >
> Updated patch attached.  The "exception" key is added to the
> auditParams map to indicate the exception (if any), rather than
> adding a whole new audit message parameter.
> 
> Cheers,
> Fraser

FRACKed by Ade (Fix, retest -> ACK).
Pushed to master (2d7722f2c9b8230e79d258ad7aa1be1e87804518)

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

Re: [Pki-devel] [PATCH] 0054 Lightweight CAs: add audit events

2016-03-01 Thread Fraser Tweedale

On Mon, Feb 22, 2016 at 02:21:58PM -0500, Ade Lee wrote:
> This patch needs to be rebased.
> 
> Tt was possible, however, to review the contents.  In general,
> everything looks good.  It would be be useful though, to be able to
> distinguish the many failure cases.  For instance --
> 
>  try {
>  ca.modifyAuthority(data.getEnabled(), data.getDescription());
> +audit(ILogger.SUCCESS, OpDef.OP_MODIFY, 
> ca.getAuthorityID().toString(), auditParams);
>  return createOKResponse(readAuthorityData(ca));
>  } catch (CATypeException e) {
> +audit(ILogger.FAILURE, OpDef.OP_MODIFY, 
> ca.getAuthorityID().toString(), auditParams);
>  throw new ForbiddenException(e.toString());
>  } catch (IssuerUnavailableException e) {
> +audit(ILogger.FAILURE, OpDef.OP_MODIFY, 
> ca.getAuthorityID().toString(), auditParams);
>  throw new ConflictingOperationException(e.toString());
>  } catch (EBaseException e) {
>  CMS.debug(e);
> +audit(ILogger.FAILURE, OpDef.OP_MODIFY, 
> ca.getAuthorityID().toString(), auditParams);
>  throw new PKIException("Error modifying authority: " + 
> e.toString());
>  }
> 
> It would be nice to be able to determine if the modify failed because of 
> permissions or otherwise.
> Can we add the exception error message to the auditParams?
> 
> Ade 
>
Updated patch attached.  The "exception" key is added to the
auditParams map to indicate the exception (if any), rather than
adding a whole new audit message parameter.

Cheers,
Fraser
From 2d7722f2c9b8230e79d258ad7aa1be1e87804518 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Mon, 2 Nov 2015 01:43:26 -0500
Subject: [PATCH] Lightweight CAs: add audit events

Add audit events for lightweight CA administration.

Fixes: https://fedorahosted.org/pki/ticket/1590
---
 base/ca/shared/conf/CS.cfg.in  |  4 +-
 .../dogtagpki/server/ca/rest/AuthorityService.java | 72 +++---
 .../src/com/netscape/certsrv/common/ScopeDef.java  |  3 +
 base/server/cmsbundle/src/LogMessages.properties   |  8 +++
 4 files changed, 64 insertions(+), 23 deletions(-)

diff --git a/base/ca/shared/conf/CS.cfg.in b/base/ca/shared/conf/CS.cfg.in
index 
05508f8b289e027cc4e97df3fe584dfbc2290f7e..c679af5b3fd7f1fcda79848f68a9258580457b63
 100644
--- a/base/ca/shared/conf/CS.cfg.in
+++ b/base/ca/shared/conf/CS.cfg.in
@@ -901,11 +901,11 @@ log.instance.SignedAudit._001=## Signed Audit Logging
 log.instance.SignedAudit._002=##
 log.instance.SignedAudit._003=##
 log.instance.SignedAudit._004=## Available Audit events:
-log.instance.SignedAudit._005=## 
AUDIT_LOG_STARTUP,AUDIT_LOG_SHUTDOWN,ROLE_ASSUME,CONFIG_CERT_POLICY,CONFIG_CERT_PROFILE,CONFIG_CRL_PROFILE,CONFIG_OCSP_PROFILE,CONFIG_AUTH,CONFIG_ROLE,CONFIG_ACL,CONFIG_SIGNED_AUDIT,CONFIG_ENCRYPTION,CONFIG_TRUSTED_PUBLIC_KEY,CONFIG_DRM,SELFTESTS_EXECUTION,AUDIT_LOG_DELETE,LOG_PATH_CHANGE,LOG_EXPIRATION_CHANGE,PRIVATE_KEY_ARCHIVE_REQUEST,PRIVATE_KEY_ARCHIVE_REQUEST_PROCESSED,PRIVATE_KEY_EXPORT_REQUEST_PROCESSED_SUCCESS,PRIVATE_KEY_EXPORT_REQUEST_PROCESSED_FAILURE,KEY_RECOVERY_REQUEST,KEY_RECOVERY_REQUEST_ASYNC,KEY_RECOVERY_AGENT_LOGIN,KEY_RECOVERY_REQUEST_PROCESSED,KEY_RECOVERY_REQUEST_PROCESSED_ASYNC,KEY_GEN_ASYMMETRIC,NON_PROFILE_CERT_REQUEST,PROFILE_CERT_REQUEST,CERT_REQUEST_PROCESSED,CERT_STATUS_CHANGE_REQUEST,CERT_STATUS_CHANGE_REQUEST_PROCESSED,AUTHZ_SUCCESS,AUTHZ_FAIL,INTER_BOUNDARY,AUTH_FAIL,AUTH_SUCCESS,CERT_PROFILE_APPROVAL,PROOF_OF_POSSESSION,CRL_RETRIEVAL,CRL_VALIDATION,CMC_SIGNED_REQUEST_SIG_VERIFY,SERVER_SIDE_KEYGEN_REQUEST_PROCESSED_FAILURE,SERVER_SIDE_KEYGEN_REQUEST_PROCESSED_SUCCESS,SERVER_SIDE_KEYGEN_REQUEST,COMPUTE_SESSION_KEY_REQUEST,COMPUTE_SESSION_KEY_REQUEST_PROCESSED_SUCCESS,
 
COMPUTE_SESSION_KEY_REQUEST_PROCESSED_FAILURE,DIVERSIFY_KEY_REQUEST,DIVERSIFY_KEY_REQUEST_PROCESSED_SUCCESS,
 
DIVERSIFY_KEY_REQUEST_PROCESSED_FAILURE,ENCRYPT_DATA_REQUEST,ENCRYPT_DATA_REQUEST_PROCESSED_SUCCESS,ENCRYPT_DATA_REQUEST_PROCESSED_FAILURE,OCSP_ADD_CA_REQUEST,OCSP_ADD_CA_REQUEST_PROCESSED,OCSP_REMOVE_CA_REQUEST,OCSP_REMOVE_CA_REQUEST_PROCESSED_SUCCESS,OCSP_REMOVE_CA_REQUEST_PROCESSED_FAILURE,COMPUTE_RANDOM_DATA_REQUEST,COMPUTE_RANDOM_DATA_REQUEST_PROCESSED_SUCCESS,COMPUTE_RANDOM_DATA_REQUEST_PROCESSED_FAILURE,CIMC_CERT_VERIFICATION,SECURITY_DOMAIN_UPDATE,CONFIG_SERIAL_NUMBER
+log.instance.SignedAudit._005=## 
AUDIT_LOG_STARTUP,AUDIT_LOG_SHUTDOWN,ROLE_ASSUME,CONFIG_CERT_POLICY,CONFIG_CERT_PROFILE,CONFIG_CRL_PROFILE,CONFIG_OCSP_PROFILE,CONFIG_AUTH,CONFIG_ROLE,CONFIG_ACL,CONFIG_SIGNED_AUDIT,CONFIG_ENCRYPTION,CONFIG_TRUSTED_PUBLIC_KEY,CONFIG_DRM,SELFTESTS_EXECUTION,AUDIT_LOG_DELETE,LOG_PATH_CHANGE,LOG_EXPIRATION_CHANGE,PRIVATE_KEY_ARCHIVE_REQUEST,PRIVATE_KEY_ARCHIVE_REQUEST_PROCESSED,PRIVATE_KEY_EXPORT_REQUEST_PROCESSED_SUCCESS,PRIVATE_KEY_EXPORT_REQUEST_PROCESSED_FAILURE,KEY_RECOVERY_REQUEST,KEY_RECOVERY_REQUEST_ASYNC,KEY_RECOVERY_AGENT_LOGIN,KEY_RECOVERY

Re: [Pki-devel] [PATCH] 278 - handle external certs

2016-03-01 Thread Ade Lee

Based on feedback, revamped the internals to make it look much nicer.

Ade
On Tue, 2016-03-01 at 13:19 -0500, Ade Lee wrote:
> On Mon, 2016-02-29 at 20:36 -0500, John Magne wrote:
> > Review Notes:
> > 
> > Looks like good stuff.
> > 
> > Just a couple of small picky questions.
> > 
> > We can delay the final ACK to after we either decide that the
> > questions
> > are not that important or we decide to make any minor changes.
> > 
> > 
> > thanks,
> > jack
> > 
> > 
> > This method:
> > 
> > in __init__.py :
> > 
> > 
> > +def load_external_certs(self, conf_file):
> > +# load external certs data
> > +if os.path.exists(conf_file):
> > +lines = open(conf_file).read().splitlines()
> > +for line in lines:
> > +parts = line.split('=', 1)
> > +name = parts[0]
> > +value = parts[1]
> > +self.external_certs[name] = value
> > +else:
> > +self.external_certs['num_certs'] = 0
> > +
> > 
> > I see if there is no file, the value of num_certs is 0.
> > 
> > What about in the loop that reads the file when it does exist? What
> > if it's empty?
> > Also when doing the splitting for the values, do we need some
> > simple
> > sanity checking?
> > 
> How about this -- 
> 
> +def load_external_certs(self, conf_file):
> +# load external certs data
> +if os.path.exists(conf_file) and os.stat(conf_file).st_size
> > 0:
> +lines = open(conf_file).read().splitlines()
> +for line in lines:
> +parts = line.split('=', 1)
>  if len(parts) != 2:
>  // throw some exception
> +name = parts[0]
> +value = parts[1]
> +self.external_certs[name] = value
> +else:
> +self.external_certs['num_certs'] = 0
> 
> +if 'num_certs' not in self.external_certs:
>  // throw some exception
> 
> > Same Q's for method:   def update_external_cert_conf(self,
> > external_path, deployer):
> > 
> > 
> > def delete_external_cert(self, nickname, token):
> > +match_found = False
> > +num_certs = int(self.external_certs['num_certs'])
> > +if num_certs > 0:
> > +for num in range(0, num_certs):
> > +current_nick = self.external_certs[str(num) +
> > ".nickname"]
> > +current_token = self.external_certs[str(num) +
> > ".token"]
> > +if current_nick == nickname and current_token ==
> > token:
> > +del self.external_certs[str(num) +
> > ".nickname"]
> > +del self.external_certs[str(num) + ".token"]
> > +match_found = True
> > +continue
> > +if match_found:
> > +self.external_certs[str(num - 1) +
> > ".nickname"] = current_nick
> > +self.external_certs[str(num - 1) + ".token"] =
> > current_token
> > +
> > +self.external_certs['num_certs'] = num_certs - 1
> > +self.save_external_cert_data()
> > 
> > I may have this wrong, but if you find a match and then continue?
> > Will the "match_found if, ever get executed?
> > 
> It will get executed on the next -- and every subsequent value.
> The purpose is to reorder subsequent values.
> 
> So lets say we have values 0,1,2,3,4, and we have a match on 1.  Then
> we will remove 1 and set 2-> 1, 3-> 2, 4->3.  So the result will be
> 0,1,2,3
> 
> > Also you could if you wanted pre-calculate those hash indexes into
> > a
> > var and use them.
> > 
> yeah - but it doesn't really help as much -- see comment above.
> > 
> > Question: Here:
> > 
> > nicks = self.import_certs(
> > +instance, cert_file, nickname, token, trust_args)
> > +self.update_instance_config(instance, nicks, token)
> > +
> > +self.print_message('Certificate imported for instance %s.'
> > %
> > +   instance_name)
> > 
> > Question is is there somewhere in that call chain that throws an
> > exception if something goes wrong?
> 
> yes.  I'm tested a number of cases where that does happen.  The
> exception comes all the way from nss out.
> 
> > Just checking to see if those late to lines get called (along with
> > the print, even if there is an error importing certs) ??
> > 
> No they don't.  The exceptions cause the code to break out.
> > 
> > Method:
> > 
> > class InstanceExternalCertDeleteCLI(pki.cli.CLI):
> > 
> > Would there be any value in allowing multiple nicknames to be
> > deleted?
> > 
> perhaps - we can add as a refinement later if needed.
> 
> > Also same at above here:
> > 
> > instance = pki.server.PKIInstance(instance_name)
> > +instance.load()
> > +
> > +self.remove_cert(instance, nickname, token)
> > +instance.delete_external_cert(nickname, token)
> > +
> > +self.print_message('Certificate removed from instance %s.'
> > %
> >

Re: [Pki-devel] [PATCH] 0056 Fix sphinx build

2016-03-01 Thread Christian Heimes

On 2016-03-01 18:39, Christian Heimes wrote:
> Hi,
> 
> this patch fixes Python package lookup for sphinx-builder. Now sphinx
> picks up the correct files from the current source directory.

Pushed to master in 298e15c16e266d2ca81e62354edb9dc5626c1f72

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

Re: [Pki-devel] [PATCH] 278 - handle external certs

2016-03-01 Thread Ade Lee

On Mon, 2016-02-29 at 20:36 -0500, John Magne wrote:
> Review Notes:
> 
> Looks like good stuff.
> 
> Just a couple of small picky questions.
> 
> We can delay the final ACK to after we either decide that the
> questions
> are not that important or we decide to make any minor changes.
> 
> 
> thanks,
> jack
> 
> 
> This method:
> 
> in __init__.py :
> 
> 
> +def load_external_certs(self, conf_file):
> +# load external certs data
> +if os.path.exists(conf_file):
> +lines = open(conf_file).read().splitlines()
> +for line in lines:
> +parts = line.split('=', 1)
> +name = parts[0]
> +value = parts[1]
> +self.external_certs[name] = value
> +else:
> +self.external_certs['num_certs'] = 0
> +
> 
> I see if there is no file, the value of num_certs is 0.
> 
> What about in the loop that reads the file when it does exist? What
> if it's empty?
> Also when doing the splitting for the values, do we need some simple
> sanity checking?
> 
How about this -- 

+def load_external_certs(self, conf_file):
+# load external certs data
+if os.path.exists(conf_file) and os.stat(conf_file).st_size > 0:
+lines = open(conf_file).read().splitlines()
+for line in lines:
+parts = line.split('=', 1)
 if len(parts) != 2:
 // throw some exception
+name = parts[0]
+value = parts[1]
+self.external_certs[name] = value
+else:
+self.external_certs['num_certs'] = 0

+if 'num_certs' not in self.external_certs:
 // throw some exception

> Same Q's for method:   def update_external_cert_conf(self,
> external_path, deployer):
> 
> 
> def delete_external_cert(self, nickname, token):
> +match_found = False
> +num_certs = int(self.external_certs['num_certs'])
> +if num_certs > 0:
> +for num in range(0, num_certs):
> +current_nick = self.external_certs[str(num) + ".nickname"]
> +current_token = self.external_certs[str(num) + ".token"]
> +if current_nick == nickname and current_token == token:
> +del self.external_certs[str(num) + ".nickname"]
> +del self.external_certs[str(num) + ".token"]
> +match_found = True
> +continue
> +if match_found:
> +self.external_certs[str(num - 1) + ".nickname"] = 
> current_nick
> +self.external_certs[str(num - 1) + ".token"] = 
> current_token
> +
> +self.external_certs['num_certs'] = num_certs - 1
> +self.save_external_cert_data()
> 
> I may have this wrong, but if you find a match and then continue?
> Will the "match_found if, ever get executed?
> 
It will get executed on the next -- and every subsequent value.
The purpose is to reorder subsequent values.

So lets say we have values 0,1,2,3,4, and we have a match on 1.  Then
we will remove 1 and set 2-> 1, 3-> 2, 4->3.  So the result will be
0,1,2,3

> Also you could if you wanted pre-calculate those hash indexes into a
> var and use them.
> 
yeah - but it doesn't really help as much -- see comment above.
> 
> Question: Here:
> 
> nicks = self.import_certs(
> +instance, cert_file, nickname, token, trust_args)
> +self.update_instance_config(instance, nicks, token)
> +
> +self.print_message('Certificate imported for instance %s.' %
> +   instance_name)
> 
> Question is is there somewhere in that call chain that throws an
> exception if something goes wrong?

yes.  I'm tested a number of cases where that does happen.  The
exception comes all the way from nss out.

> Just checking to see if those late to lines get called (along with
> the print, even if there is an error importing certs) ??
> 
No they don't.  The exceptions cause the code to break out.
> 
> Method:
> 
> class InstanceExternalCertDeleteCLI(pki.cli.CLI):
> 
> Would there be any value in allowing multiple nicknames to be
> deleted?
> 
perhaps - we can add as a refinement later if needed.

> Also same at above here:
> 
> instance = pki.server.PKIInstance(instance_name)
> +instance.load()
> +
> +self.remove_cert(instance, nickname, token)
> +instance.delete_external_cert(nickname, token)
> +
> +self.print_message('Certificate removed from instance %s.' %
> +   instance_name)
> 
> 
> Do we get to the end if instance.load does not work?
> 
No we don't.  Exceptions propagate up.

> 
> 
> 
> 
> 
> - Original Message -
> From: "Ade Lee" 
> To: pki-devel@redhat.com
> Sent: Monday, February 29, 2016 9:25:30 AM
> Subject: [Pki-devel] [PATCH] 278 - handle external certs
> 
> This is to resolve ticket 1742.
> 
> For this ticket, we need a mechanism to import thi

Re: [Pki-devel] [PATCH] 0056 Fix sphinx build

2016-03-01 Thread Matthew Harmsen


On 03/01/2016 10:39 AM, Christian Heimes wrote:

Hi,

this patch fixes Python package lookup for sphinx-builder. Now sphinx
picks up the correct files from the current source directory.

Christian


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

Thanks!  Worked like a charm.

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

[Pki-devel] [PATCH] 0056 Fix sphinx build

2016-03-01 Thread Christian Heimes

Hi,

this patch fixes Python package lookup for sphinx-builder. Now sphinx
picks up the correct files from the current source directory.

Christian
From 620872288075a9ffeb52d670d7e26543c87c067c Mon Sep 17 00:00:00 2001
From: Christian Heimes 
Date: Tue, 1 Mar 2016 11:08:59 +0100
Subject: [PATCH] Use CMAKE_CURRENT_SOURCE_DIR for sphinx-build

sphinx-build used wrong search path for pki Python package. This caused
builds to fail on some machines. On systems with pki-base installed,
sphinx-build picked up the wrong files.
---
 base/common/python/CMakeLists.txt | 8 ++--
 base/common/python/conf.py| 3 ++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/base/common/python/CMakeLists.txt b/base/common/python/CMakeLists.txt
index d667421bfb3d42577ee11a21d919a65ff6828633..a164597755e4a48169610fbf81a092c57cc0dd9f 100644
--- a/base/common/python/CMakeLists.txt
+++ b/base/common/python/CMakeLists.txt
@@ -16,23 +16,27 @@ configure_file(
 add_custom_target(dogtag_python_client_docs ALL
 ${SPHINX_EXECUTABLE}
 -b html
--c "${CMAKE_CURRENT_BINARY_DIR}"
+-c "${CMAKE_CURRENT_SOURCE_DIR}"
 -w "${CMAKE_CURRENT_BINARY_DIR}/python-client-lib-html.log"
 -a
 -W
 "${CMAKE_CURRENT_SOURCE_DIR}"
 "${CMAKE_CURRENT_BINARY_DIR}/html"
+WORKING_DIRECTORY
+${CMAKE_CURRENT_SOURCE_DIR}
 COMMENT "Building Python Client Library HTML documentation")
 
 add_custom_target(dogtag_python_client_man_docs ALL
 ${SPHINX_EXECUTABLE}
 -b man
--c "${CMAKE_CURRENT_BINARY_DIR}"
+-c "${CMAKE_CURRENT_SOURCE_DIR}"
 -w "${CMAKE_CURRENT_BINARY_DIR}/python-client-lib-man.log"
 -a
 -W
 "${CMAKE_CURRENT_SOURCE_DIR}"
 "${CMAKE_CURRENT_BINARY_DIR}/man"
+WORKING_DIRECTORY
+${CMAKE_CURRENT_SOURCE_DIR}
 COMMENT "Building Python Client Library manual pages")
 
 install(
diff --git a/base/common/python/conf.py b/base/common/python/conf.py
index 3e548147ea9b3c3928100e0f2dd0623a9bd68c02..f996c6371cbd545ae5fc9cc3e12cab0977d56cdb 100644
--- a/base/common/python/conf.py
+++ b/base/common/python/conf.py
@@ -19,7 +19,8 @@ import sphinx
 # If extensions (or modules to document with autodoc) are in another directory,
 # add these directories to sys.path here. If the directory is relative to the
 # documentation root, use os.path.abspath to make it absolute, like shown here.
-sys.path.insert(0, os.path.abspath('pki'))
+sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
+
 
 # -- General configuration ---
 
-- 
2.5.0



signature.asc
Description: OpenPGP digital signature
___
Pki-devel mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel

Re: [Pki-devel] [PATCH] 281 - separate pki-base into python and java components

2016-03-01 Thread Ade Lee

Thanks. Fixed.  pushed to master.

To ssh://vakw...@git.fedorahosted.org/git/pki.git
   11f8fbb..49e4fff  master -> master

On Tue, 2016-03-01 at 12:44 +0100, Christian Heimes wrote:
> On 2016-03-01 06:53, Ade Lee wrote:
> > In this patch, I move all java components (and requirements) for
> > pki-base to a new package pki-base-java.
> > 
> > This makes the footprint much smaller for dogtag python clients -
> > like openstack for instance.
> 
> Shouldn't pki-base-java depend on pki-base, too?
> 
> %package -n   pki-base-java
> ...
> Requires: pki-base = %{version}-%{release}
> 
> Christian
> 

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

Re: [Pki-devel] [PATCH] 281 - separate pki-base into python and java components

2016-03-01 Thread Christian Heimes

On 2016-03-01 06:53, Ade Lee wrote:
> In this patch, I move all java components (and requirements) for
> pki-base to a new package pki-base-java.
> 
> This makes the footprint much smaller for dogtag python clients -
> like openstack for instance.

Shouldn't pki-base-java depend on pki-base, too?

%package -n   pki-base-java
...
Requires: pki-base = %{version}-%{release}

Christian



signature.asc
Description: OpenPGP digital signature
___
Pki-devel mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel

Re: [Pki-devel] [PATCH] 0051 Lightweight CAs: lookup correct issuer for OCSP responses

Re: [Pki-devel] [PATCH] 278 - handle external certs

Re: [Pki-devel] [PATCH] 0054 Lightweight CAs: add audit events

Re: [Pki-devel] [PATCH] 0054 Lightweight CAs: add audit events

Re: [Pki-devel] [PATCH] 278 - handle external certs

Re: [Pki-devel] [PATCH] 0056 Fix sphinx build

Re: [Pki-devel] [PATCH] 278 - handle external certs

Re: [Pki-devel] [PATCH] 0056 Fix sphinx build

[Pki-devel] [PATCH] 0056 Fix sphinx build

Re: [Pki-devel] [PATCH] 281 - separate pki-base into python and java components

Re: [Pki-devel] [PATCH] 281 - separate pki-base into python and java components

11 matches

Site Navigation

Mail list logo

Footer information