[Pki-devel] [PATCH] 721-724 Fixed activity logs for certificate revocations.

[Endi Sukma Dewata]

Attached are some patches to fix the activity logs for token certificate 
revocations. The code had to be refactored to reduce the complexity.


--
Endi S. Dewata
>From 27659dae3a1fb34ae71d5f8d1038bdcc9a821f21 Mon Sep 17 00:00:00 2001
From: "Endi S. Dewata" 
Date: Fri, 22 Apr 2016 01:37:37 +0200
Subject: [PATCH] Added TPSCertRecord.getSerialNumberInBigInteger().

The code that parses the token certificate serial number has been
refactored into a new method in TPSCertRecord.
---
 .../src/org/dogtagpki/server/tps/TPSTokendb.java   | 27 --
 .../dogtagpki/server/tps/dbs/TPSCertRecord.java| 13 +++
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/base/tps/src/org/dogtagpki/server/tps/TPSTokendb.java b/base/tps/src/org/dogtagpki/server/tps/TPSTokendb.java
index 4142bab4fe1cedcd55102f5bd88bec0a0876da02..dcb3bc1c221ecd3b31cf8af72df3787870bbcbf7 100644
--- a/base/tps/src/org/dogtagpki/server/tps/TPSTokendb.java
+++ b/base/tps/src/org/dogtagpki/server/tps/TPSTokendb.java
@@ -509,24 +509,15 @@ public class TPSTokendb {
 }
 }
 
