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. Second point, somewhat related: if we end up with an empty chain at this point, then it seems preferrable to me to discard/free the STACK_OF(X509) right on the spot (and not store it in pkp->ca_certs). Kaspar
