Re: [Pki-devel] [pki-devel][PATCH] 0098-SCP03-support-fix-Key-Changeover-with-HSM-RHCS.patch

2017-06-29 Thread Christina Fu

looks good. ACK.

Christina


On 06/29/2017 03:43 PM, John Magne wrote:

[PATCH] SCP03 support: fix Key Changeover with HSM (RHCS)

Ticket #2764.

This relatively simple fix involves making sure the correct crypto token is 
being used to search for the master key int the case of symmetric key changover 
where the master key resides on an HSM.


___
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] [pki-devel][PATCH] 0095-Resolve-1663-Add-SCP03-support.patch

2017-06-02 Thread John Magne
PUshed to master:

commit a614eb15476adb00df571d3ea05fdd8ea282141d
Author: Jack Magne <jma...@dhcp-16-206.sjc.redhat.com>
Date:   Fri Jun 2 15:40:52 2017 -0700

Resolve  #1663 Add SCP03 support .

This particular fix resolves a simple issue when formatting a token in FIPS 
mode for SCP03.


- Original Message -
From: "Matthew Harmsen" <mharm...@redhat.com>
To: "John Magne" <jma...@redhat.com>, "pki-devel" <pki-devel@redhat.com>
Sent: Friday, June 2, 2017 4:01:14 PM
Subject: Re: [Pki-devel] [pki-devel][PATCH] 
0095-Resolve-1663-Add-SCP03-support.patch

On 06/02/2017 04:44 PM, John Magne wrote:
>
>
>
> Ticket: Resolve  #1663 Add SCP03 support .
>  
>  This particular fix resolves a simple issue when formatting a token in 
> FIPS mode for SCP03.
>
>
> ___
> Pki-devel mailing list
> Pki-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel

Confirmed that import statements were removed by Eclipse, and that 
commented out block of code is there for future testing.

As jmagne confirmed that this had been tested (including on the 
offending machine configuration) --- ACK

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


Re: [Pki-devel] [pki-devel][PATCH] 0095-Resolve-1663-Add-SCP03-support.patch

2017-06-02 Thread Matthew Harmsen

On 06/02/2017 04:44 PM, John Magne wrote:




Ticket: Resolve  #1663 Add SCP03 support .
 
 This particular fix resolves a simple issue when formatting a token in FIPS mode for SCP03.



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


Confirmed that import statements were removed by Eclipse, and that 
commented out block of code is there for future testing.


As jmagne confirmed that this had been tested (including on the 
offending machine configuration) --- ACK


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

[Pki-devel] [pki-devel][PATCH] Non server keygen issue in SCP03.

2017-05-05 Thread John Magne
[PATCH] Non server keygen issue in SCP03.

Ticket 1663 Add SCP03 support: https://pagure.io/dogtagpki/issue/1663

We discovered a minor issue when trying to log values that don't exist when 
performing the non server side keygen case. For instance , we don't need to 
generate a kek session key in this case, and we were trying to print info about 
it to the logs. This fix allows this case to work without issue.
From d58e929de707ad5139c57cd493fae5485ca3acae Mon Sep 17 00:00:00 2001
From: Jack Magne 
Date: Fri, 5 May 2017 11:44:17 -0700
Subject: [PATCH] Non server keygen issue in SCP03.

Ticket 1663 Add SCP03 support: https://pagure.io/dogtagpki/issue/1663

We discovered a minor issue when trying to log values that don't exist when performing the non server side keygen case. For instance , we don't need to generate a kek session key in this case, and we were trying to print info about it to the logs. This fix allows this case to work without issue.
---
 .../server/tps/channel/SecureChannel.java  |  4 +-
 .../server/tps/processor/TPSProcessor.java | 51 +++---
 2 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/base/tps/src/org/dogtagpki/server/tps/channel/SecureChannel.java b/base/tps/src/org/dogtagpki/server/tps/channel/SecureChannel.java