-CARemoteRequestHandler caRH = null;
-caRH = new CARemoteRequestHandler(connID);
-String hexSerial = cert.getSerialNumber();
-if (hexSerial.length() >= 3 && hexSerial.startsWith("0x")) {
-String serial = hexSerial.substring(2); // skip over the '0x'
-BigInteger bInt = new BigInteger(serial, 16);
-String serialStr = bInt.toString();
-CMS.debug(method + ": found cert hex serial: " + serial +
-" dec serial:" + serialStr);
-CARevokeCertResponse response =
-caRH.revokeCertificate(isRevoke, serialStr, cert.getCertificate(),
-revokeReason);
-CMS.debug(method + ": response status =" + response.getStatus());
-} else {
-logMsg = "mulformed hex serial number :" + hexSerial;
-CMS.debug(method + ": " + logMsg);
-throw new Exception(logMsg);
-}
+CARemoteRequestHandler caRH = new CARemoteRequestHandler(connID);
+BigInteger bInt = cert.getSerialNumberInBigInteger();
+String serialStr = bInt.toString();
+CMS.debug(method + ": found cert hex serial: " + cert.getSerialNumber() +
+" dec serial: " + serialStr);
+CARevokeCertResponse response =
+caRH.revokeCertificate(isRevoke, serialStr, cert.getCertificate(),
+revokeReason);
+CMS.debug(method + ": response status: " + response.getStatus());
 
 // update certificate status
 if (isRevoke) {
diff --git a/base/tps/src/org/dogtagpki/server/tps/dbs/TPSCertRecord.java b/base/tps/src/org/dogtagpki/server/tps/dbs/TPSCertRecord.java
index 288f25f53518d713500fdb01ca802d7d6d74a8b0..0f846c6ded6297dffed3d007c9ac8b96ae232599 100644
--- a/base/tps/src/org/dogtagpki/server/tps/dbs/TPSCertRecord.java
+++ b/base/tps/src/org/dogtagpki/server/tps/dbs/TPSCertRecord.java
@@ -18,6 +18,7 @@
 
 package org.dogtagpki.server.tps.dbs;
 
+import java.math.BigInteger;
 import java.util.Date;
 
 import com.netscape.cmscore.dbs.DBAttribute;
@@ -68,6 +69,18 @@ public class TPSCertRecord extends DBRecord {
 this.serialNumber = serialNumber;
 }
 
+public BigInteger getSerialNumberInBigInteger()  {
+
+if (serialNumber == null) return null;
+
+if (serialNumber.length() < 3 || !serialNumber.startsWith("0x")) {
+throw new NumberFormatException("Malformed hex serial number: " + serialNumber);
+}
+
+String value = serialNumber.substring(2); // skip over the '0x'
+return new BigInteger(value, 16);
+}
+
 @DBAttribute("tokenSubject")
 public String getSubject() {
 return subject;
-- 
2.5.5

>From 15f0af6401f9b3f00e69f5e6371ebc491c5fdbcb Mon Sep 17 00:00:00 2001
From: "Endi S. Dewata" 
Date: Fri, 22 Apr 2016 02:48:01 +0200
Subject: [PATCH] Moved TPSTokendb.tdbGetTokenEntry() invocations.

The TPSTokendb.tdbGetTokenEntry() invocations in shouldRevoke()
have been moved into revokeCertsByCUID().
---
 .../src/org/dogtagpki/server/tps/TPSTokendb.java   | 30 ++
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/base/tps/src/org/dogtagpki/server/tps/TPSTokendb.java b/base/tps/src/org/dogtagpki/server/tps/TPSTokendb.java
index dcb3bc1c221ecd3b31cf8af72df3787870bbcbf7..f50fd46f41ce4f24795e48fd660be031ead10e9f 100644
--- a/base/tps/src/org/dogtagpki/server/tps/TPSTokendb.java
+++ b/base/tps/src/org/dogtagpki/server/tps/TPSTokendb.java
@@ -379,7 +379,7 @@ public class TPSTokendb {
 return true;
 }
 
-private boolean shouldRevoke(TPSCertRecord cert, String cuid, String tokenReason,
+private boolean shouldRevoke(TokenRecord tokenRecord, TPSCertRecord cert, 

Re: [Pki-devel] [PATCH] 720 Replaced TPS OP_DO_TOKEN activity.

[Endi Sukma Dewata]

On 4/21/2016 11:12 AM, Christina Fu wrote:

In general, the idea is good to use something more descriptive than
"OP_DO_TOKEN" in activity report, however, I think "OP_DO_TOKEN" was
intended for any activities that were performed on the token records, so
perhaps "OP_TOKEN_*" (e.g. OP_TOKEN_MODIFY) instead of "OP_*" would be
more appropriate?

Also, in TPSTokendb.java, where the three activities report that
revocation is not permitted, why do they have "success".  Shouldn't they
be "failure" since the op was not allowed to happen?  I know you didn't
touch that code, but I'm just wondering.


Christina


The patch has been revised as described, ACKed over IRC (thanks!), and 
pushed to master.


The activity logs are indeed incorrect and they will be fixed in 
subsequent patches.


--
Endi S. Dewata

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

[Pki-devel] Karma request for Dogtag 10.2.6 on Fedora 23

[Matthew Harmsen]

The following ticket has been addressed in Fedora 23:

 * PKI TRAC Ticket #2022 - pkispawn ignores 3rd party CA certs in
   pki_clone_pkcs12_path (python hash fix)
   

by the following build:

 * pki-core-10.2.6-19.fc23
   

Please provide Karma for this build in Bodhi located at:

 * pki-core-10.2.6-19.fc23
   

Thanks,
-- Matt

___
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

[Fraser Tweedale]

On Thu, Apr 21, 2016 at 10:29:32AM -0400, Ade Lee wrote:
> ACK on latest 96 and 99.
> 
> I will ask  cfu or jmagne to look at the KeyRetrieveRunner logic today.
> 
> Ade
> 
Thanks Ade; I'll wait for their feedback before I merge it.

Cheers,
Fraser

> On Thu, 2016-04-21 at 14:58 +1000, Fraser Tweedale wrote:
> > Thanks Ade.  Updated patch 0096 attached.  Comments inline.
> > 
> > On Wed, Apr 20, 2016 at 11:30:52AM -0400, Ade Lee wrote:
> > > Comments:
> > > 
> > > 95 - ack
> > > 
> > > 96 -
> > > 
> > > 1. You have made the return type of initSigUnit() to be boolean. 
> > >  Should you be checking the return value in init()?
> > > 
> > It is not needed to check it here; only when re-entering init from
> > the KeyReplicatorRunner thread.
> > 
> > > 2. In addInstanceToAuthorityKeyHosts(), you are still using only
> > > the
> > > hostname.  Should be host:port
> > > 
> > Good pickup.  Fixed in latest patch.
> > 
> > > 3. The logic in the KeyRetrieverRunner class looks OK to me, but
> > > I'd
> > > like cfu and/or jmagne to check it and make sure we are calling the
> > > right primitives to wrap/unwrap inside the cryptographic token.
> > > 
> > > Also I'd like them to confirm that this would wor for an HSM.
> > > Statements like the following make me question that:
> > >CryptoToken token = manager.getInternalKeyStorageToken()
> > > 
> > It won't work on HSM.  Can I get an HSM to test with? ;) I've filed
> > a ticket for HSM support[1].  FreeIPA does not yet support HSM[2] so
> > I think we can put it in 10.4 milestone (I've put it there for now).
> > 
> > [1] https://fedorahosted.org/pki/ticket/2292
> > [2] https://fedorahosted.org/freeipa/ticket/5608
> > 
> > > 4. Can you explain what happens if for some reason the script fails
> > > to
> > > retrieve the key?  Do we end up retrying later and if so, when?
> > > 
> > If the script fails to retrieve the key, it does not retry
> > automatically.  I filed a ticket[3] to implement retry with
> > backoff (this patchset is big enough already!) and put it in
> > 10.3.1 milestone (that's up for discussion).
> > 
> > [3] https://fedorahosted.org/pki/ticket/2293
> > 
> > Right now, the following events cause authority reinitialisation,
> > entailing key retrieval if necessary:
> > 
> > - Dogtag is restarted
> > - LDAP disconnect-reconnect
> > - LDAP modification of authority replicated from another clone
> > 
> > > 97- ACK
> > > 
> > > 98 - ACK
> > >  
> > Thanks.  Any feedback on patch 0099?

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

Re: [Pki-devel] [PATCH] 720 Replaced TPS OP_DO_TOKEN activity.

[Christina Fu]

In general, the idea is good to use something more descriptive than 
"OP_DO_TOKEN" in activity report, however, I think "OP_DO_TOKEN" was 
intended for any activities that were performed on the token records, so 
perhaps "OP_TOKEN_*" (e.g. OP_TOKEN_MODIFY) instead of "OP_*" would be 
more appropriate?


Also, in TPSTokendb.java, where the three activities report that 
revocation is not permitted, why do they have "success".  Shouldn't they 
be "failure" since the op was not allowed to happen?  I know you didn't 
touch that code, but I'm just wondering.



Christina


On 04/18/2016 09:38 AM, Endi Sukma Dewata wrote:

For clarity the TPS operatons that generate OP_DO_TOKEN activity
has been modified to generate OP_MODIFY instead, except for the
changeTokenStatus() which will generate OP_STATUS_CHANGE.

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



___
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

Re: [Pki-devel] Trac; add "Lightweight CAs" feature?

[Matthew Harmsen]

On 04/20/2016 10:54 PM, Fraser Tweedale wrote:

Hi all,

Could someone with the relevant permissions please add a
"Lightweight CAs" feature to the pki trac?  There's a substantial
quantity of outstanding tickets for this feature so it would be good
to have something more formal than the summary by which to group
them.

Thanks,
Fraser

Done.

___
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

[Ade Lee]

ACK on latest 96 and 99.

I will ask  cfu or jmagne to look at the KeyRetrieveRunner logic today.

Ade

On Thu, 2016-04-21 at 14:58 +1000, Fraser Tweedale wrote:
> Thanks Ade.  Updated patch 0096 attached.  Comments inline.
> 
> On Wed, Apr 20, 2016 at 11:30:52AM -0400, Ade Lee wrote:
> > Comments:
> > 
> > 95 - ack
> > 
> > 96 -
> > 
> > 1. You have made the return type of initSigUnit() to be boolean. 
> >  Should you be checking the return value in init()?
> > 
> It is not needed to check it here; only when re-entering init from
> the KeyReplicatorRunner thread.
> 
> > 2. In addInstanceToAuthorityKeyHosts(), you are still using only
> > the
> > hostname.  Should be host:port
> > 
> Good pickup.  Fixed in latest patch.
> 
> > 3. The logic in the KeyRetrieverRunner class looks OK to me, but
> > I'd
> > like cfu and/or jmagne to check it and make sure we are calling the
> > right primitives to wrap/unwrap inside the cryptographic token.
> > 
> > Also I'd like them to confirm that this would wor for an HSM.
> > Statements like the following make me question that:
> >CryptoToken token = manager.getInternalKeyStorageToken()
> > 
> It won't work on HSM.  Can I get an HSM to test with? ;) I've filed
> a ticket for HSM support[1].  FreeIPA does not yet support HSM[2] so
> I think we can put it in 10.4 milestone (I've put it there for now).
> 
> [1] https://fedorahosted.org/pki/ticket/2292
> [2] https://fedorahosted.org/freeipa/ticket/5608
> 
> > 4. Can you explain what happens if for some reason the script fails
> > to
> > retrieve the key?  Do we end up retrying later and if so, when?
> > 
> If the script fails to retrieve the key, it does not retry
> automatically.  I filed a ticket[3] to implement retry with
> backoff (this patchset is big enough already!) and put it in
> 10.3.1 milestone (that's up for discussion).
> 
> [3] https://fedorahosted.org/pki/ticket/2293
> 
> Right now, the following events cause authority reinitialisation,
> entailing key retrieval if necessary:
> 
> - Dogtag is restarted
> - LDAP disconnect-reconnect
> - LDAP modification of authority replicated from another clone
> 
> > 97- ACK
> > 
> > 98 - ACK
> >  
> Thanks.  Any feedback on patch 0099?

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