Re: [Pki-devel] [PATCH] 699 Fixed exception handling in EnrollProfile.

2016-03-24 Thread Endi Sukma Dewata

On 3/23/2016 2:16 PM, Ade Lee wrote:

ACK


Thanks! Restored & updated some log messages per jmagne's feedback. 
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] 0084..0086 Lightweight CA replication support

2016-03-24 Thread Ade Lee
A few comments.

1. One of the first things that struck me as odd was making
CertificateAuthority implement Runnable.  I think it would be cleaner
to have a static inner class called AuthorityMonitor or similar to
which we pass in the CertificateAuthority.

2. I do like the fact that the caMap updates are done through a static
database connection factory created by the hostCA.  How do you ensure
that the database connection factory is created before being used by
other CAs? 

3.  I'm not sure I understand how the initialLoadDone counter is
supposed to work.  Are all the CA's supposed to stop until the hostCA
has completed its initial load?  Because it looks like only the hostCA
calls await().

4. There is a lot of code in that initial patch.  It would help review
to split that off into at least two patches, say one in which you add
the functions in CertificateAuthority that handle modifications in the
caMap based on persistent search results, and one which adds the new
monitor thread.

5. Some in-code documentation would not go amiss.  For instance, I have
no idea why this code is correct --

String[] objectClasses =
entry.getAttribute("objectClass").getStringValueArray();
if 
(Arrays.asList(objectClasses).contains("organizationalUnit")) {
initialNumAuthorities = new Integer(
entry.getAttribute("numSubordinates")
.getStringValueArray()[0]);
checkInitialLoadDone();
continue;
}
organizationalUnit?

There are lots of different variables like initialNumAuthorities etc. which 
could
potentially be hidden in an inner class, making this more understandable.
 
Ade

On Tue, 2016-03-22 at 16:00 +1000, Fraser Tweedale wrote:
> On Fri, Mar 18, 2016 at 02:30:24PM +1000, Fraser Tweedale wrote:
> > Hi all,
> > 
> > The attached patches implement replication support for lightweight
> > CAs.  These patches do not implement key replication via Custodia
> > (my next task) but they do implement the persistent search thread
> > and appropriate** API behaviour when the signing keys are not yet
> > available.
> > 
> > ** In most cases, we respond 503 Service Unavailable; this is open
> >for discussion.  ca-authority-find and ca-authority-show include
> >a boolean field indicating whether the CA is ready to sign.
> >There might be (probably are) endpoints I've missed.
> > 
> > Cheers,
> > Fraser
> > 
> Updated patches attached - small change in patch 0084 to fix a race
> condition when deleting an authority that can cause NPE.
> 
> Thanks,
> Fraser
> ___
> Pki-devel mailing list
> Pki-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel

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


[Pki-devel] [PATCH] pki-cfu-0116-Ticket-1006-Audit-logging-for-TPS-REST-operations.patch

2016-03-24 Thread Christina Fu

Attached please find the patch for ticket 1006:
https://fedorahosted.org/pki/ticket/1006 Audit logging for TPS REST 
operations


Most of the work is on
1. finding the right places to place the audit calls
2. deciding on what should be audited: since all read operations are 
captured by AUTZ, the REST operations audited are only write operations

3. deciding on the audit events that should be provided for the operations
4. making needed information available at the places where auditing is 
happening


thanks
Christina
From b739dd8935499e833655be5d05a8002df07cc8e9 Mon Sep 17 00:00:00 2001
From: Christina Fu 
Date: Thu, 24 Mar 2016 16:23:05 -0700
Subject: [PATCH] Ticket #1006 Audit logging for TPS REST operations

