https://issues.apache.org/bugzilla/show_bug.cgi?id=54357
--- Comment #20 from Alex Bligh <[email protected]> --- (In reply to Kaspar Brand from comment #19) > (In reply to Alex Bligh from comment #16) > > 1. Does the OCSP somehow requires a state per vhost - for instance what > > happens if 2 sites have a different stapling_force_url for the same > > certificate? As SSLStaplingForceURL is a vhost ctx directive, wouldn't this > > introduce a back compatibility problem? > > No, it doesn't. stapling_renew_response() favors stapling_force_url over all > certs for a given vhost, i.e. any OCSP URI from stapling_cert_info (i.e. the > cert itself) is ignored in this case. Oh I see. So ssl_stapling_init_cert remains done once (or more) per vhost, but we use a per SSL server cache. We keep the check for no stapling URI and no force URI *on that server*, then if the cert has no URI we error if any of the vhosts using that cert have no force URI. Presumably I then need to use the global SSL state's pool rather than the server pool, which means I can drop the changes to pass pool in and use (I presume) myModConfig(s)->pPool or similar. > (In reply to Alex Bligh from comment #17) > > * Drop use of idx member from cert_info structure. Reason: the > > index to the apr hash structure has to be held somewhere for all > > the recorded certificates. Whilst we do calculate the SHA1 hash > > on the fly, the apr hash algorithm needs something to compare it > > to (i.e. the SHA1s of all the recorded certificates) so it can > > index the correct URI. As the SHA1 value needs to be stored > > somewhere for each certificate, it might as well be stored in > > the struct. It's possible I've misunderstood what Kaspar was > > suggesting here. > > Previously the SHA-1 hash was used as a key for the OCSP response cache > (mc->stapling_cache), and as there was ex_info attached to each certificate, > it was convenient to have it only calculated once. But now that we always > have to calculate the hash in stapling_cb()/stapling_get_cert_info() anyway, > we could just pass it directly to stapling_cache_response() and > stapling_get_cached_response() (these are the only places where cinf->idx is > still used) and avoid the reduntant storage. I.e., you could merge the code > from staping_get_cert_info() right into stapling_cb() and use a local "idx" > which you would pass as an argument to stapling_get_cached_response() and > stapling_renew_response() (which in turn would supply it to > stapling_cache_response()). I get the feeling I'm being dumb here. Yes, I understand that the code does not use cinf->idx /afterwards/ save where you point out and if it did it could simply use the locally generated idx. Or to put it another way, before my changes we could have locally generated idx with an X.509 digest in stapling_cb()/stapling_get_cert_info(). 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. IE when we create the local X.509 SHA1, we then need to go through each of the cert_info structures within the apr hash to determine which ones match, so we can get access to the OTHER fields within cert_info. Therefore the SHA1 of each certificate needs to be stored somewhere with a reference to the relevant cert_info structure, and as good a place as any is to store it within the cert_info structure itself - partly because that's minimum code change, and partly because frankly where else are you going to store it? I suppose I am assuming (possible wrongly) that apr_hash_set isn't actually making a copy of the key here. Again, perhaps I'm missing the point here. -- 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]
