The patch does not help but I think it got me on the right track though I'm
a bit confused about the 'dirty' flag. Where is that flag supposed to be
used ? In both trunk and 2.4.7 I only found one place
(./modules/session/mod_session.c:200) where that flag is used but none that
remotely looked like triggering a session/cookie replacing.
I assume the real problem lies in mod_session's ap_session_load(). There
the comment says "If the session doesn't exist, a blank one will be
created." but that's simply not true if the session decryption failed.
Watching the session_rec pointer it will be NULL even after calling
ap_session_load(). This holds true for the subsequent calls to
ap_session_get() in get_session_auth() as well.If you look at mod_session's
ap_session_load() it's obvious why: The session_rec pointer z will never be
set if decoding the session failed.
I would expect a failure to decode the session to result in some form of
reset on the session so the surrounding code can go ahead and set
everything as required, including encrypting the session with a new key.
Looking at mod_auth_form's get_session_auth() this seems to be the
assumption there.
I thought of something as simple as
diff --git a/modules/session/mod_session.c b/modules/session/mod_session.c
index a3354a5..c3c2f27 100644
--- a/modules/session/mod_session.c
+++ b/modules/session/mod_session.c
@@ -142,6 +142,15 @@ static apr_status_t ap_session_load(request_rec * r,
session_rec ** z)
ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01817)
"error while decoding the session, "
"session not loaded: %s", r->uri);
+
+ /* no luck, create a blank session */
+ zz = (session_rec *) apr_pcalloc(r->pool, sizeof(session_rec));
+ zz->pool = r->pool;
+ zz->entries = apr_table_make(zz->pool, 10);
+ zz->uuid = (apr_uuid_t *) apr_pcalloc(zz->pool,
sizeof(apr_uuid_t));
+ apr_uuid_get(zz->uuid);
+ *z = zz;
+
return rv;
}
}
but that didn't do the trick. Going to take another look at it tomorrow.
On Thu, Dec 12, 2013 at 12:25 AM, Graham Leggett <[email protected]> wrote:
> On 09 Dec 2013, at 10:50 AM, Thomas Eckert <[email protected]>
> wrote:
>
> > So it should work out of the box. I figured as much but was unsure
> whether I hit a bug or forgot a configuration directive. Will look into it
> once I have the time :-/
>
> Here is an untested patch, can you give it a try and confirm?
>
> Index: modules/session/mod_session_crypto.c
> ===================================================================
> --- modules/session/mod_session_crypto.c (revision 1550312)
> +++ modules/session/mod_session_crypto.c (working copy)
> @@ -222,7 +222,7 @@
> * Returns APR_SUCCESS if successful.
> */
> static apr_status_t decrypt_string(request_rec * r, const apr_crypto_t *f,
> - session_crypto_dir_conf *dconf, const char *in, char **out)
> + session_crypto_dir_conf *dconf, const char *in, char **out, int
> *dirty)
> {
> apr_status_t res;
> apr_crypto_key_t *key = NULL;
> @@ -252,6 +252,9 @@
> apr_size_t len = decodedlen;
> char *slider = decoded;
>
> + /* if not first passphrase, mark the session as dirty */
> + *dirty = *dirty & (i > 0);
> +
> /* encrypt using the first passphrase in the list */
> res = apr_crypto_passphrase(&key, &ivSize, passphrase,
> strlen(passphrase),
> @@ -382,7 +385,7 @@
> if ((dconf->passphrases_set) && z->encoded && *z->encoded) {
> apr_pool_userdata_get((void **)&f, CRYPTO_KEY,
> r->server->process->pconf);
> - res = decrypt_string(r, f, dconf, z->encoded, &encoded);
> + res = decrypt_string(r, f, dconf, z->encoded, &encoded,
> &z->dirty);
> if (res != APR_SUCCESS) {
> ap_log_rerror(APLOG_MARK, APLOG_ERR, res, r, APLOGNO(01842)
> "decrypt session failed, wrong passphrase?");
>
>
> Regards,
> Graham
> --
>
>