---
 .../src/com/netscape/certsrv/logging/IAuditor.java |   3 +-
 .../com/netscape/cms/servlet/base/PKIService.java  |  15 ++
 .../org/dogtagpki/server/rest/AuditService.java| 117 ++--
 base/server/cmsbundle/src/LogMessages.properties   |  55 +++-
 .../src/com/netscape/cmscore/logging/Auditor.java  |  19 +-
 base/tps/shared/conf/CS.cfg.in |   4 +-
 .../dogtagpki/server/tps/config/ConfigService.java |  30 ++-
 .../server/tps/rest/AuthenticatorService.java  | 187 ++---
 .../server/tps/rest/ConnectorService.java  | 180 ++---
 .../server/tps/rest/ProfileMappingService.java | 157 +--
 .../dogtagpki/server/tps/rest/ProfileService.java  | 171 ++--
 .../dogtagpki/server/tps/rest/TokenService.java| 297 +
 12 files changed, 1028 insertions(+), 207 deletions(-)

diff --git a/base/common/src/com/netscape/certsrv/logging/IAuditor.java b/base/common/src/com/netscape/certsrv/logging/IAuditor.java
index a9362259633e29d33f5ae8fc3efc86628c9ebe35..89c03b838ca2e546224bfd2b3785c6684b59aeed 100644
--- a/base/common/src/com/netscape/certsrv/logging/IAuditor.java
+++ b/base/common/src/com/netscape/certsrv/logging/IAuditor.java
@@ -70,9 +70,10 @@ public interface IAuditor {
  * @return a delimited string of one or more delimited name/value pairs
  */
 public String getParamString(String scope, String type, String id, Map params);
+public String getParamString(StringBuilder paramsters, Map params);
 
 /**
  * Log audit message.
  */
 public void log(String message);
-}
\ No newline at end of file
+}
diff --git a/base/server/cms/src/com/netscape/cms/servlet/base/PKIService.java b/base/server/cms/src/com/netscape/cms/servlet/base/PKIService.java
index d2e55b5a3c42ee89f5784da12de860fd4e8880bf..7ed9c0dc86b08add5066a9c6df29ce791775e54e 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/base/PKIService.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/base/PKIService.java
@@ -259,6 +259,21 @@ public class PKIService {
 auditor.log(auditMessage);
 }
 
+public void auditConfigTokenGeneral(String status, String service, Map params, String info) {
+CMS.debug("PKIService.auditConfigTokenGeneral begins");
+
+String msg = CMS.getLogMessage(
+"LOGGING_SIGNED_AUDIT_CONFIG_TOKEN_GENERAL_5",
+servletRequest.getUserPrincipal().getName(),
+status,
+service,
+auditor.getParamString(null, params),
+info);
+auditor.log(msg);
+
+CMS.debug("PKIService.auditConfigTokenGeneral ends");
+}
+
 /**
  * Get the values of the fields annotated with @FormParam.
  */
diff --git a/base/server/cms/src/org/dogtagpki/server/rest/AuditService.java b/base/server/cms/src/org/dogtagpki/server/rest/AuditService.java
index e32c36c3313e8bbb3ea729245dcd51d5f2fda7a7..273625e81776ab8a8cd024f503f5e45ade94612a 100644
--- a/base/server/cms/src/org/dogtagpki/server/rest/AuditService.java
+++ b/base/server/cms/src/org/dogtagpki/server/rest/AuditService.java
@@ -21,6 +21,7 @@ package org.dogtagpki.server.rest;
 import java.io.UnsupportedEncodingException;
 import java.net.URI;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.TreeMap;
 import java.util.TreeSet;
@@ -42,6 +43,7 @@ import com.netscape.certsrv.base.IConfigStore;
 import com.netscape.certsrv.base.PKIException;
 import com.netscape.certsrv.logging.AuditConfig;
 import com.netscape.certsrv.logging.AuditResource;
+import com.netscape.certsrv.logging.ILogger;
 import com.netscape.cms.servlet.base.PKIService;
 
 /**
@@ -66,29 +68,62 @@ public class AuditService extends PKIService implements AuditResource {
 }
 
 public AuditConfig createAuditConfig() throws UnsupportedEncodingException, EBaseException {
+return createAuditConfig(null);
+}
+
+public AuditConfig createAuditConfig(Map auditParams)
+throws UnsupportedEncodingException, EBaseException {
 
 IConfigStore cs = CMS.getConfigStore();
 
 AuditConfig auditConfig = new AuditConfig();
-auditConfig.setStatus(cs.getBoolean("log.instance.SignedAudit.enable", false