Based on CFU's conditional verbal ACK, addressed the issues and pushed to master.
Ticket # 885.
----- Original Message -----
From: "Christina Fu" <cfu(a)redhat.com>
To: pki-devel(a)redhat.com
Sent: Tuesday, July 22, 2014 5:28:54 PM
Subject: Re: [Pki-devel] [pki-devel][PATCH] 0015-First-cut-of-enrollment-feature.patch
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(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel