On 9/12/2016 2:41 PM, Yann Ylavic wrote:
On Mon, Sep 12, 2016 at 10:31 AM, Ewald Dieterich <ew...@mailbox.org> wrote:
On 06/13/2016 09:38 AM, Ewald Dieterich wrote:
Looks like the problem is this:
Thanks for invertigating!
Yes, I recently found a case where this error comes up as well and this
investigation helped a lot. I'm also interested in resuming the
discussion about potentially fixing it.
Thanks for the patch, possibly a simpler way to fix it would be:
Index: modules/session/mod_session_crypto.c
===================================================================
--- modules/session/mod_session_crypto.c (revision 1760149)
+++ modules/session/mod_session_crypto.c (working copy)
@@ -246,6 +246,8 @@ static apr_status_t decrypt_string(request_rec * r
return res;
}
+ key = apr_pcalloc(r->pool, sizeof *key);
+
/* try each passphrase in turn */
for (; i < dconf->passphrases->nelts; i++) {
const char *passphrase = APR_ARRAY_IDX(dconf->passphrases, i, char *);
_
so that crypto_passphrase() will use the given key instead of
allocating a new one.
From my understanding, apr_crypto_key_t is an opaque struct defined
separately by each crypto provider, so mod_session_crypto will not be
able to do the sizeof.
My initial reaction is thinking that the crypto providers should be
allocating the keys using the pool passed to apr_crypto_passphrase
instead of the one passed to apr_crypto_make. But it looks like a
relatively recent change to APR trunk [1] makes it more explicit in the
documentation that the allocated keys last until the context is cleaned up.
That implies we either need a context per request, like Ewald's patch,
or use a lock (to avoid multithreded pool access) and reuse the keys (to
avoid increased memory consumption). But the keys may change per
directory, so I don't know if that's feasible.
[1] https://svn.apache.org/viewvc?view=revision&revision=1752008
Regards,
Paul Spangler
LabVIEW R&D
National Instruments
