[gnutls-help] Session resumption/unpack error while getting timestamp, expired sessions/db clean questions

[Jonathan Roudiere]

Hello Nikos,

On GnuTLS 3.2.x stable release, when session db cache is used,
timestamp can't be got through gnutls_db_check_entry_time().

- When session are packed into _gnutls_session_pack() magic number
(PACKED_SESSION_MAGIC) - and security_parameters.timestamp - are
copied in big endian order (through BUFFER_APPEND_NUM(), ...
_gnutls_write_uint32() ... _gnutls_buffer_append_data()) but when they
are unpacked in gnutls_db_check_entry_time() they are memcopied
without any treatment thus on x86 they are badly unpacked and this
forbid to get timestamp.

The following patch solve the problem.

diff --git a/lib/gnutls_db.c b/lib/gnutls_db.c
index 2d9b744..faa158c 100644
--- a/lib/gnutls_db.c
+++ b/lib/gnutls_db.c
@@ -164,12 +164,12 @@ time_t gnutls_db_check_entry_time(gnutls_datum_t * entry)
    if (entry->size < 8)
        return gnutls_assert_val(0);

-   memcpy(&magic, entry->data, 4);
+   magic = _gnutls_read_uint32(entry->data);

    if (magic != PACKED_SESSION_MAGIC)
        return gnutls_assert_val(0);

-   memcpy(&t, &entry->data[4], 4);
+   t = _gnutls_read_uint32(&entry->data[4]);

    return t;
 }

- Else, db remove function is never called internally by GnuTLS even
if timestamp is checked against expire_time when a session is unpacked
through _gnutls_session_unpack(). In _gnutls_server_restore_session()
a comment specifies "expiration check is performed inside" but no
action are taken in order to remove expired session from the cache/db.

Is GnuTLS should not remove session from cache/db in this case ? I
have patched GnuTLS in order to call the db_remove_func function from
_gnutls_server_restore_session(), I don't know if this hurt GnuTLS
design and should be done in another place or not at all (initially
patch was against _gnutls_session_unpack()).

diff --git a/lib/gnutls_db.c b/lib/gnutls_db.c
index 2d9b744..6d860a4 100644
--- a/lib/gnutls_db.c
+++ b/lib/gnutls_db.c
@@ -285,6 +285,9 @@ _gnutls_server_restore_session(gnutls_session_t session,

     /* expiration check is performed inside */
     ret = gnutls_session_set_data(session, data.data, data.size);
+    if (ret == GNUTLS_E_EXPIRED)
+        session->internals.db_remove_func(session->internals.db_ptr, key);
+
     gnutls_free(data.data);

     if (ret < 0) {


- Last question, expire_time is set on session itself so why it is not
packed with the session data in order to check it when session is
unpacked ? and to provide a function like gnutls_db_check_entry_time()
which will return remaining time or GNUTLS_E_EXPIRED.

Thanks,
Joe