Re: [Pki-devel] Review Request: TMS ECC Key Recovery
Hi Jack,
Thanks for the review. Please find my response in-line below.
thanks,
Christina
On 09/26/2012 06:39 PM, John Magne wrote:
Couple Questions:
In the following piece of the patch:
+ SECITEM_FreeItem(&der, PR_FALSE);
81 SECKEY_DestroySubjectPublicKeyInfo(spki);
82
It looks like both are executed , even when the call to
spki = SECKEY_DecodeDERSubjectPublicKeyInfo(&der);
Fails and spki is NULL. Does the DestroySubjectPublicKeyInfo handle this? Also Will
"der" be in a state to harm the call to SECITEM_FreeItem?
DestroySubjectPublicKeyInfo checks before it frees so it is okay if spki happens to be
null:
void
SECKEY_DestroySubjectPublicKeyInfo(CERTSubjectPublicKeyInfo *spki)
{
if (spki && spki->arena) {
PORT_FreeArena(spki->arena, PR_FALSE);
}
}
Also, the content of der is copied over in
SECKEY_DecodeDERSubjectPublicKeyInfo(), so once the call is done, the
code has no use for der any more, hence the SECITEM_FreeItem after.
Also, SECITEM_FreeItem checks for null before it frees as well, so it's
safe to call.
Index: tps/src/engine/RA.cpp
187 ===================================================================
188 --- tps/src/engine/RA.cpp (revision 2497)
189 +++ tps/src/engine/RA.cpp (working copy)
190 @@ -1238,7 +1238,10 @@
191 goto loser;
192 } else {
193 RA::Debug(LL_PER_PDU, "RecoverKey", "got public key =%s",
tmp);
194 - *publicKey_s = PL_strdup(tmp);
195 + char *tmp_publicKey_s = PL_strdup(tmp);
196 + Buffer *decodePubKey = Util::URLDecode(tmp_publicKey_s);
197 + *publicKey_s =
198 + BTOA_DataToAscii(decodePubKey->getBuf(),
decodePubKey->getLen());
199 }
200
201 tmp = NULL;
202 @@ -1256,7 +1259,7 @@
203 RA::Error(LL_PER_PDU, "RecoverKey",
204 "did not get iv_param for recovered key in DRM response");
205 } else {
206 - RA::Debug(LL_PER_PDU, "ServerSideKeyGen", "got iv_param for
recovered key =%s", tmp);
207 + RA::Debug(LL_PER_PDU, "RecoverKey", "got iv_param for
recovered key =%s", tmp);
208 *ivParam_s = PL_strdup(tmp);
209 }
In the above code we are doing a strdup giving publicKey_s.
Are we freeing that string anywhere?
I introduced two variables there. Yes, I should free them.
----- Original Message -----
From: "Christina Fu"<cfu(a)redhat.com>
To: "pki-devel"<pki-devel(a)redhat.com>
Sent: Monday, September 24, 2012 4:06:32 PM
Subject: [Pki-devel] Review Request: TMS ECC Key Recovery
https://fedorahosted.org/pki/ticket/252 - TMS - ECC Key Recovery
patch for review:
https://fedorahosted.org/pki/attachment/ticket/252/TMS-ECC-Recovery.patch...
This patch provides code to allow ECC key recovery in the TMS environment.
It was tested to work with tpsclient. The key injection part of
implementation for the actual smart card tokens is scheduled to be done
in #235 at a later time.
I can do a demo tomorrow in office.
thanks,
Christina
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel