[Pki-devel] [pki-devel][PATCH] 0015-First-cut-of-enrollment-feature.patch

Christina Fu cfu at redhat.com
Wed Jul 23 00:28:54 UTC 2014


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 at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20140722/50e86295/attachment.htm>


More information about the Pki-devel mailing list