This is the bulk of the work that writes objects to the token with a lot of goodies.

The following are my my review comments for 0015-First-cut-of-end-to-end-enrollment-feature.patch

* CreatePinAPDU.java - It appears that the old TPS has SetP1(p1).  Is there a reason why you removed that here?

* SecureChannel.java
   -appendKeyCapabilities:  the getting keyCapabilities part from the config is very repetitive.  You might want to write a convenience routine to iterate through all capabilities.
    - for most perms, you did:
+        TPSBuffer perms = new TPSBuffer();
+
+        perms.add((byte) 0xff);
+        perms.add((byte) 0xff);
+        perms.add((byte) 0x40);
+        perms.add((byte) 0x00);
+        perms.add((byte) 0x40);
+        perms.add((byte) 0x00);
      but for "createCertificate" you did:
+        byte[] perms = { (byte) 0xff, (byte) 0xff, 0x40, 0x00, 0x40, 0x00 };
+
+        TPSBuffer permissions = new TPSBuffer(perms);

both achieve the same thing, but you probably want to be consistent.  The 2nd way seems cleaner

  - writeObject() throws TPSStatus.STATUS_ERROR_MAC_ENROLL_PDU in case of response checkResult failure.  Is it always the same error at each call?
  How about createObject()?

 * PKCS11Obj
 - addObjectSpec() - I think it'd be more useful if you put objectId in the the debug message when one duplicated object is removed.
  - getRawHeaderData() - typo "returing"

 *TPSProcessor
 - mapPattern() - some debug messages call themselves buildTokenLabel instead...
 - mapPattern() -   I am not sure it is what you wanted.  You had
+        if ((nextPos - firstPos) <= 1) {
+            return "token";
+        }
   that will literally return "token" if the pattern turns out to be a string of non-tokens (not pattern).
  What you want to do is to return the exact same pattern if it contains no '$'.

 * makeKeyIDFromPublicKeyInfo() - Most our existing code call MessageDigest like this:
MessageDigest md = MessageDigest.getInstance("SHA1");
I'm curious if it makes any difference if you specify the provider or not?
 I'm also not so sure "mozilla" is the appropriate name for the digesgt (I know we call our JSS provider "Mozilla-JSS", but a message digest is just a message digest)

* about authentication: per our discussion, with what we have now, the CS.cfg configuration needs to be manually changed and server restarted between swapping the clients between ESC and tpsclient.  I will just add my changes after your patch is checked in.

Christina


On 07/18/2014 11:26 AM, John Magne wrote:
 First cut of enrollment feature.

The following features implemented for enrollment.

1. Standard enrollment of a list of RSA certificates.
2. Certificates are only done with token side keygen.
3. Minimual enrollment based pin reset functionality implemented to create
a pin for the enrolled token.
4. Much work done to the PKCS11 object code, which allows us to write the
compressed object blob to the token, allowing coolkey to access it and use
the certs and keys on the token.
5. Tested with Bob Relyea's "smartcard" utility to prove that signing and encryption
operations worked as expected.
6. Some work done to get authentication working with esc.
7. Created of stub of standalone Pin Reset Processor. Now it returns an error from
esc but the pin reset command is accepted.

To Do.

1. We need to support server side keygen.
2. Symmetric Key Changeover in another ticket.
3. Finish up the stand alone Pin Reset Processor in another ticket.


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