index fc5472c..5e5646b 100644
--- a/base/tps/src/org/dogtagpki/server/tps/channel/SecureChannel.java
+++ b/base/tps/src/org/dogtagpki/server/tps/channel/SecureChannel.java
@@ -148,8 +148,8 @@ public class SecureChannel {
 
 CMS.debug("SecureChannel.SecureChannel: For SCP03. :  ");
 
-CMS.debug("kekDesKey: " + kekDesKey.toHexString());
-CMS.debug("keyCheck: " + keyCheck.toHexString());
+if (keyCheck != null)
+CMS.debug("keyCheck: " + keyCheck.toHexString());
 
 this.platProtInfo = platformInfo;
 this.processor = processor;
diff --git a/base/tps/src/org/dogtagpki/server/tps/processor/TPSProcessor.java b/base/tps/src/org/dogtagpki/server/tps/processor/TPSProcessor.java
index 0cfac59..0f96915 100644
--- a/base/tps/src/org/dogtagpki/server/tps/processor/TPSProcessor.java
+++ b/base/tps/src/org/dogtagpki/server/tps/processor/TPSProcessor.java
@@ -33,6 +33,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import netscape.security.x509.RevocationReason;
+
 import org.dogtagpki.server.tps.TPSSession;
 import org.dogtagpki.server.tps.TPSSubsystem;
 import org.dogtagpki.server.tps.authentication.AuthUIParameter;
@@ -100,8 +102,6 @@ import com.netscape.cms.servlet.tks.SecureChannelProtocol;
 import com.netscape.cmsutil.crypto.CryptoUtil;
 import com.netscape.symkey.SessionKey;
 
-import netscape.security.x509.RevocationReason;
-
 public class TPSProcessor {
 
 public static final int RESULT_NO_ERROR = 0;
@@ -923,20 +923,39 @@ public class TPSProcessor {
 TPSBuffer drmDesKeyBuff = resp.getDRM_Trans_DesKey();
 TPSBuffer kekDesKeyBuff = resp.getKekWrappedDesKey();
 
-CMS.debug(method + " encSessionKeyBuff: " + encSessionKeyBuff.toHexString());
-CMS.debug(method + " kekSessionKeyBuff: " + kekSessionKeyBuff.toHexString());
-CMS.debug(method + " macSessionKeyBuff: " + macSessionKeyBuff.toHexString());
-CMS.debug(method + " hostCryptogramBuff: " + hostCryptogramBuff.toHexString());
-CMS.debug(method + " keyCheckBuff: " + keyCheckBuff.toHexString());
-CMS.debug(method + " drmDessKeyBuff: " + drmDesKeyBuff.toHexString());
-CMS.debug(method + " kekDesKeyBuff: " + kekDesKeyBuff.toHexString());
-
-encSessionKeySCP03 = (PK11SymKey) protocol.unwrapWrappedSymKeyOnToken(token, sharedSecret,
-encSessionKeyBuff.toBytesArray(), false, SymmetricKey.AES);
-macSessionKeySCP03 = (PK11SymKey) protocol.unwrapWrappedSymKeyOnToken(token, sharedSecret,
-macSessionKeyBuff.toBytesArray(), false, SymmetricKey.AES);
-kekSessionKeySCP03 = (PK11SymKey) protocol.unwrapWrappedSymKeyOnToken(token, sharedSecret,
-kekSessionKeyBuff.toBytesArray(), false, SymmetricKey.AES);
+if (encSessionKeyBuff != null)
+CMS.debug(method + " encSessionKeyBuff: " + encSessionKeyBuff.toHexString());
+
+if (kekSessionKeyBuff != null)
+CMS.debug(method + " kekSessionKeyBuff: " + kekSessionKeyBuff.toHexString());
+
+if (macSessionKeyBuff != null)
+CMS.debug(method + " macSessionKeyBuff: " + macSessionKeyBuff.toHexString());
+
+if (hostCryptogramBuff != null)
+CMS.debug(method + " hostCryptogramBuff: " + hostCryptogramBuff.toHexString());
+
+if (keyCheckBuff != null)
+CMS.debug(method + " keyCheckBuff: " + keyCheckBuff.toHexString());
+
+if (drmDesKeyBuff != null)
+CMS.debug(method + " drmDessKeyBuff: " + 

[Pki-devel] [pki-devel][PATCH]

2017-04-26 Thread John Magne

CA in the certificate profiles the startTime parameter is not working as 
expected.

This simple fix addresses an overflow in the "startTime" paramenter in 4 
places in the code. I felt that honing in only on the startTime value was the 
best way to go. In some of the files other than ValidityDefault.java, there 
were possibly some values that could be changed from int to long. Due to the 
complexity of some of the calculations involved in some of those cases, it is 
best to fix the exact issue at hand instead of introducing some other possible 
side effects.

Tested with a simple enrollment in the caUserCert profile by setting the 
startTime constraint to the offending value listed in the ticket/bug. The 
correct start time 30 days in the future was calculated and made part of the 
cert.


Issue:

https://pagure.io/dogtagpki/issue/2520From 91d7f82be94532a691768021a0661efd6a93e093 Mon Sep 17 00:00:00 2001
From: Jack Magne 
Date: Wed, 26 Apr 2017 15:21:39 -0700
Subject: [PATCH] CA in the certificate profiles the startTime parameter is not
 working as expected.

This simple fix addresses an overflow in the "startTime" paramenter in 4 places in the code. I felt that honing in only on the startTime value was the best way to go. In some of the files other than ValidityDefault.java, there were possibly some values that could be changed from int to long. Due to the complexity of some of the calculations involved in some of those cases, it is best to fix the exact issue at hand instead of introducing some other possible side effects.
---
 .../src/com/netscape/cms/profile/def/CAValidityDefault.java  | 12 ++--
 .../cms/profile/def/PrivateKeyUsagePeriodExtDefault.java |  4 ++--
 .../netscape/cms/profile/def/RandomizedValidityDefault.java  |  2 +-
 .../src/com/netscape/cms/profile/def/ValidityDefault.java| 10 +-
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/base/server/cms/src/com/netscape/cms/profile/def/CAValidityDefault.java b/base/server/cms/src/com/netscape/cms/profile/def/CAValidityDefault.java
index 2df256e..2ecd484 100644
--- a/base/server/cms/src/com/netscape/cms/profile/def/CAValidityDefault.java
+++ b/base/server/cms/src/com/netscape/cms/profile/def/CAValidityDefault.java
@@ -24,6 +24,11 @@ import java.util.Calendar;
 import java.util.Date;
 import java.util.Locale;
 
+import netscape.security.x509.BasicConstraintsExtension;
+import netscape.security.x509.CertificateValidity;
+import netscape.security.x509.PKIXExtensions;
+import netscape.security.x509.X509CertInfo;
+
 import com.netscape.certsrv.apps.CMS;
 import com.netscape.certsrv.base.IConfigStore;
 import com.netscape.certsrv.ca.ICertificateAuthority;
@@ -34,11 +39,6 @@ import com.netscape.certsrv.property.EPropertyException;
 import com.netscape.certsrv.property.IDescriptor;
 import com.netscape.certsrv.request.IRequest;
 
-import netscape.security.x509.BasicConstraintsExtension;
-import netscape.security.x509.CertificateValidity;
-import netscape.security.x509.PKIXExtensions;
-import netscape.security.x509.X509CertInfo;
-
 /**
  * This class implements a CA signing cert enrollment default policy
  * that populates a server-side configurable validity
@@ -348,7 +348,7 @@ public class CAValidityDefault extends EnrollDefault {
 if (startTimeStr == null || startTimeStr.equals("")) {
 startTimeStr = "60";
 }
-int startTime = Integer.parseInt(startTimeStr);
+long startTime = Long.parseLong(startTimeStr);
 
 Date notBefore = new Date(CMS.getCurrentDate().getTime() + (1000 * startTime));
 CMS.debug("CAValidityDefault: not before: " + notBefore);
diff --git a/base/server/cms/src/com/netscape/cms/profile/def/PrivateKeyUsagePeriodExtDefault.java b/base/server/cms/src/com/netscape/cms/profile/def/PrivateKeyUsagePeriodExtDefault.java
index 6532a13..2f05f32 100644
--- a/base/server/cms/src/com/netscape/cms/profile/def/PrivateKeyUsagePeriodExtDefault.java
+++ b/base/server/cms/src/com/netscape/cms/profile/def/PrivateKeyUsagePeriodExtDefault.java
@@ -296,13 +296,13 @@ public class PrivateKeyUsagePeriodExtDefault extends EnrollExtDefault {
 if (startTimeStr == null || startTimeStr.equals("")) {
 startTimeStr = "60";
 }
-int startTime = Integer.parseInt(startTimeStr);
+long startTime = Long.parseLong(startTimeStr);
 Date notBefore = new Date(CMS.getCurrentDate().getTime() +
 (1000 * startTime));
 long notAfterVal = 0;
 
 notAfterVal = notBefore.getTime() +
-(mDefault * Integer.parseInt(getConfig(CONFIG_DURATION)));
+(mDefault * Long.parseLong(getConfig(CONFIG_DURATION)));
 Date notAfter = new Date(notAfterVal);
 
 ext = new PrivateKeyUsageExtension(notBefore, notAfter);
diff --git 

Re: [Pki-devel] [pki-devel][PATCH] 0091-SCP03 support for g 7 card.patch

2017-04-10 Thread Christina Fu

looks fine.

ack.

Christina


On 03/29/2017 11:22 AM, John Magne wrote:

[PATCH] SCP03 support for g sc 7 card.

Ticket:

https://pagure.io/dogtagpki/issue/1663 Add SCP03 support


This allows the use of the g 7 card.
This will require the following:

1. An out of band method is needed to generate an AES based master key.
We do not as of yet have support with tkstool for this:

Ex:

/usr/lib64/nss/unsupported-tools/symkeyutil -d . -K -n new_master_aes -t aes -s 
16

2. There are some new config params that can be adjusted to support either the 
6.0 or 7.0 cards:

Ex:

tks.defKeySet._005=## tks.prot3   , protocol 3 specific settings
tks.defKeySet._006=## divers= emv,visa2 : Values for the master key case, or > 
version one.
tks.defKeySet._007=## diversVer1 = emv,visa2, or none. This is for developer or 
version one keyset
tks.defKeySet._008=## devKeyType = DES3or AES. This is for the key type of 
developer or version one keys.
tks.defKeySet._009=## masterKeyType = DES3 or AES. This is for the type of key 
for the master key.
tks.defKeySet._010=##
tks.defKeySet._011=## Only supports two tokens now: G Smart Cafe 6 and Smart 
Cafe 7, use these exact settings
tks.defKeySet._013=## Smart Cafe 6 settings:
tks.defKeySet._014=##tks.defKeySet.prot3.divers=emv
tks.defKeySet._015=##tks.defKeySet.prot3.diversVer1Keys=emv
tks.defKeySet._016=##tks.defKeySet.prot3.devKeyType=DES3
tks.defKeySet._017=##tks.defKeySet.prot3.masterKeyType=DES3
tks.defKeySet._018=##Smart Cafe 7 settings:
tks.defKeySet._019=##tks.defKeySet.prot3.divers=none
tks.defKeySet._020=##tks.defKeySet.prot3.diversVer1Keys=none
tks.defKeySet._021=##tks.defKeySet.prot3.devKeyType=AES
tks.defKeySet._022=##tks.defKeySet.prot3.masterKeyType=AES
tks.defKeySet._023=##
tks.defKeySet._024=##


___
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] [pki-devel][PATCH] 0086-Ticket-2569-Token-memory-not-wiped-after-key-deletio.patch

2017-01-05 Thread Christina Fu
Overall, it looks good.  Just some minor suggestions, mostly for 
clarification purposes.


* SecureChannel.java : clearAppletKeySlotData

  - would appreciate comments describing the content and format 
expected in the input "data"


  - maybe a positive debug message after the successful cleanup (as 
negative result is non-fatal regardless)


* PKCS11Obj.java : getKeyIndexList

 - please add high level comment to tell what this does

 - how about go with the convention and assign a String method for 
debug messages?


 - I couldn't figure out why the code needs to traverse the cert 
objects while it has no interest in them;  I don't think it hurts 
though;  I'm okay with it if you decide to leave it in.


 - One question: if TPSBuffer data ends up not having anything add to 
it, will this reference blow up? data.toHexString()


Conditional ACK.

thanks,

Christina



On 12/16/2016 04:28 PM, John Magne wrote:

Author: Jack Magne
Date:   Fri Dec 16 16:25:48 2016 -0800

 Ticket #2569: Token memory not wiped after key deletion
 
 This is the dogtag upstream side of the TPS portion of this ticket.

 This fix also involves an applet fix, handled in another bug.


___
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] [pki-devel][PATCH] 0086-Ticket-2569-Token-memory-not-wiped-after-key-deletio.patch

2016-12-16 Thread John Magne
Author: Jack Magne 
Date:   Fri Dec 16 16:25:48 2016 -0800

Ticket #2569: Token memory not wiped after key deletion

This is the dogtag upstream side of the TPS portion of this ticket.
This fix also involves an applet fix, handled in another bug.
From 08fa0ff96d7dd6ed6c3b11527251e604b93f045a Mon Sep 17 00:00:00 2001
From: Jack Magne 
Date: Fri, 16 Dec 2016 16:25:48 -0800
Subject: [PATCH] Ticket #2569: Token memory not wiped after key deletion

This is the dogtag upstream side of the TPS portion of this ticket.
This fix also involves an applet fix, handled in another bug.
---
 base/common/src/org/dogtagpki/tps/apdu/APDU.java   |  3 +-
 .../org/dogtagpki/tps/apdu/ClearKeySlotsAPDU.java  | 24 ++
 .../server/tps/channel/SecureChannel.java  | 36 +
 .../org/dogtagpki/server/tps/main/PKCS11Obj.java   | 90 +-
 .../server/tps/processor/TPSEnrollProcessor.java   | 22 --
 5 files changed, 165 insertions(+), 10 deletions(-)
 create mode 100644 base/common/src/org/dogtagpki/tps/apdu/ClearKeySlotsAPDU.java

diff --git a/base/common/src/org/dogtagpki/tps/apdu/APDU.java b/base/common/src/org/dogtagpki/tps/apdu/APDU.java
index 390252f..009c470 100644
--- a/base/common/src/org/dogtagpki/tps/apdu/APDU.java
+++ b/base/common/src/org/dogtagpki/tps/apdu/APDU.java
@@ -57,7 +57,8 @@ public abstract class APDU {
 APDU_SET_ISSUERINFO,
 APDU_GET_ISSUERINFO,
 APDU_GENERATE_KEY_ECC,
-APDU_GET_LIFECYCLE
+APDU_GET_LIFECYCLE,
+APDU_CLEAR_KEY_SLOTS
 }
 
 protected byte cla;
diff --git a/base/common/src/org/dogtagpki/tps/apdu/ClearKeySlotsAPDU.java b/base/common/src/org/dogtagpki/tps/apdu/ClearKeySlotsAPDU.java
new file mode 100644
index 000..b3d71c4
--- /dev/null
+++ b/base/common/src/org/dogtagpki/tps/apdu/ClearKeySlotsAPDU.java
@@ -0,0 +1,24 @@
+package org.dogtagpki.tps.apdu;
+
+import org.dogtagpki.tps.main.TPSBuffer;
+
+
+public class ClearKeySlotsAPDU extends APDU {
+public ClearKeySlotsAPDU(byte[] slotList) {
+setCLA((byte) 0x84);
+setINS((byte) 0x55);
+setP1((byte) 0x0);
+setP2((byte) 0x0);
+
+data = new TPSBuffer();
+data.addBytes(slotList);
+
+}
+
+@Override
+public Type getType()
+{
+return Type.APDU_CLEAR_KEY_SLOTS;
+}
+
+}
diff --git a/base/tps/src/org/dogtagpki/server/tps/channel/SecureChannel.java b/base/tps/src/org/dogtagpki/server/tps/channel/SecureChannel.java
index 8860f48..13fb534 100644
--- a/base/tps/src/org/dogtagpki/server/tps/channel/SecureChannel.java
+++ b/base/tps/src/org/dogtagpki/server/tps/channel/SecureChannel.java
@@ -23,6 +23,7 @@ import org.dogtagpki.server.tps.engine.TPSEngine;
 import org.dogtagpki.server.tps.processor.TPSProcessor;
 import org.dogtagpki.tps.apdu.APDU;
 import org.dogtagpki.tps.apdu.APDUResponse;
+import org.dogtagpki.tps.apdu.ClearKeySlotsAPDU;
 import org.dogtagpki.tps.apdu.CreateObjectAPDU;
 import org.dogtagpki.tps.apdu.CreatePinAPDU;
 import org.dogtagpki.tps.apdu.DeleteFileAPDU;
@@ -850,6 +851,41 @@ public class SecureChannel {
 return keyInfoData;
 }
 
+//Call the applet to clear unused key slots
+public void clearAppletKeySlotData(TPSBuffer data) {
+String method = "SecureChannel.clearAppletKeySlotData: ";
+
+CMS.debug(method + " entering ...");
+
+if(data == null) {
+CMS.debug(method + " Invalid input data returning...");
+return;
+}
+
+
+
+   // CMS.debug(method + " apdu sending: " + clearKey.getEncoding().toHexString());
+
+
+
+APDUResponse response;
+try {
+
+ClearKeySlotsAPDU  clearKey = new ClearKeySlotsAPDU(data.toBytesArray());
+computeAPDU(clearKey);
+response = processor.handleAPDURequest(clearKey);
+} catch (TPSException | IOException e) {
+CMS.debug(method + " bad apdu return!");
+return;
+
+}
+
+if (!response.checkResult()) {
+CMS.debug(method + " bad apdu return!");
+}
+
+}
+
 public void writeObject(TPSBuffer objectID, TPSBuffer objectData) throws TPSException, IOException {
 CMS.debug("SecureChannel.writeObject: entering ...");
 
diff --git a/base/tps/src/org/dogtagpki/server/tps/main/PKCS11Obj.java b/base/tps/src/org/dogtagpki/server/tps/main/PKCS11Obj.java
index 6af39a7..ccfcbb7 100644
--- a/base/tps/src/org/dogtagpki/server/tps/main/PKCS11Obj.java
+++ b/base/tps/src/org/dogtagpki/server/tps/main/PKCS11Obj.java
@@ -265,6 +265,92 @@ public class PKCS11Obj {
 
 }
 
+public TPSBuffer getKeyIndexList() {
+
+TPSBuffer data = new TPSBuffer();
+
+
+int objectCount = getObjectSpecCount();
+
+
+CMS.debug("PKCS11Obj:getKeyIndexList: objectCount: " + objectCount);
+
+  //Add first byte for length, set to 0 for now
+
+byte keyListCount = 0;
+
+

Re: [Pki-devel] [pki-devel][PATCH] 0086-Resolve-pkispawn-does-not-change-default-ecc-key-siz.patch

2016-12-08 Thread Matthew Harmsen

On 12/08/2016 05:42 PM, John Magne wrote:

Simple patch will provide a fix to this issue.


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


Tested original code to confirm incorrect ECC signing curve; tested 
patched code to confirm correct ECC signing curve.


ACK

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

[Pki-devel] [pki-devel][PATCH] 0086-Resolve-pkispawn-does-not-change-default-ecc-key-siz.patch

2016-12-08 Thread John Magne

Simple patch will provide a fix to this issue.From e7821b4061d22d23013f7d00c066fc6e59d83167 Mon Sep 17 00:00:00 2001
From: Jack Magne 
Date: Thu, 8 Dec 2016 16:35:20 -0800
Subject: [PATCH] Resolve: pkispawn does not change default ecc key size from
 nistp256 when nistp384 is specified in spawn config

Ticket #2552.

This fix turned out simple. The client was correctly setting the required data, but it was putting the curveName in the
"keySize" field of the SystemCertData object sent to the back end. The configuration routine was trying to find the name in the "curveName" field when its really in the "keySize" field. This issue is restricted to the ECC case. It is fine to simply fix this in the server, since the "keySize" is a string anyway and it makes decent sense.
---
 .../cms/src/org/dogtagpki/server/rest/SystemConfigService.java| 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/base/server/cms/src/org/dogtagpki/server/rest/SystemConfigService.java b/base/server/cms/src/org/dogtagpki/server/rest/SystemConfigService.java
index 2f9d0d6..40f4b58 100644
--- a/base/server/cms/src/org/dogtagpki/server/rest/SystemConfigService.java
+++ b/base/server/cms/src/org/dogtagpki/server/rest/SystemConfigService.java
@@ -34,6 +34,8 @@ import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.Request;
 import javax.ws.rs.core.UriInfo;
 
+import netscape.security.x509.X509CertImpl;
+
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.mutable.MutableBoolean;
 import org.mozilla.jss.CryptoManager;
@@ -66,8 +68,6 @@ import com.netscape.cms.servlet.csadmin.SystemCertDataFactory;
 import com.netscape.cmsutil.crypto.CryptoUtil;
 import com.netscape.cmsutil.util.Utils;
 
-import netscape.security.x509.X509CertImpl;
-
 /**
  * @author alee
  *
@@ -453,8 +453,8 @@ public class SystemConfigService extends PKIService implements SystemConfigResou
 
 } else if (!request.getStepTwo()) {
 if (keytype.equals("ecc")) {
-String curvename = certData.getKeyCurveName() != null ?
-certData.getKeyCurveName() : cs.getString("keys.ecc.curve.default");
+String curvename = certData.getKeySize() != null ?
+certData.getKeySize() : cs.getString("keys.ecc.curve.default");
 cs.putString("preop.cert." + tag + ".curvename.name", curvename);
 ConfigurationUtils.createECCKeyPair(token, curvename, cs, tag);
 
-- 
2.5.0

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

Re: [Pki-devel] [pki-devel][PATCH]

2016-11-22 Thread John Magne
Verbally discussed issue with cfu, was given cond ack upon fixing the issue:

Issue has been fixed, checked into master.

commit cdb8d2f7a3655b4ba97b70a9460721e0d2d8afe7
Author: Jack Magne <jma...@dhcp-16-206.sjc.redhat.com>
Date:   Tue Nov 15 17:37:07 2016 -0800

Change lifecycle at end of enrollment if it is not already set.

TPS throws "err=6" when attempting to format and enroll G Cards.
https://bugzilla.redhat.com/show_bug.cgi?id=1320283

This fix addresses this bug , but also:
Fixes this issue:

Applet upgrade during rekey operation results in formatted token.

 Also, it takes care of a related issue where the new apdu needed for the
lifecycle state causes the testing tool "tpslcient" to seg fault.
The fix here is a minimal fix to have tpsclient return an error when it gets
this apdu it can't handle, instead of crashing.


Closed ticket # 2544



- Original Message -
> From: "Christina Fu" <c...@redhat.com>
> To: pki-devel@redhat.com
> Sent: Wednesday, November 16, 2016 6:25:49 PM
> Subject: Re: [Pki-devel] [pki-devel][PATCH]
> 
> 
> 
> I compared this patch with the original C patch. There was a check in C that
> does not exist in your Java patch:
>   1019
> if(data.size() != 3){
> 
>   1020
> lifecycle = 0xf0;
> 
>   1021
> RA::Error(LL_PER_PDU, "RA_Processor::GetLifecycle", "apdu response is the
> wrong size, the size is: %x", data.size());
> 
>   1022
> goto loser;
> 
>   1023
> }
> 
> Why does it not apply in Java?
> 
> Thanks,
> Christina
> 
> On 11/15/2016 06:20 PM, John Magne wrote:
> 
> 
> 
> Ticket: TPS throws "err=6" when attempting to format and e :
> https://fedorahosted.org/pki/ticket/2544 Fix tested on standard card, it
> does what it is supposed to do. It checks first to make sure the lifecycle
> state needs to be changed before attempting to do so. This will prevent any
> cards that return an error when
> one tries to over write the value with the same value it had before.
> 
> 
> ___
> 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 mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel


Re: [Pki-devel] [pki-devel][PATCH]

2016-11-16 Thread Christina Fu
I compared this patch with the original C patch.  There was a check in C 
that does not exist in your Java patch:


1019

if(data.size() != 3){

1020

lifecycle = 0xf0;

1021

RA::Error(LL_PER_PDU, "RA_Processor::GetLifecycle", "apdu response is the 
wrong size, the size is: %x", data.size());

1022

goto loser;

1023

}


Why does it not apply in Java?

Thanks,
Christina

On 11/15/2016 06:20 PM, John Magne wrote:


Ticket: TPS throws "err=6" when attempting to format and e : 
https://fedorahosted.org/pki/ticket/2544

Fix tested on standard card, it does what it is supposed to do. It checks first 
to make sure the lifecycle
state needs to be changed before attempting to do so. This will prevent any 
cards that return an error when
one tries to over write the value with the same value it had before.



___
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] [pki-devel][PATCH] 0084-TPS-token-enrollment-fails-to-setupSecureChannel-whe.patch

2016-10-21 Thread Christina Fu
Just a minor suggestion.  Endi added in CryptalUtil.java lately to fix 
similar FIPS related issue:


isInternalToken().

You might want to take advantage of that instead as it does ignore case.

It's up to you.

ACK.

Christina


On 10/20/2016 03:24 PM, John Magne wrote:

TPS token enrollment fails to setupSecureChannel when TPS and TKS security db 
is on fips mode.
 
 Ticket #2513.
 
 Simple fix allows the TPS and TKS the ability to obtain the proper internal token, even in FiPS mode.



___
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] [pki-devel][PATCH] 0084-TPS-token-enrollment-fails-to-setupSecureChannel-whe.patch

2016-10-20 Thread John Magne

TPS token enrollment fails to setupSecureChannel when TPS and TKS security db 
is on fips mode.

Ticket #2513.

Simple fix allows the TPS and TKS the ability to obtain the proper internal 
token, even in FiPS mode.
From 00bba5092fa32b956d646b4711411b8c57bd8f75 Mon Sep 17 00:00:00 2001
From: Jack Magne 
Date: Thu, 20 Oct 2016 15:18:12 -0700
Subject: [PATCH] TPS token enrollment fails to setupSecureChannel when TPS and
 TKS security db is on fips mode.

Ticket #2513.

Simple fix allows the TPS and TKS the ability to obtain the proper internal token, even in FiPS mode.
---
 .../cms/src/com/netscape/cms/servlet/tks/SecureChannelProtocol.java| 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/base/server/cms/src/com/netscape/cms/servlet/tks/SecureChannelProtocol.java b/base/server/cms/src/com/netscape/cms/servlet/tks/SecureChannelProtocol.java
index db42cab..1997d11 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/tks/SecureChannelProtocol.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/tks/SecureChannelProtocol.java
@@ -688,10 +688,11 @@ public class SecureChannelProtocol {
 
 public CryptoToken returnTokenByName(String name, CryptoManager manager) throws NoSuchTokenException {
 
+CMS.debug("returnTokenByName: requested name: " + name);
 if (name == null || manager == null)
 throw new NoSuchTokenException();
 
-if (name.equals("internal") || name.equals("Internal KeyStorage Token")) {
+if (name.equals("internal") || name.equals("Internal Key Storage Token")) {
 return manager.getInternalKeyStorageToken();
 } else {
 return manager.getTokenByName(name);
-- 
2.5.0

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

Re: [Pki-devel] [pki-devel][PATCH] 0083-PIN_RESET-policy-is-not-giving-expected-results-when.patch

2016-10-19 Thread Christina Fu

code looks fine.  If tested to work, ACK.

Christina


On 10/18/2016 07:02 PM, John Magne wrote:

PIN_RESET policy is not giving expected results when set on a token.
 
 Simple fix to actually honor the PIN_RESET=or policy for a given token.

Minor logging improvements added as well for this error condition.
 
 Ticket #2510.





___
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] [pki-devel][PATCH] 0082-Cert-Key-recovery-is-successful-when-the-cert-serial.patch

2016-10-19 Thread John Magne
Pushed to master:

commit 9090451aa9f1a2dfcef8b852bb1e1d13d270098d
Author: Jack Magne <jma...@dhcp-16-206.sjc.redhat.com>
Date:   Tue Oct 18 15:08:44 2016 -0700

 Cert/Key recovery is successful when the cert serial number and key id on 
the ldap user mismatches

 Fixes this bug #1381375.
The portion this patch fixes involves URL encoding glitch we encountered 
when recovering keys using
the "by cert" method.

Also this bug addresses:

Bug 1379379 - Unable to read an encrypted email using renewed tokens
The URL encoding problem was affecting the proper verification of this bug.

and

Bug 1379749 - Automatic recovery of encryption cert is not working when a 
token is physically damaged and a temporary token is issued

The URI encoding was also making this bug appear to fail more than it 
should have.
There is also a minor fix to the feature that makes sure it works.

This small fix is in TPSEngine.java where the constant for 
GenerateNewAndRecoverLast scheme is declared.

- Original Message -
From: "Christina Fu" <c...@redhat.com>
To: pki-devel@redhat.com
Sent: Tuesday, October 18, 2016 4:24:08 PM
Subject: Re: [Pki-devel] [pki-devel][PATCH] 
0082-Cert-Key-recovery-is-successful-when-the-cert-serial.patch



If tested to work for all cases, ACK. 

Christina 

On 10/18/2016 03:22 PM, John Magne wrote: 



Cert/Key recovery is successful when the cert serial number and key id on the 
ldap user mismatches

 Fixes this bug #1381375.
The portion this patch fixes involves URL encoding glitch we encountered 
when recovering keys using
the "by cert" method.

Also this bug addresses:

Bug 1379379 - Unable to read an encrypted email using renewed tokens
The URL encoding problem was affecting the proper verification of this bug.

and

Bug 1379749 - Automatic recovery of encryption cert is not working when a 
token is physically damaged and a temporary token is issued

The URI encoding was also making this bug appear to fail more than it 
should have.
There is also a minor fix to the feature that makes sure it works.

This small fix is in TPSEngine.java where the constant for 
GenerateNewAndRecoverLast scheme is declared. 


___
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 mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel


[Pki-devel] [pki-devel][PATCH] 0083-PIN_RESET-policy-is-not-giving-expected-results-when.patch

2016-10-18 Thread John Magne
PIN_RESET policy is not giving expected results when set on a token.

Simple fix to actually honor the PIN_RESET=or policy for a given 
token.
Minor logging improvements added as well for this error condition.

Ticket #2510.


From 09dba122f01881b93d32a03a51d0be37c247cb30 Mon Sep 17 00:00:00 2001
From: Jack Magne 
Date: Tue, 18 Oct 2016 18:58:21 -0700
Subject: [PATCH] PIN_RESET policy is not giving expected results when set on a
 token.

Simple fix to actually honor the PIN_RESET=or policy for a given token.

Ticket #2510.
---
 .../server/tps/processor/TPSPinResetProcessor.java | 34 --
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/base/tps/src/org/dogtagpki/server/tps/processor/TPSPinResetProcessor.java b/base/tps/src/org/dogtagpki/server/tps/processor/TPSPinResetProcessor.java
index 9d0625a..fe3f801 100644
--- a/base/tps/src/org/dogtagpki/server/tps/processor/TPSPinResetProcessor.java
+++ b/base/tps/src/org/dogtagpki/server/tps/processor/TPSPinResetProcessor.java
@@ -21,6 +21,7 @@ import java.io.IOException;
 
 import org.dogtagpki.server.tps.TPSSession;
 import org.dogtagpki.server.tps.TPSSubsystem;
+import org.dogtagpki.server.tps.TPSTokenPolicy;
 import org.dogtagpki.server.tps.channel.SecureChannel;
 import org.dogtagpki.server.tps.dbs.ActivityDatabase;
 import org.dogtagpki.server.tps.dbs.TokenRecord;
@@ -98,15 +99,7 @@ public class TPSPinResetProcessor extends TPSProcessor {
 TPSStatus.STATUS_ERROR_MAC_RESET_PIN_PDU);
 }
 
-TokenStatus status = tokenRecord.getTokenStatus();
-
-CMS.debug(method + ": Token status: " + status);
-
-if (!status.equals(TokenStatus.ACTIVE)) {
-throw new TPSException(method + " Attempt to reset pin of token not currently active!",
-TPSStatus.STATUS_ERROR_MAC_RESET_PIN_PDU);
-
-}
+TPSTokenPolicy tokenPolicy = new TPSTokenPolicy(tps);
 
 session.setTokenRecord(tokenRecord);
 
@@ -142,6 +135,29 @@ public class TPSPinResetProcessor extends TPSProcessor {
 
 checkAndAuthenticateUser(appletInfo, tokenType);
 
+TokenStatus status = tokenRecord.getTokenStatus();
+
+CMS.debug(method + ": Token status: " + status);
+
+if (!status.equals(TokenStatus.ACTIVE)) {
+logMsg = method + "Can not reset the pin of a non active token.";
+auditPinReset(session.getIpAddress(), userid, appletInfo, "failure", null, logMsg);
+throw new TPSException(method + " Attempt to reset pin of token not currently active!",
+TPSStatus.STATUS_ERROR_MAC_RESET_PIN_PDU);
+
+}
+
+boolean pinResetAllowed = tokenPolicy.isAllowedPinReset(tokenRecord.getId());
+
+CMS.debug(method + ": PinResetPolicy: Pin Reset Allowed:  " + pinResetAllowed);
+logMsg = method + " PinReset Policy forbids pin reset operation.";
+if (pinResetAllowed == false) {
+auditPinReset(session.getIpAddress(), userid, appletInfo, "failure", null, logMsg);
+throw new TPSException(method + " Attempt to reset pin when token policy disallows it.!",
+TPSStatus.STATUS_ERROR_MAC_RESET_PIN_PDU);
+
+}
+
 checkAndUpgradeApplet(appletInfo);
 appletInfo = getAppletInfo();
 
-- 
2.5.0

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

Re: [Pki-devel] [pki-devel][PATCH] 0082-Cert-Key-recovery-is-successful-when-the-cert-serial.patch

2016-10-18 Thread Christina Fu

If tested to work for all cases, ACK.

Christina


On 10/18/2016 03:22 PM, John Magne wrote:

Cert/Key recovery is successful when the cert serial number and key id on the 
ldap user mismatches
 
  Fixes this bug #1381375.

 The portion this patch fixes involves URL encoding glitch we encountered 
when recovering keys using
 the "by cert" method.
 
 Also this bug addresses:
 
 Bug 1379379 - Unable to read an encrypted email using renewed tokens

 The URL encoding problem was affecting the proper verification of this bug.
 
 and
 
 Bug 1379749 - Automatic recovery of encryption cert is not working when a token is physically damaged and a temporary token is issued
 
 The URI encoding was also making this bug appear to fail more than it should have.

 There is also a minor fix to the feature that makes sure it works.
 
 This small fix is in TPSEngine.java where the constant for GenerateNewAndRecoverLast scheme is declared.



___
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] [pki-devel][PATCH] 0082-Cert-Key-recovery-is-successful-when-the-cert-serial.patch

2016-10-18 Thread John Magne
Cert/Key recovery is successful when the cert serial number and key id on the 
ldap user mismatches

 Fixes this bug #1381375.
The portion this patch fixes involves URL encoding glitch we encountered 
when recovering keys using
the "by cert" method.

Also this bug addresses:

Bug 1379379 - Unable to read an encrypted email using renewed tokens
The URL encoding problem was affecting the proper verification of this bug.

and

Bug 1379749 - Automatic recovery of encryption cert is not working when a 
token is physically damaged and a temporary token is issued

The URI encoding was also making this bug appear to fail more than it 
should have.
There is also a minor fix to the feature that makes sure it works.

This small fix is in TPSEngine.java where the constant for 
GenerateNewAndRecoverLast scheme is declared.
From a1f1e030298c38e0c08a514852a435e77d88a2b9 Mon Sep 17 00:00:00 2001
From: Jack Magne 
Date: Tue, 18 Oct 2016 15:08:44 -0700
Subject: [PATCH]  Cert/Key recovery is successful when the cert serial number
 and key id on the ldap user mismatches

 Fixes this bug #1381375.
The portion this patch fixes involves URL encoding glitch we encountered when recovering keys using
the "by cert" method.

Also this bug addresses:

Bug 1379379 - Unable to read an encrypted email using renewed tokens
The URL encoding problem was affecting the proper verification of this bug.

and

Bug 1379749 - Automatic recovery of encryption cert is not working when a token is physically damaged and a temporary token is issued

The URI encoding was also making this bug appear to fail more than it should have.
There is also a minor fix to the feature that makes sure it works.

This small fix is in TPSEngine.java where the constant for GenerateNewAndRecoverLast scheme is declared.
---
 .../server/tps/cms/KRARemoteRequestHandler.java|  9 ++--
 .../org/dogtagpki/server/tps/engine/TPSEngine.java | 15 --
 .../server/tps/processor/TPSEnrollProcessor.java   | 63 --
 3 files changed, 50 insertions(+), 37 deletions(-)

diff --git a/base/tps/src/org/dogtagpki/server/tps/cms/KRARemoteRequestHandler.java b/base/tps/src/org/dogtagpki/server/tps/cms/KRARemoteRequestHandler.java
index 80439ca..3674526 100644
--- a/base/tps/src/org/dogtagpki/server/tps/cms/KRARemoteRequestHandler.java
+++ b/base/tps/src/org/dogtagpki/server/tps/cms/KRARemoteRequestHandler.java
@@ -23,7 +23,6 @@ import java.util.Hashtable;
 
 import org.dogtagpki.server.connector.IRemoteRequest;
 import org.dogtagpki.server.tps.TPSSubsystem;
-import org.dogtagpki.tps.main.Util;
 
 import com.netscape.certsrv.apps.CMS;
 import com.netscape.certsrv.base.EBaseException;
@@ -265,15 +264,15 @@ public class KRARemoteRequestHandler extends RemoteRequestHandler
 String sendMsg = null;
 try {
 if (b64cert != null) { // recover by cert
-// CMS.debug("KRARemoteRequestHandler: recoverKey(): uriEncoded cert= " + Util.uriEncode(b64cert));
+// CMS.debug("KRARemoteRequestHandler: recoverKey(): uriEncoded cert= " + b64cert);
 sendMsg = IRemoteRequest.TOKEN_CUID + "=" +
 cuid +
 "&" + IRemoteRequest.KRA_UserId + "=" +
 userid +
 "&" + IRemoteRequest.KRA_RECOVERY_CERT + "=" +
-Util.uriEncode(b64cert) +
+b64cert  +
 "&" + IRemoteRequest.KRA_Trans_DesKey + "=" +
-Util.uriEncode(sDesKey);
+sDesKey;
 } else if (keyid != BigInteger.valueOf(0)) { // recover by keyid ... keyid != BigInteger.valueOf(0)
 CMS.debug("KRARemoteRequestHandler: recoverKey(): keyid = " + keyid);
 sendMsg = IRemoteRequest.TOKEN_CUID + "=" +
@@ -283,7 +282,7 @@ public class KRARemoteRequestHandler extends RemoteRequestHandler
 "&" + IRemoteRequest.KRA_RECOVERY_KEYID + "=" +
 keyid.toString() +
 "&" + IRemoteRequest.KRA_Trans_DesKey + "=" +
-Util.uriEncode(sDesKey);
+sDesKey;
 }
 } catch (Exception e) {
 CMS.debug("KRARemoteRequestHandler: recoverKey(): uriEncode failed: " + e);
diff --git a/base/tps/src/org/dogtagpki/server/tps/engine/TPSEngine.java b/base/tps/src/org/dogtagpki/server/tps/engine/TPSEngine.java
index 93edfde..319ff67 100644
--- a/base/tps/src/org/dogtagpki/server/tps/engine/TPSEngine.java
+++ b/base/tps/src/org/dogtagpki/server/tps/engine/TPSEngine.java
@@ -185,7 +185,7 @@ public class TPSEngine {
 public static final String CFG_PIN_RESET_STRING = "create_pin.string";
 
 public static final String CFG_SCHEME = "scheme";
-public static final String 

Re: [Pki-devel] [pki-devel][PATCH] 0077-Make-starting-CRL-Number-configurable.patch

2016-07-27 Thread John Magne
Verbally acked by edewata thanks! :

pushed to master

Closing ticket: #2406



- Original Message -
> From: "John Magne" <jma...@redhat.com>
> To: "pki-devel" <pki-devel@redhat.com>
> Sent: Wednesday, July 27, 2016 11:53:34 AM
> Subject: [Pki-devel] [pki-devel][PATCH]   
> 0077-Make-starting-CRL-Number-configurable.patch
> 
> Make starting CRL Number configurable.
> 
> Ticket #2406 Make starting CRL Number configurable
> 
> This simple patch provides a pkispawn config param that passes
> some starting crl number value to the config process.
> 
> Here is a sample:
> 
> [CA]
> pki_ca_starting_crl_number=4000
> 
> After the CA comes up the value of "crlNumber" in the db will
> reflect that value of 4000.
> 
> Currently no other values are changed. We can talk about if we
> need more values reset in the given case.
> 
> Also, this creates a setting in the CS.cfg
> 
> ca.crl.MasterCrl.startingCrlNumber=4000
> 
> This setting is only consulted when the crl Issuing Point record is
> created
> for the first time.
> 
> ___
> 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] [pki-devel][PATCH] 0076-MAN-Apply-generateCRMFRequest-removed-from-Firefox-w.patch

2016-07-14 Thread John Magne
Conditionally ACKED by cfu.
She wanted me to test the new ECC signing cert only profile I added:

Test was a success.

Pushed to master
Closing ticket #1285


Also release note bug on how to use the new profiles here:
https://bugzilla.redhat.com/show_bug.cgi?id=1355849

- Original Message -
From: "John Magne" 
To: "pki-devel" , pki-devel@redhat.com
Cc: c...@redhat.com
Sent: Thursday, July 14, 2016 11:42:36 AM
Subject: [pki-devel][PATCH] 
0076-MAN-Apply-generateCRMFRequest-removed-from-Firefox-w.patch

 [MAN] Apply 'generateCRMFRequest() removed from Firefox'
 workarounds to appropriate 'pki' man page

Ticket #1285 

This fix will involve the following changes to the source tree.

1. Fixes to the CS.cfg to add two new cert profiles.
2. Make the caDualCert.cfg profile invisible since it has little chance of
working any more in Firefox.
3. Create caSigningUserCert.cfg and caSigningECUserCert.cfg to allow the CLI
to have convenient profiles from which to enroll signing ONLY certificates.

To go along with this I have filed a downstream release note bug that shows 
exactly how to
deploy the new  profile to separately create one signing cert and one 
encryption cert (with archival),
which allows one to accomplish what the formater caDualCert profile used to do 
when Firefox supported it.

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


Re: [Pki-devel] [pki-devel][PATCH] 0073-Separated-TPS-does-not-automatically-receive-shared-.patch

2016-07-01 Thread John Magne
ACKED verbally by cfu, with some very minor changes.

Pushed to master:


commit 0f056221d096a30307834265ecd1c527087bb0f7
Author: Jack Magne 
Date:   Mon Jun 13 11:27:59 2016 -0700

Separated TPS does not automatically receive shared secret from remote TKS.



Closing ticket # 2349




- Original Message -
From: "John Magne" 
To: "pki-devel" 
Sent: Thursday, June 23, 2016 3:33:44 PM
Subject: [pki-devel][PATCH] 
0073-Separated-TPS-does-not-automatically-receive-shared-.patch



[PATCH] Separated TPS does not automatically receive shared secret
 from remote TKS.

Support to allow the TPS to do the following:

1. Request that the TKS creates a shared secret with the proper ID, pointing to 
the TPS.
2. Have the TKS securely return the shared secret back to the TPS during the 
end of configuration.
3. The TPS then imports the wrapped shared secret into it's own internal NSS db 
permanenty and.
4. Given a name that is mapped to the TPS's id string.

Additional fixes:

1. The TKS was modified to actually be able to use multiple shared secrets 
registered by
multiple TPS instances.

Caveat:

At this point if the same remote TPS instance is created over and over again, 
the TPS's user
in the TKS will accumulate "userCert" attributes, making the exportation of teh 
shared secret
not functional. At this point we need to assume that the TPS user has ONE 
"userCert" registered
at this time.


Tested with a remote TPS talking to a shared TMS system consisting of a TPS, 
TKS, and KRA .

The shared secret was imported successfully after manually deleting the user 
representing the TPS from previous installs.
This way I was assured one cert stored for the user, since it had to be created 
fresh.

Also tested that the TKS can work successfully with the new TPS AND the prior 
shared TPS on the original instance.
The TKS can now host more than one shared secret in it's db and address the 
correct one when a given TPS makes a request of it.

Please forgive some spurious changes that happened when formatting a couple of 
the files in question. Every legit change is related to the shared secret and 
can be found easily.

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


Re: [Pki-devel] [pki-devel][PATCH] 0075-Generting-Symmetric-key-fails-with-key-generate-when.patch

2016-07-01 Thread John Magne
Pushed to master, ACK from mharmsen

Closing #1114


commit cfab57d057c7ada71ea9c360c278249d14e018d9
Author: Jack Magne <jma...@dhcp-16-206.sjc.redhat.com>
Date:   Fri Jun 24 17:04:15 2016 -0700

Generting Symmetric key fails with key-generate when --usages verify is 
passed

Ticket #1114

Minor adjustment to the man page for the key management commands to say
which usages are appropriate for sym keys and those appropriate for asym 
keys.



- Original Message -
From: "Matthew Harmsen" <mharm...@redhat.com>
To: "John Magne" <jma...@redhat.com>, "pki-devel" <pki-devel@redhat.com>
Sent: Thursday, June 30, 2016 2:54:29 PM
Subject: Re: [Pki-devel] [pki-devel][PATCH] 
0075-Generting-Symmetric-key-fails-with-key-generate-when.patch

On 06/24/2016 06:23 PM, John Magne wrote:
> Generting Symmetric key fails with key-generate when --usages verify is passed
>  
>  Ticket #1114
>  
>  Minor adjustment to the man page for the key management commands to say
>  which usages are appropriate for sym keys and those appropriate for asym 
> keys.
>  
>
>
> ___
> Pki-devel mailing list
> Pki-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel
ACK

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


Re: [Pki-devel] [pki-devel][PATCH 0003] Added condition for checking instance id in kra commands

2016-06-30 Thread Endi Sukma Dewata

On 6/30/2016 5:09 AM, Abhijeet Kasurde wrote:

Hi All,

Please review this patch.

Partially fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1351295

--
Thanks,
Abhijeet Kasurde


Thanks! Pushed to master with some changes:

1. The original code was supposed to normalize the token name, so if 
it's 'internal' or 'Internal Key Storage Token' it will be normalized to 
None. If token name is None we don't add -h  when calling 
certutil since by default certutil will use internal token.


There's a bug in PKIInstance.get_token_password() though. If the caller 
specifies token parameter to be None explicitly, it won't get the 
default value of 'internal'. The method has been fixed to check for None 
value.


2. The code that catches CalledProcessError has been moved into the main 
program (i.e. pki-server) so similar errors will be handled more 
consistently.


3. Some error messages are changed for consistency.

--
Endi S. Dewata

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


Re: [Pki-devel] [pki-devel][PATCH 0002] Added fix for checking ldapmodify return code in db-schema-upgrade

2016-06-30 Thread Endi Sukma Dewata

On 6/29/2016 7:43 AM, Abhijeet Kasurde wrote:

Hi All,

Please review the patch.

--
Thanks,
Abhijeet Kasurde


Thanks! Pushed to master with some changes to handle all LDAP errors 
instead of some specific ones.


--
Endi S. Dewata

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


Re: [Pki-devel] [pki-devel][PATCH] 0075-Generting-Symmetric-key-fails-with-key-generate-when.patch

2016-06-30 Thread Matthew Harmsen

On 06/24/2016 06:23 PM, John Magne wrote:

Generting Symmetric key fails with key-generate when --usages verify is passed
 
 Ticket #1114
 
 Minor adjustment to the man page for the key management commands to say

 which usages are appropriate for sym keys and those appropriate for asym 
keys.
 



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

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

Re: [Pki-devel] [pki-devel] [PATCH] 0074-Add-ability-to-disallow-TPS-to-enroll-a-single-user-.patch

2016-06-27 Thread Christina Fu

Just a few minor ones.

* configuration parameters referencing token existence in tokendb should 
use names begin with "tokendb".  e.g.

tokendb.allowMultiActiveTokensPerUser.externalReg=false
tokendb.allowMultiActiveTokensPerUser.nonExternalReg=false

* boolean allowMultiCerts  -- I think the name is misleading.  how about 
alowMultiTokens


* existing calls to tps.tdb.tdbHasActiveToken() need to be decided:
 e.g.
   1. TPSEnrollProcessor.java search for tdbHasActiveToken (first 
occurrence) , you will find that it is called with "TODO:" comment. I 
believe that whole try/catch with the tps.tdb.tdbHasActiveToken(userid); 
call can be removed since you already call that earlier in your patch
2. TPSEnrollProcessor.java, the 2nd occurrence is when the 
enrolling token is suspended.  You need to look into what it is doing at 
the point and whether that check can also be eliminated.


thanks,
Christina

On 06/24/2016 11:08 AM, John Magne wrote:

Add ability to disallow TPS to enroll a single user on multiple tokens.
 
 This patch will install a check during the early portion of the enrollment

 process check a configurable policy whether or not a user should be allowed
 to have more that one active token.
 
 This check will take place only for brand new tokens not seen before.

 The check will prevent the enrollment to proceed and will exit before the 
system
 has a chance to add this new token to the TPS tokendb.
 
 The behavior will be configurable for the the external reg and not external reg scenarios

 as follows:
 
 op.enroll.nonExternalReg.allowMultiActiveTokensUser=false

 op.enroll.externalReg.allowMultiActiveTokensUser=false


___
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] [pki-devel][PATCH] 0075-Generting-Symmetric-key-fails-with-key-generate-when.patch

2016-06-24 Thread John Magne
Generting Symmetric key fails with key-generate when --usages verify is passed

Ticket #1114

Minor adjustment to the man page for the key management commands to say
which usages are appropriate for sym keys and those appropriate for asym 
keys.

From a211222ee4b30ad390228adeb96645fd840be3ad Mon Sep 17 00:00:00 2001
From: Jack Magne 
Date: Fri, 24 Jun 2016 17:04:15 -0700
Subject: [PATCH] Generting Symmetric key fails with key-generate when --usages
 verify is passed

Ticket #1114

Minor adjustment to the man page for the key management commands to say
which usages are appropriate for sym keys and those appropriate for asym keys.

t
---
 base/java-tools/man/man1/pki-key.1 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/base/java-tools/man/man1/pki-key.1 b/base/java-tools/man/man1/pki-key.1
index 30ac909..669ab86 100644
--- a/base/java-tools/man/man1/pki-key.1
+++ b/base/java-tools/man/man1/pki-key.1
@@ -329,7 +329,9 @@ Following is the description of the various parameters in the key retrieval temp
 -- clientKeyID - Client specified unique key identifier
 -- keyAlgorithm - Algorithm to be used to generate key (AES/DES/DES3/DESede/RC2/RC4)
 -- keySize - Value for the size of the key to be generated.
--- keyUsage - usages of the generated key(wrap,unwrap,sign,verify,encrypt,decrypt)
+-- keyUsage - usages of the generated key
+useful for Symmetric Keys (DES3,AES,etc) (wrap,unwrap,encrypt,decrypt)
+useful for Asymmetric Keys (RSA, EC,etc) (wrap,unwrap,encrypt,decrypt,sign,verify,sign_recover,verify_recover)
 
 To create a key generation request using the template file:
 
-- 
2.5.0

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

[Pki-devel] [pki-devel][PATCH] 0073-Separated-TPS-does-not-automatically-receive-shared-.patch

2016-06-23 Thread John Magne


[PATCH] Separated TPS does not automatically receive shared secret
 from remote TKS.

Support to allow the TPS to do the following:

1. Request that the TKS creates a shared secret with the proper ID, pointing to 
the TPS.
2. Have the TKS securely return the shared secret back to the TPS during the 
end of configuration.
3. The TPS then imports the wrapped shared secret into it's own internal NSS db 
permanenty and.
4. Given a name that is mapped to the TPS's id string.

Additional fixes:

1. The TKS was modified to actually be able to use multiple shared secrets 
registered by
multiple TPS instances.

Caveat:

At this point if the same remote TPS instance is created over and over again, 
the TPS's user
in the TKS will accumulate "userCert" attributes, making the exportation of teh 
shared secret
not functional. At this point we need to assume that the TPS user has ONE 
"userCert" registered
at this time.


Tested with a remote TPS talking to a shared TMS system consisting of a TPS, 
TKS, and KRA .

The shared secret was imported successfully after manually deleting the user 
representing the TPS from previous installs.
This way I was assured one cert stored for the user, since it had to be created 
fresh.

Also tested that the TKS can work successfully with the new TPS AND the prior 
shared TPS on the original instance.
The TKS can now host more than one shared secret in it's db and address the 
correct one when a given TPS makes a request of it.

Please forgive some spurious changes that happened when formatting a couple of 
the files in question. Every legit change is related to the shared secret and 
can be found easily.From ef5d954d1160002f6ed0ff05894f5968d7982d24 Mon Sep 17 00:00:00 2001
From: Jack Magne 
Date: Mon, 13 Jun 2016 11:27:59 -0700
Subject: [PATCH] Separated TPS does not automatically receive shared secret
 from remote TKS.

Support to allow the TPS to do the following:

1. Request that the TKS creates a shared secret with the proper ID, pointing to the TPS.
2. Have the TKS securely return the shared secret back to the TPS during the end of configuration.
3. The TPS then imports the wrapped shared secret into it's own internal NSS db permanenty and.
4. Given a name that is mapped to the TPS's id string.

Additional fixes:

1. The TKS was modified to actually be able to use multiple shared secrets registered by
multiple TPS instances.

Caveat:

At this point if the same remote TPS instance is created over and over again, the TPS's user
in the TKS will accumulate "userCert" attributes, making the exportation of teh shared secret
not functional. At this point we need to assume that the TPS user has ONE "userCert" registered
at this time.
---
 .../src/com/netscape/certsrv/key/KeyData.java  |  17 +-
 .../cms/servlet/csadmin/ConfigurationUtils.java| 252 -
 .../cms/servlet/tks/SecureChannelProtocol.java |   3 +-
 .../com/netscape/cms/servlet/tks/TokenServlet.java |  26 ++-
 .../server/tks/rest/TPSConnectorService.java   | 147 +---
 .../server/tps/processor/TPSProcessor.java |   8 +-
 .../server/tps/rest/TPSInstallerService.java   |  12 +-
 .../com/netscape/cmsutil/crypto/CryptoUtil.java| 157 +
 8 files changed, 426 insertions(+), 196 deletions(-)

diff --git a/base/common/src/com/netscape/certsrv/key/KeyData.java b/base/common/src/com/netscape/certsrv/key/KeyData.java
index 6da4d38..d50c9ec 100644
--- a/base/common/src/com/netscape/certsrv/key/KeyData.java
+++ b/base/common/src/com/netscape/certsrv/key/KeyData.java
@@ -48,7 +48,8 @@ public class KeyData {
 @XmlElement
 Integer size;
 
-String privateData;
+@XmlElement
+String additionalWrappedPrivateData;
 
 public KeyData() {
 // required for JAXB (defaults)
@@ -68,6 +69,14 @@ public class KeyData {
 this.wrappedPrivateData = wrappedPrivateData;
 }
 
+public String getAdditionalWrappedPrivateData() {
+return additionalWrappedPrivateData;
+}
+
+public void setAdditionalWrappedPrivateData(String additionalWrappedPrivateData) {
+this.additionalWrappedPrivateData = additionalWrappedPrivateData;
+}
+
 /**
  * @return the nonceData
  */
@@ -126,11 +135,5 @@ public class KeyData {
 this.size = size;
 }
 
-public String getPrivateData() {
-return privateData;
-}
 
-public void setPrivateData(String privateData) {
-this.privateData = privateData;
-}
 }
diff --git a/base/server/cms/src/com/netscape/cms/servlet/csadmin/ConfigurationUtils.java b/base/server/cms/src/com/netscape/cms/servlet/csadmin/ConfigurationUtils.java
index 2da4e48..56ee9fa 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/csadmin/ConfigurationUtils.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/csadmin/ConfigurationUtils.java
@@ -15,7 +15,7 @@
 // (C) 2012 Red Hat, Inc.
 // All rights reserved.
 // --- END COPYRIGHT BLOCK ---
- 

Re: [Pki-devel] [pki-devel][PATCH] 0072-Revocation-failure-causes-AUDIT_PRIVATE_KEY_ARCHIVE_.patch

2016-06-17 Thread John Magne
ACK'd by cfu:

Pushed to master, closing ticket #2340

   
- Original Message -
From: "John Magne" 
To: "pki-devel" 
Sent: Tuesday, June 14, 2016 4:07:49 PM
Subject: [pki-devel][PATCH] 
0072-Revocation-failure-causes-AUDIT_PRIVATE_KEY_ARCHIVE_.patch

Revocation failure causes AUDIT_PRIVATE_KEY_ARCHIVE_REQUEST

The fix here is to make sure no archive related audits get issued for doing
things other than key archivals.

Other operations such as revoking and unrevoking cert in the code path 
laready
have audit logs issued separately for success or failure.

Ticket #2340.

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


Re: [Pki-devel] [pki-devel][PATCH] 0070-Fix-coverity-warnings-for-tkstool.patch

2016-06-14 Thread Matthew Harmsen

On 06/06/2016 05:39 PM, John Magne wrote:

Fix attached.


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

ACK

Personally, I always prefer the use of enclosing braces "{ . . . }" 
after a conditional even when it only has one line.
___
Pki-devel mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel

Re: [Pki-devel] [pki-devel][PATCH] 0069-Show-KeyOwner-info-when-viewing-recovery-requests.patch

2016-06-03 Thread John Magne

Pushed to master based on cfu's verbal conditional ACK for this 
(after I modded it the way she requested)

Tested to work.

commit 3cd58a98022141da2af4bf0bad29ab1dbdc86fbe
Author: Jack Magne <jma...@dhcp-16-206.sjc.redhat.com>
Date:   Wed Jun 1 15:05:20 2016 -0700




Closing ticket #1512

- Original Message -
> From: "Christina Fu" <c...@redhat.com>
> To: pki-devel@redhat.com
> Sent: Friday, June 3, 2016 2:46:28 PM
> Subject: Re: [Pki-devel] [pki-devel][PATCH] 
> 0069-Show-KeyOwner-info-when-viewing-recovery-requests.patch
> 
> while the patch works, I think the original code logic is somehow flawed in a
> way that it uses the "profile" attribute to determine whether the request
> was non-TMS archival requests, and if null it treats it as TMS. It would
> make better sense if we add a separate case instead of lumping the handling
> of recovery requests inside where the TMS handling is at.
> 
> thanks,
> Christina
> 
> On 06/01/2016 03:13 PM, John Magne wrote:
> 
> 
> 
> Show KeyOwner info when viewing recovery requests.
> 
> This simple fix will grab the subject info out of the cert
> associated with either pending or complete recovery requests being
> viewed in the KRA UI.
> 
> For example:
> 
> KeyOwner:  UID=jmagne, O=Token Key User
> 
> Will be displayed.
> Have seen this display for both pending and completed recovery requests.
> 
> This simple fix should be good enough for this round, despite the bug
> asking about agent info and such. Those enhancements for later.
> 
> Ticket : Ticket #1512 : Key owner info missing from the Search results of
> Recovery request
> 
> 
> ___
> 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 mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel


Re: [Pki-devel] [pki-devel][PATCH] 0069-Show-KeyOwner-info-when-viewing-recovery-requests.patch

2016-06-03 Thread Christina Fu
while the patch works, I think the original code logic is somehow flawed 
in a way that it uses the "profile" attribute to determine whether the 
request was non-TMS archival requests, and if null it treats it as TMS.  
It would make better sense if we add a separate case instead of lumping 
the handling of recovery requests inside where the TMS handling is at.


thanks,
Christina

On 06/01/2016 03:13 PM, John Magne wrote:

Show KeyOwner info when viewing recovery requests.
 
 This simple fix will grab the subject info out of the cert

 associated with either pending or complete recovery requests being
 viewed in the KRA UI.
 
 For example:
 
 KeyOwner:  UID=jmagne, O=Token Key User
 
 Will be displayed.

 Have seen this display for both pending and completed recovery requests.

 This simple fix should be good enough for this round, despite the bug
 asking about agent info and such. Those enhancements for later.
 
 Ticket : Ticket #1512 : Key owner info missing from the Search results of Recovery request



___
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] [pki-devel][PATCH] 0064-Port-symkey-JNI-to-Java-classes.patch

2016-05-23 Thread John Magne
Checked into master:

commit 1d60c55940e310aa77befe09c970db3831bb5042
Author: Jack Magne <jma...@dhcp-16-206.sjc.redhat.com>
Date:   Tue Mar 29 10:39:27 2016 -0700

Port symkey JNI to Java classes.
Ticket #801 : Merge pki-symkey into jss

What is supported:

1. Everything that is needed to support Secure Channel Protocol 01.
2. Supports the nist sp800 kdf and the original kdf.
3. Supports key unwrapping used by TPS which was formerly in the symkey JNI.

Requires:

1. A new JSS that supports more advanced symkey operations such as key 
derivation, more advanced key
unwrapping , and a way to list and identify a given symmetric key by name. 
Version of new Jss will be forthcoming.

Still to do:

1. Port over the 2 or 3 SCP02 routines from Symkey to use this code.
2. The original symkey will remain in place until we can port over 
everything.
3. SCP03 support can be added later.


New ticket created for future refinements:

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


Closing #801


- Original Message -
From: "Christina Fu" <c...@redhat.com>
To: pki-devel@redhat.com
Sent: Monday, May 23, 2016 8:56:40 AM
Subject: Re: [Pki-devel] [pki-devel][PATCH] 
0064-Port-symkey-JNI-to-Java-classes.patch

Just realized that I missed this comment for conditional ack:

It was already communicated to Jack. Please file a  ticket for this.
derivedKey = encryptDes3.derive();
  it throws exception when fail (in nethsm it seems to be the case now), 
and then default to encryption.
  It'd be better to provide a config param to turn on and off derive 
v.s. encryption in case we know for sure a certain hsm does not handle 
such thing, then the process won't waste the consistent exceptions.

Once again, great job on such daunting task!!
thanks,
Christina

On 05/18/2016 06:31 PM, Christina Fu wrote:
> This is the re-review of the patches that addressed my original comments.
> Overall I think it's good for this round.  I only have a few comments 
> and most have already been communicated to jack.
>
> Conditional ACK upon completion of the following, and of course, 
> tested to work:
>
> * Please open the new tickets for tasks you wish to push for later. 
> Feel free to combine things in same area into one ticket if it makes 
> sense. Please list the ticket(s) at commit response.
> * Please write a wrapper function computeKekKey() to call the 
> computeSessionKey_SCP01() with null null, so for people who read the 
> code it's clear that it's actually getting the kek key handle rather 
> than a session key.
> * wrapSessionKey() now takes a wrapping key, and if it's null, it 
> takes a global transportKey.  Please put this in a top block method 
> comment to make it clear what this method does
> * you defined cryptogram types (per my earlier comment), but you did 
> not replace the 0 and 1 in the calling method.
> * the top of method comment convention is usually using /* ...*/ 
> instead of a whole bunch of //'s
>
>
> thanks!
> Christina
>
>
> On 05/17/2016 06:44 PM, John Magne wrote:
>> Enclosed revised patches:
>>
>> Thanks to cfu for careful review.
>>
>> Also enclosed responses to comments ,for convenience.
>>
>>
>>
>>
>> - Original Message -
>>> From: "Christina Fu" <c...@redhat.com>
>>> To: pki-devel@redhat.com
>>> Sent: Friday, May 13, 2016 11:34:17 AM
>>> Subject: Re: [Pki-devel] [pki-devel][PATCH] 
>>> 0064-Port-symkey-JNI-to-Java-classes.patch
>>>
>>> Hi,
>>> First of all, I have to say that Jack did a wonderful job on such 
>>> daunting
>>> task. The sheer amount of code and complexity does make the review more
>>> challenging, but I dug through them with my teeth and claws 
>>> regardless ;-).
>>>
>>> We discussed and think we should postpone the checkin to next 
>>> release so we
>>> can make sure it gets the kind of attention in details that it 
>>> deserves.
>>>
>>> For the first round of reviews, I sent him two separate sets of review
>>> comments last week. One for JSS, and one for the rest.
>>> The JSS patch was not attached to his original email request for 
>>> review. It
>>> is attached to the following ticket:
>>> https://fedorahosted.org/pki/ticket/801
>>>
>>> You can find my review comments attached to this email.
>>>
>>> thanks,
>>> Christina
>>>
>>> On 04/15/2016 07:03 PM, John Magne wrote:
>>>
>>>
>>>
>>> Subject: [PATCH] Port symkey JNI to Java classes. Ticket #801 : Merge
>>>   pki-symkey into jss
>>>
>>> Wha

Re: [Pki-devel] [pki-devel][PATCH] 0064-Port-symkey-JNI-to-Java-classes.patch

2016-05-18 Thread Christina Fu

This is the re-review of the patches that addressed my original comments.
Overall I think it's good for this round.  I only have a few comments 
and most have already been communicated to jack.


Conditional ACK upon completion of the following, and of course, tested 
to work:


* Please open the new tickets for tasks you wish to push for later. Feel 
free to combine things in same area into one ticket if it makes sense. 
Please list the ticket(s) at commit response.
* Please write a wrapper function computeKekKey() to call the 
computeSessionKey_SCP01() with null null, so for people who read the 
code it's clear that it's actually getting the kek key handle rather 
than a session key.
* wrapSessionKey() now takes a wrapping key, and if it's null, it takes 
a global transportKey.  Please put this in a top block method comment to 
make it clear what this method does
* you defined cryptogram types (per my earlier comment), but you did not 
replace the 0 and 1 in the calling method.
* the top of method comment convention is usually using /* ...*/ instead 
of a whole bunch of //'s



thanks!
Christina


On 05/17/2016 06:44 PM, John Magne wrote:

Enclosed revised patches:

Thanks to cfu for careful review.

Also enclosed responses to comments ,for convenience.




- Original Message -

From: "Christina Fu" <c...@redhat.com>
To: pki-devel@redhat.com
Sent: Friday, May 13, 2016 11:34:17 AM
Subject: Re: [Pki-devel] [pki-devel][PATCH] 
0064-Port-symkey-JNI-to-Java-classes.patch

Hi,
First of all, I have to say that Jack did a wonderful job on such daunting
task. The sheer amount of code and complexity does make the review more
challenging, but I dug through them with my teeth and claws regardless ;-).

