You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Yann Ylavic <yl...@gmail.com> on 2016/10/05 10:23:54 UTC
Re: Random AH01842 errors in mod_session_crypto
[Adding dev@apr, with a little abstract]
On Mon, Sep 12, 2016 at 10:31 AM, Ewald Dieterich <ew...@mailbox.org> wrote:
>
> Looks like the problem is this:
>
> * In session_crypto_init() a crypto context is created from a global pool
> (server->pconf).
> * In encrypt_string() and decrypt_string() a key is created from the context
> via apr_crypto_passphrase() using the global pool for allocating memory for
> the key.
> * Multiple threads might access the global pool at the same time.
Unless given an already allocated key, apr_crypto_passphrase() and
apr_crypto_key() will create/allocate the key from the pool of an
apr_crypto_t (the *const* context), not the given pool.
So we tried to allocate the key before the call to apr_crypto_passphrase()...
On Tue, Oct 4, 2016 at 6:21 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Tue, Oct 4, 2016 at 5:29 PM, Graham Leggett <mi...@sharp.fm>
> wrote:
>> On 4 Oct 2016, at 15:47, Paul Spangler <pa...@ni.com>
>> wrote:
>>
>>> 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.
>>
>> That's a sizeof a pointer to apr_crypto_key_t, not the sizeof
>> apr_crypto_key_t itself.
>
> I think Paul is correct, apr_crypto_passphrase() requires its given
> *(apr_crypto_key_t**)key to be not NULL, otherwise it will allocate
> one from its (providers's) array, which is not thread safe.
>
> How are we supposed to have a *key not NULL given apr_crypto_key_t is
> opaque?
... no way.
>>
>> Keys are read at server start and reused. Trying to regenerate the
>> key on every request has performance implications.
Looks like the heavy operation is creating the context (apr_crypto_t),
not the key, but anyway the keys need to be generated at run time,
taking the salt into account.
So I don't see the point of caching (and allocating) the keys in the
context while we could use the given runtime pool.
If the user wanted some cache s/he could still handle it based on
her/his own pool, but:
> It seems that apr_crypto_passphrase()'s **key is updated for each
> call, though...
so we can't really cache it anyway.
Hence I propose to remove the keys' cache in apr_crypto, and let the
things work with the given pool(s)...
Patch attached, WDYT?
Regards,
Yann.
Re: Random AH01842 errors in mod_session_crypto
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Fri, Dec 2, 2016 at 4:28 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Fri, Dec 2, 2016 at 10:06 PM, Yann Ylavic <yl...@gmail.com> wrote:
> > On Wed, Oct 5, 2016 at 12:23 PM, Yann Ylavic <yl...@gmail.com>
> wrote:
> >>
> >> Patch attached, WDYT?
> >
> > Ping, probably is worth considering for 1.6 (even 1.5) ?
>
> Committed to APR trunk (r1772414), 1.6.x (r1772415) and 1.5.x (r1772416).
>
Excellent! Docs edits also welcome for clarification.
Re: Random AH01842 errors in mod_session_crypto
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Fri, Dec 2, 2016 at 4:28 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Fri, Dec 2, 2016 at 10:06 PM, Yann Ylavic <yl...@gmail.com> wrote:
> > On Wed, Oct 5, 2016 at 12:23 PM, Yann Ylavic <yl...@gmail.com>
> wrote:
> >>
> >> Patch attached, WDYT?
> >
> > Ping, probably is worth considering for 1.6 (even 1.5) ?
>
> Committed to APR trunk (r1772414), 1.6.x (r1772415) and 1.5.x (r1772416).
>
Excellent! Docs edits also welcome for clarification.
Re: Random AH01842 errors in mod_session_crypto
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Dec 2, 2016 at 10:06 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Wed, Oct 5, 2016 at 12:23 PM, Yann Ylavic <yl...@gmail.com> wrote:
>>
>> Patch attached, WDYT?
>
> Ping, probably is worth considering for 1.6 (even 1.5) ?
Committed to APR trunk (r1772414), 1.6.x (r1772415) and 1.5.x (r1772416).
Regards,
Yann.
Re: Random AH01842 errors in mod_session_crypto
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Fri, Dec 2, 2016 at 3:06 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Wed, Oct 5, 2016 at 12:23 PM, Yann Ylavic <yl...@gmail.com> wrote:
> >
> > Patch attached, WDYT?
>
> Ping, probably is worth considering for 1.6 (even 1.5) ?
>
Provided you don't *break* the API contract with 1.x.x, corrections
can be applied to 1.(current). This looks like it falls in that category.
Things get wonky when you have duplicates-permitted and decide
that's bad, so change the behavior to duplicates-disallowed. That
would actually violate any 1.x release update.
But as long as we are conforming the behavior to the published
description, bug fixes are welcome.
Re: Random AH01842 errors in mod_session_crypto
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Fri, Dec 2, 2016 at 3:06 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Wed, Oct 5, 2016 at 12:23 PM, Yann Ylavic <yl...@gmail.com> wrote:
> >
> > Patch attached, WDYT?
>
> Ping, probably is worth considering for 1.6 (even 1.5) ?
>
Provided you don't *break* the API contract with 1.x.x, corrections
can be applied to 1.(current). This looks like it falls in that category.
Things get wonky when you have duplicates-permitted and decide
that's bad, so change the behavior to duplicates-disallowed. That
would actually violate any 1.x release update.
But as long as we are conforming the behavior to the published
description, bug fixes are welcome.
Re: Random AH01842 errors in mod_session_crypto
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Dec 2, 2016 at 10:06 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Wed, Oct 5, 2016 at 12:23 PM, Yann Ylavic <yl...@gmail.com> wrote:
>>
>> Patch attached, WDYT?
>
> Ping, probably is worth considering for 1.6 (even 1.5) ?
Committed to APR trunk (r1772414), 1.6.x (r1772415) and 1.5.x (r1772416).
Regards,
Yann.
Re: Random AH01842 errors in mod_session_crypto
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Oct 5, 2016 at 12:23 PM, Yann Ylavic <yl...@gmail.com> wrote:
>
> Patch attached, WDYT?
Ping, probably is worth considering for 1.6 (even 1.5) ?
>
> Regards,
> Yann.
Re: Random AH01842 errors in mod_session_crypto
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Oct 5, 2016 at 12:23 PM, Yann Ylavic <yl...@gmail.com> wrote:
>
> Patch attached, WDYT?
Ping, probably is worth considering for 1.6 (even 1.5) ?
>
> Regards,
> Yann.