https://issues.apache.org/bugzilla/show_bug.cgi?id=54357

--- Comment #22 from Kaspar Brand <[email protected]> ---
(In reply to Alex Bligh from comment #20)
> However, now we are using stapling_cert_info, that no longer holds true (I
> think). That's because we need to store the SHA1 of each certificate
> somewhere
> anyway, because those SHA1 values (now) form the key to the
> stapling_cert_info
> hash.

Ah yes, you're right. I didn't realize that the stapling_cert_info hash is
populated with apr_hash_set()...

> I suppose I am assuming (possible wrongly) that apr_hash_set isn't actually
> making a copy of the key here.

... and your assumption is correct, of course (it's indeed a straightforward
way of keeping the idx around).

Two comments about v4: 

- In ssl_stapling_init_cert, we're potentially allocating/"leaking" unneeded
certinfo structs, I think. If a specific certificate is configured for more
than one vhost (or at restart), we're constructing "cinf" even if we might not
really need it later for apr_hash_set. Is it possible to first check with
apr_hash_get if there's already a suitable entry in stapling_cert_info, and
return early in this case? (Needs a local idx[] for temporary storage of the
X509_digest result, I guess.)

- It is actually safe to use

        cinf->uri = apr_pstrdup(p, sk_OPENSSL_STRING_value(aia, 0));

in ssl_stapling_init_cert (without checking the intermediate result of
sk_OPENSSL_STRING_value), as aia will only be non-NULL if the stack includes at
least one element (and furthermore, apr_pstrdup has a non-NULL check for the
second argument).

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to