Re: [Pki-devel] Review Request: Token Management System ECC infrastructure
Just a few Comments/Questions ...
GenerateKeyPairServlet.java
Apparently StringTokenizer is discouraged in new code in favor of String.split.
Could the following code be made a bit more efficient using some built in container? :
// is the specified curve supported?
61 + boolean isSupportedCurve = false;
62 + for (int i=0; i<supportedECCurves.length; i++) {
63 + if (rKeycurve.equals(supportedECCurves[i])) {
64 + isSupportedCurve = true;
65 + }
66 + }
RA_Enroll_Processor.cpp
In this code:
rv = ATOB_ConvertAsciiToItem (&der, pKey_ascii);
293 if (rv != SECSuccess){
294 - RA::Debug(LL_PER_CONNECTION,FN,
295 - "failed to convert b64 private key to binary");
296 - SECITEM_FreeItem(&der, PR_FALSE);
297 - status = STATUS_ERROR_MAC_ENROLL_PDU;
298 - PR_snprintf(audit_msg, 512, "ServerSideKeyGen: failed to convert b64
private key to binary");
299 - goto loser;
300 - }else {
If the call to ATOB_ConvertAsciiToItem fails, will there be a "der" variable to
free with SECITEM_FreeItem ?
Also, I'm curious why the change of the original code? Looks like we've added an
extra step of processing as here:
+ Buffer *decodePubKey = Util::URLDecode(pKey);
287 + char *pKey_ascii =
288 + BTOA_DataToAscii(decodePubKey->getBuf(), decodePubKey->getLen());
How are we freeing the eccParams? I see the commented out section. Does it happen later or
automatically somewhere?
Buffer.cpp/Buffer.h
Looks like we've added functions to the buffer that are already present.
The length is gotten with the method "size()". TPS_PUBLIC unsigned int size()
const { return len; }
The raw buffer is obtainable with the operator "()". TPS_PUBLIC operator BYTE*()
{ return buf; }
SecureChannel.cpp
Looks like debug buffer calls like the following were previously commented out:
RA::DebugBuffer("Secure_Channel::ComputeAPDUMac", "Data To
MAC'ed",
1387 + &data);
Was this done for a reason? I ask because we are actually now issuing these debug
commands.
----- Original Message -----
From: "Christina Fu" <cfu(a)redhat.com>
To: "pki-devel" <pki-devel(a)redhat.com>
Sent: Tuesday, September 11, 2012 6:19:16 PM
Subject: [Pki-devel] Review Request: Token Management System ECC infrastructure
https://fedorahosted.org/pki/attachment/ticket/304/TPS-ECC.patch2
This patch provides TMS ECC infrastructure as described in task #304:
https://fedorahosted.org/pki/ticket/304
I have merged/sanitized the code from two sources:
* Token ECC enrollment with client-side key generation support (provided
by jmagne(a)redhat.com)
* TMS ECC enrollment with server-side key generation and key archival
support (provided by myself - cfu(a)redhat.com)
The following tests have been conducted:
* ECC enrollment via tpsclient
* RSA enrollment via tpsclient
* RSA server-side key generation via tpsclient
* ECC server-side key generation via tpsclient
* ECC enrollment via smart card token (Safenet 330j)
* RSA enrollment via smart card token (Safenet sc650)
note 1: For ECC enrollments, you will need a newer java applet, which is
not yet ready for checkin.
note 2: server-side key generation is currently not yet supported by the
smart card token because of the lack of the key injection code, which
will be covered by task #235 (https://fedorahosted.org/pki/ticket/235)
thanks,
Christina
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel