[Pki-devel] Review Request: TMS ECC Key Recovery
Christina Fu
cfu at redhat.com
Thu Sep 27 17:56:18 UTC 2012
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 at redhat.com>
> To: "pki-devel"<pki-devel at 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.patchForReview
>
> 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel
More information about the Pki-devel
mailing list