We discussed and think we should postpone the checkin to next release so we
can make sure it gets the kind of attention in details that it deserves.

For the first round of reviews, I sent him two separate sets of review
comments last week. One for JSS, and one for the rest.
The JSS patch was not attached to his original email request for review. It
is attached to the following ticket:
https://fedorahosted.org/pki/ticket/801

You can find my review comments attached to this email.

thanks,
Christina

On 04/15/2016 07:03 PM, John Magne wrote:



Subject: [PATCH] Port symkey JNI to Java classes. Ticket #801 : Merge
  pki-symkey into jss

What is supported:

1. Everything that is needed to support Secure Channel Protocol 01.
2. Supports the nist sp800 kdf and the original kdf.
3. Supports key unwrapping used by TPS which was formerly in the symkey JNI.

Requires:

1. A new JSS that supports more advanced symkey operations such as key
derivation, more advanced key
unwrapping , and a way to list and identify a given symmetric key by name.
Version of new Jss will be forthcoming.

Still to do:

1. Port over the 2 or 3 SCP02 routines from Symkey to use this code.
2. The original symkey will remain in place until we can port over
everything.
3. SCP03 support can be added later.


___
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 mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel


Re: [Pki-devel] [pki-devel][PATCH] 0064-Port-symkey-JNI-to-Java-classes.patch

2016-05-13 Thread Christina Fu

Hi,
First of all, I have to say that Jack did a wonderful job on such 
daunting task.  The sheer amount of code and complexity does make the 
review more challenging, but I dug through them with my teeth and claws 
regardless ;-).


We discussed and think we should postpone the checkin to next release so 
we can make sure it gets the kind of attention in details that it deserves.


For the first round of reviews, I sent him two separate sets of review 
comments last week.  One for JSS, and one for the rest.
The JSS patch was not attached to his original email request for 
review.  It is attached to the following ticket:

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

You can find my review comments attached to this email.

thanks,
Christina

On 04/15/2016 07:03 PM, John Magne wrote:

Subject: [PATCH] Port symkey JNI to Java classes. Ticket #801 : Merge
  pki-symkey into jss

What is supported:

1. Everything that is needed to support Secure Channel Protocol 01.
2. Supports the nist sp800 kdf and the original kdf.
3. Supports key unwrapping used by TPS which was formerly in the symkey JNI.

Requires:

1. A new JSS that supports more advanced symkey operations such as key 
derivation, more advanced key
unwrapping , and a way to list and identify a given symmetric key by name. 
Version of new Jss will be forthcoming.

Still to do:

1. Port over the 2 or 3 SCP02 routines from Symkey to use this code.
2. The original symkey will remain in place until we can port over everything.
3. SCP03 support can be added later.


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


--- Begin Message ---

Sending this out to you just to get it off my back...
this is just the JSS part.

Some of these already communicated to you.  I'm just listing them out to 
track for the re-review.


==

JSS
*PK11KeyWrapper.java
  - The original unwrapSymmetric) assumes temporary true, and you want 
to have a function that treats it as false.
  You made a copy of the original and named it unwrapSymmetricPerm() 
and set the temporary to false.
  -  I think it'd be better if you just add a "temporary" param to the 
original function, and add the function with original signature to call 
into the one with temporary true;  Then you call into the new signature 
with temporary false (instead of adding this extra unwrapSymmetricPerm()


* Java_org_mozilla_jss_pkcs11_PK11Store_putSymKeysInVector
This function gets a list of sym keys from a token and put them into a 
vector.  It seems more appropriate to be called
getSymKeysInVector()  ("put" made me think initially that you are 
putting keys into the token)


* PK11SymKey.c: JSS_PK11_wrapSymKey
 - Are we sure that all sym keys have nicknames?
   Should we maintain the original "no nickname" code path by checking 
if nickname is null and call original calls?


* in Java_org_mozilla_jss_pkcs11_PK11SymKey_setNickNameNative
 /* name the key */
status = PK11_SetSymKeyNickname( key, keyname );
if( status != SECSuccess ) {
JSS_throwMsgPrErr(env, TOKEN_EXCEPTION,
"Failed to name symmetric key");
 - is there not an error code that you can get and throw back to make 
it more useful?
 I noticed this applies to many other areas as well.  it's a good idea 
to get errorcode to throw back


* setNickName
 - you might want to check if nickname is null before passing it down to C

* might want to check if new JSS files created should bear the same 
Netscape etc. license


*PK11SymmetricKeyDeriver.c :  bestSlot = 
PK11_GetBestSlot(deriveMechanism, NULL);
 should test it out on hsm (already communicated to jack... item here 
to track)
  - suggest if issues found, revert to old code (or code similar to old 
code) and create a separate ticket to tackle

  - new code should test to work on both nethsm and lunasa

* the test SymKeyDeriving seems to fail at NSS init (Jack found the hard 
coded lib):  tracking here
--- End Message ---
--- Begin Message ---


=
SecureChannelProtocol.java

** diversifyKey()
  - I don't see KDD being checked for null when conext is being 
assigned to it.
  - in the 3 calls you made to standardKDF.getDiversificationData(), 
first param you passed in is "KDD". Could you mean "context"? because
   if nistSP800_108KdfUseCuidAsKdd is true, then your KDD is most 
likely null, isn't it?

 - I got so confused with these standardKDF.getDiversificationData() calls.
   reason being that the getDiversificationData() is really not 
specific to "standardKDF".  It is used and used if you choose 
NistSP800_108KDF too!!!
   How about moving the getDiversificationData() method to the base KDF 
class instead?


*computeSessionKey_SCP02()
note yet implemented, but your debug log says it's SCP01.

**computeMAC_SCP01()
 - are you sure the cipher needs to be initialized inside the loop 
instead of before the loop?

cipher.initEncrypt(symKey);
  - also, oddly, I don't see any 

Re: [Pki-devel] [pki-devel][PATCH]0061-Enhance-tkstool-for-capabilities-and-security.patch

2016-05-12 Thread John Magne
Ticket #1641 Enhance tkstool for capabilities and security

The key is now generated with the flags needed to keep the data from being 
displayed
with simple tools such as symkeyutil.


As per cfu's instructions,
I was able to test this with the nethsm only.

I also was able to make the key des3 and everything works fine with the master 
key.
This will help all the warnings we get about insecure des2 keys.

If there is a problem with luna, we can file another ticket.
Also there could be a built in tool for luna to generate keys such as is 
present on hsm.

Pushed to master.

- Original Message -
From: "Christina Fu" <c...@redhat.com>
To: pki-devel@redhat.com
Sent: Wednesday, January 27, 2016 10:24:26 AM
Subject: Re: [Pki-devel] 
[pki-devel][PATCH]0061-Enhance-tkstool-for-capabilities-and-security.patch

I think I will be more conservative and give conditional ACK to this patch 
pending on tests on servers running on both LunaSA and nethsm. Although the 
code in the patch might very well work for both, those two HSM's are known to 
require different sets of pk11AtrFlags and often one set would work for one but 
not the other. 

thanks, 
Christina 

On 01/15/2016 04:24 PM, John Magne wrote: 



Enhance tkstool for capabilities and security

This simple ticket is to fix tkstool to allow it
to create the master key with the proper flags to make
the key data private such that it can't be easily viewed when
using tools to print out sym keys on the token.

Fix tested on the "internal" token by trying the various tkstool
cmds to make sure having the key private does not cause issues.
Also tried a simple key changeover operation with tpsclient to make
sure that symkey can still do what it needs to do witht the master key.

Further testing with a full hsm will be required.
The goal was the create the key with the same flags that are used with the
previous "PK11_GenKeyOnToken" (name approx) is used. This version had no
flags and created a default set. This fix uses the version With flags and
does what the old one did, but made sure the key is private and sensitive.

Master key can be tested by using the tool:

/usr/lib64/nss/unsupported-tools/symkeyutil -d ./ -L 


___
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 mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel


Re: [Pki-devel] [pki-devel][PATCH] 0066-TPS-auth-special-characters-fix.patch

2016-05-03 Thread Christina Fu

ACK

On 04/27/2016 01:59 PM, John Magne wrote:

TPS auth special characters fix.
 
 Ticket #1636.

 Smartcard token enroll/format fails when the ldap user has special 
characters in userid or password
 
 Tested with both esc and tpsclient. The problem was when using a real card because the client uri encodes

 the authentication creds and the server needs to decode them.


___
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] [pki-devel][PATCH] 0066-TPS-auth-special-characters-fix.patch

2016-04-27 Thread John Magne
TPS auth special characters fix.

Ticket #1636.
Smartcard token enroll/format fails when the ldap user has special 
characters in userid or password

Tested with both esc and tpsclient. The problem was when using a real card 
because the client uri encodes
the authentication creds and the server needs to decode them.
From e6bcb9f1fac9c7db95a1aa4767cdfe6ac4ccbd16 Mon Sep 17 00:00:00 2001
From: Jack Magne 
Date: Wed, 27 Apr 2016 13:52:10 -0700
Subject: [PATCH] TPS auth special characters fix.

Ticket #1636.
Smartcard token enroll/format fails when the ldap user has special characters in userid or password

Tested with both esc and tpsclient. The problem was when using a real card because the client uri encodes
the authentication creds and the server needs to decode them.
---
 base/common/src/org/dogtagpki/tps/msg/TPSMessage.java | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/base/common/src/org/dogtagpki/tps/msg/TPSMessage.java b/base/common/src/org/dogtagpki/tps/msg/TPSMessage.java
index 84e991e..f622b9b 100644
--- a/base/common/src/org/dogtagpki/tps/msg/TPSMessage.java
+++ b/base/common/src/org/dogtagpki/tps/msg/TPSMessage.java
@@ -456,17 +456,17 @@ public class TPSMessage {
 break;
 case MSG_EXTENDED_LOGIN_RESPONSE:
 result =
-new ExtendedLoginResponseMsg(op_val,
-get(UID_NAME),
-get(PASSWORD_NAME),
-extsMap);
+new ExtendedLoginResponseMsg(op_val,
+Util.uriDecode(get(UID_NAME)),
+Util.uriDecode(get(PASSWORD_NAME)),
+extsMap);
 break;
 case MSG_LOGIN_REQUEST:
 break;
 case MSG_LOGIN_RESPONSE:
 result =
-new LoginResponseMsg(get(SCREEN_NAME_NAME),
-get(PASSWORD_NAME_1));
+new LoginResponseMsg(Util.uriDecode(get(SCREEN_NAME_NAME)),
+Util.uriDecode(get(PASSWORD_NAME_1)));
 break;
 case MSG_NEW_PIN_REQUEST:
 break;
-- 
2.5.0

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

Re: [Pki-devel] [pki-devel][PATCH] 0062-Allow-cert-and-key-indexes-9.patch

2016-02-05 Thread John Magne
ACKED by cfu:

Pushed to master: commit 9bd94a0a54793a0720b803846ce2291e5064c2ae

Various fedora libcoolkey builds done with patch to support this, winding 
through the system.

Closing #1734.

- Original Message -
From: "Christina Fu" <c...@redhat.com>
To: pki-devel@redhat.com
Sent: Friday, February 5, 2016 4:22:40 PM
Subject: Re: [Pki-devel] [pki-devel][PATCH] 
0062-Allow-cert-and-key-indexes-9.patch

the code looks good. 
I applied the patch and upgraded my libcoolkey and played with it. I was able 
to enroll for 2 certs and "recover" 5 (makes a total of 7), and then continued 
to run externalReg enrollment again to delete one cert and recover another. 

ACK, 
Christina 

On 02/02/2016 06:46 PM, John Magne wrote: 



Subject: [PATCH] Allow cert and key indexes > 9.

Ticket: Ticket #1734 : TPS issue with overflowing PKCS#11 cert index numbers

This patch contains the following:

1. Fixes in TPS to allow the server to set and read muscle object ID's that are 
greater than 9.

The id is stored as a single ASCII byte in the object id. Previous libcoolkey 
patches exist to now support numbers
larger than 9, by the following:

0-9 is represented by the ascii chars for 0 through 9,.
10 - 35 represented by the ascii chars for 'A' through 'Z'.
36 - 61 represented by the ascii chars for 'a' through 'z'.

Once coolkey is updated it will be able to read these id's.

TPS with this patch will be able to both read number 0 - 62 and to set them 
when creating pkcs#11 objects to be stored on the token.

When the proper libcoolkey is installed, the coolkey driver will be able to 
read certs and keys with id's > 9. Thus, for instance a cert with an id of C6, 
with keys of k12, and k13, will be supported and viewable in the Firefox cert 
viewer. Also the certs will be usable for operations.

2. A fix to the routine that finds a free id number to assign to a soon to be 
recovered cert will now have the ability to find unused slots instead of just 
inrementing one over the highest currently used index.

3. Made a couple of minor cleanup fixes to externalReg functionality discovered 
during testing of this feature.

Tested up to 7 certs on the token. Also did some re-tests of cfu's cert 
retention feature and those checked. 


___
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 mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel


[Pki-devel] [pki-devel][PATCH] 0062-Allow-cert-and-key-indexes-9.patch

2016-02-02 Thread John Magne
Subject: [PATCH] Allow cert and key indexes > 9.

Ticket: Ticket #1734 : TPS issue with overflowing PKCS#11 cert index numbers

This patch contains the following:

1. Fixes in TPS to allow the server to set and read muscle object ID's that are 
greater than 9.

The id is stored as a single ASCII byte in the object id. Previous libcoolkey 
patches exist to now support numbers
larger than 9, by the following:

0-9 is represented by the ascii chars for 0 through 9,.
10 - 35 represented by the ascii chars for 'A' through 'Z'.
36 - 61 represented by the ascii chars for 'a' through 'z'.

Once coolkey is updated it will be able to read these id's.

TPS with this patch will be able to both read number 0 - 62 and to set them 
when creating pkcs#11 objects to be stored on the token.

When the proper libcoolkey is installed, the coolkey driver will be able to 
read certs and keys with id's > 9. Thus, for instance a cert with an id of C6, 
with keys of k12, and k13, will be supported and viewable in the Firefox cert 
viewer. Also the certs will be usable for operations.

2. A fix to the routine that finds a free id number to assign to a soon to be 
recovered cert will now have the ability to find unused slots instead of just 
inrementing one over the highest currently used index.

3. Made a couple of minor cleanup fixes to externalReg functionality discovered 
during testing of this feature.

Tested up to 7 certs on the token. Also did some re-tests of cfu's cert 
retention feature and those checked.
From 911d7fde7a49d2f854f391ea95771b4000c8535e Mon Sep 17 00:00:00 2001
From: Jack Magne 
Date: Fri, 22 Jan 2016 18:03:36 -0800
Subject: [PATCH] Allow cert and key indexes > 9.

Ticket: Ticket #1734 : TPS issue with overflowing PKCS#11 cert index numbers

This patch contains the following:

1. Fixes in TPS to allow the server to set and read muscle object ID's that are greater than 9.

The id is stored as a single ASCII byte in the object id. Previous libcoolkey patches exist to now support numbers
larger than 9, by the following:

0-9 is represented by the ascii chars for 0 through 9,.
10 - 35 represented by the ascii chars for 'A' through 'Z'.
36 - 61 represented by the ascii chars for 'a' through 'z'.

Once coolkey is updated it will be able to read these id's.

TPS with this patch will be able to both read number 0 - 62 and to set them when creating pkcs#11 objects to be stored on the token.

When the proper libcoolkey is installed, the coolkey driver will be able to read certs and keys with id's > 9. Thus, for instance a cert with an id of C6, with keys of k12, and k13, will be supported and viewable in the Firefox cert viewer. Also the certs will be usable for operations.

2. A fix to the routine that finds a free id number to assign to a soon to be recovered cert will now have the ability to find unused slots instead of just inrementing one over the highest currently used index.

3. Made a couple of minor cleanup fixes to externalReg functionality discovered during testing of this feature.
---
 .../org/dogtagpki/server/tps/main/ObjectSpec.java  | 208 +++-
 .../org/dogtagpki/server/tps/main/PKCS11Obj.java   |  92 -
 .../server/tps/processor/CertEnrollInfo.java   |   9 +-
 .../server/tps/processor/EnrolledCertsInfo.java|   7 +
 .../server/tps/processor/TPSEnrollProcessor.java   | 213 -
 5 files changed, 380 insertions(+), 149 deletions(-)

diff --git a/base/tps/src/org/dogtagpki/server/tps/main/ObjectSpec.java b/base/tps/src/org/dogtagpki/server/tps/main/ObjectSpec.java
index a8dbdb1..00cc447 100644
--- a/base/tps/src/org/dogtagpki/server/tps/main/ObjectSpec.java
+++ b/base/tps/src/org/dogtagpki/server/tps/main/ObjectSpec.java
@@ -236,7 +236,8 @@ public class ObjectSpec {
 // down to the cert's id, the code below changes both "4" and "5" back
 // to "2".
 
-int val = (objectID.charAt(1) - '0');
+int val = objectSpec.getObjectIndex();
+
 switch (objectID.charAt(0)) {
 case 'c':
 
@@ -290,7 +291,7 @@ public class ObjectSpec {
 
 fixedAttrs = 0x0080; /* CKA_TOKEN */
 xclass = (int) PKCS11Constants.CKO_CERTIFICATE;
-id = objectID.charAt(1) - '0';
+id = objectSpec.getObjectIndex();
 
 objectSpec.setFixedAttributes(fixedAttrs | (xclass << 4) | id);
 }
@@ -453,4 +454,207 @@ public class ObjectSpec {
 return data;
 }
 
+public int getObjectIndex() {
+return ObjectSpec.getObjectIndex(this.objectID);
+}
+
+public static int getObjectIndex(long objectID) {
+char char_index = (char) ((objectID >> 16) & 0xff);
+int index = -1;
+
+if (char_index >= '0' && char_index <= '9') {
+index = char_index - '0';
+}
+if (char_index >= 'A' && char_index <= 'Z') {
+index = char_index - 'A' + 10;
+}
+if (char_index >= 'a' && char_index <= 'z') {
+

Re: [Pki-devel] [pki-devel][PATCH]0061-Enhance-tkstool-for-capabilities-and-security.patch

2016-01-27 Thread Christina Fu
I think I will be more conservative and give conditional ACK to this 
patch pending on tests on servers running on both LunaSA and nethsm.  
Although the code in the patch might very well work for both, those two 
HSM's are known to require different sets of pk11AtrFlags and often one 
set would work for one but not the other.


thanks,
Christina

On 01/15/2016 04:24 PM, John Magne wrote:

Enhance tkstool for capabilities and security

This simple ticket is to fix tkstool to allow it
to create the master key with the proper flags to make
the key data private such that it can't be easily viewed when
using tools to print out sym keys on the token.

Fix tested on the "internal" token by trying the various tkstool
cmds to make sure having the key private does not cause issues.
Also tried a simple key changeover operation with tpsclient to make
sure that symkey can still do what it needs to do witht the master key.

Further testing with a full hsm will be required.
The goal was the create the key with the same flags that are used with the
previous "PK11_GenKeyOnToken" (name approx) is used. This version had no
flags and created a default set. This fix uses the version With flags and
does what the old one did, but made sure the key is private and sensitive.

Master key can be tested by using the tool:

/usr/lib64/nss/unsupported-tools/symkeyutil -d ./ -L




___
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