On 17/09/2011 11:37, Kaspar Brand wrote: > On 14.09.2011 22:32, Daniel Ruggeri wrote: >> My usage tests pass muster with the approach we have discussed, so I >> have updated trunk and the 2.2 backport proposal. At this point, I am >> satisfied with this particular patch, though I won't lose sight of the >> server-side issue. Since the patch should now be complete, I have given >> my vote in the 2.2 STATUS file and would appreciate any further >> review/votes. > > Looks good, judging from my (admittedly not very comprehensive) testing. > >> + chain = X509_STORE_CTX_get1_chain(sctx); >> + sk_X509_shift(chain); >> + i=sk_X509_num(chain); >> pkp->ca_certs[n] = chain; > > I think with the "sk_X509_shift(chain)" line, we're leaking an X509 > struct... maybe Steve can confirm (?). Adding a short comment as to why > the first cert is dropped would be useful IMO, too. >
Yes you need store the returned value and free it with X509_free(). Note also that because you ignore return values of X509_verify_cert() you might have a situation where the chain is not complete and so deleting the last element will remove a non-root CA. Steve. -- Dr Stephen Henson. OpenSSL Software Foundation, Inc. 1829 Mount Ephraim Road Adamstown, MD 21710 +1 877-673-6775 [email protected]
