Store a hash of the client's cookie instead of the cookie itself.

That ensures that the cookies cannot be leaked from the server.
On a hash collision, the IP of the other cookie in use will be
hijacked.

src/cookies.c

@@ -180,7 +180,7 @@ struct htable_iter iter;
 			e->proc->cookie_ptr = NULL;
 
 		e->proc = NULL;
-		safe_memset(e->cookie, 0, e->cookie_size);
+		safe_memset(e->cookie_hash, 0, sizeof(e->cookie_hash));
 		talloc_free(e);
 
 		e = htable_next(db->db, &iter);
@@ -210,7 +210,7 @@ time_t now = time(0);
 		htable_delval(db->db, &iter);
 		db->total--;
 
-		safe_memset(e->cookie, 0, e->cookie_size);
+		safe_memset(e->cookie_hash, 0, sizeof(e->cookie_hash));
 		talloc_free(e);
  cont:
 		e = htable_next(db->db, &iter);
@@ -223,7 +223,7 @@ static size_t rehash(const void* _e, void* unused)
 {
 const struct cookie_entry_st * e = _e;
 
-	return hash_any(e->cookie, e->cookie_size, 0);
+	return hash_any(e->cookie_hash, sizeof(e->cookie_hash), 0);
 }
 
 void cookie_db_init(void *pool, struct cookie_entry_db_st* db)
@@ -238,8 +238,7 @@ static bool cookie_entry_cmp(const void* _c1, void* _c2)
 const struct cookie_entry_st* c1 = _c1;
 struct cookie_entry_st* c2 = _c2;
 
-	if (c1->cookie_size == c2->cookie_size &&
-		memcmp(c1->cookie, c2->cookie, c2->cookie_size) == 0)
+	if (memcmp(c1->cookie_hash, c2->cookie_hash, sizeof(c1->cookie_hash)) == 0)
 		return 1;
 
 	return 0;
@@ -249,9 +248,12 @@ struct cookie_entry_st *find_cookie_entry(struct cookie_entry_db_st* db, void *c
 {
 	struct cookie_entry_st *e;
 	struct cookie_entry_st t;
+	int ret;
 
-	t.cookie = cookie;
-	t.cookie_size = cookie_size;
+	ret = gnutls_hash_fast(COOKIE_HASH, cookie, cookie_size, t.cookie_hash);
+	if (ret < 0) {
+		return NULL;
+	}
 
 	e = htable_get(db->db, hash_any(cookie, cookie_size, 0), cookie_entry_cmp, &t);
 	if (e == NULL)
@@ -271,18 +273,19 @@ void revive_cookie(struct cookie_entry_st * e)
 struct cookie_entry_st *new_cookie_entry(struct cookie_entry_db_st* db, proc_st *proc, void *cookie, unsigned cookie_size)
 {
 	struct cookie_entry_st *t;
+	int ret;
 
 	t = talloc(db->db, struct cookie_entry_st);
 	if (t == NULL)
 		return NULL;
 
 	t->expiration = -1;
-	t->cookie = talloc_memdup(t, cookie, cookie_size);
-	t->cookie_size = cookie_size;
 
-	if (t->cookie == NULL) {
+	ret = gnutls_hash_fast(COOKIE_HASH, cookie, cookie_size, t->cookie_hash);
+	if (ret < 0) {
 		goto fail;
 	}
+
 	t->proc = proc;
 
 	if (htable_add(db->db, rehash(t, NULL), t) == 0) {

src/main-auth.c

@@ -189,7 +189,8 @@ struct cookie_entry_st *old;
 	memcpy(proc->dtls_session_id, cmsg->session_id.data, cmsg->session_id.len);
 	proc->dtls_session_id_size = cmsg->session_id.len;
 
-	/* cookie is good so far, now read config (in order to know whether roaming is allowed or not */
+	/* cookie is good so far, now read config (in order to know
+	 * whether roaming is allowed or not */
 	memset(&proc->config, 0, sizeof(proc->config));
 	apply_default_sup_config(s->config, proc);
 
@@ -226,7 +227,8 @@ struct cookie_entry_st *old;
 	/* check for a valid stored cookie */
 	if ((old=find_cookie_entry(&s->cookies, req->cookie.data, req->cookie.len)) != NULL) {
 		if (old->proc != NULL) {
-			mslog(s, old->proc, LOG_DEBUG, "disconnecting '%s' due to new cookie connection", old->proc->username);
+			mslog(s, old->proc, LOG_DEBUG, "disconnecting '%s' (%u) due to new cookie connection",
+				old->proc->username, (unsigned)old->proc->pid);
 
 			/* steal its leases */
 			steal_ip_leases(old->proc, proc);
@@ -234,7 +236,8 @@ struct cookie_entry_st *old;
 			/* steal its cookie */
 			old->proc->cookie_ptr = NULL;
 
-			kill(old->proc->pid, SIGTERM);
+			if (old->proc->pid > 0)
+				kill(old->proc->pid, SIGTERM);
 		} else {
 			revive_cookie(old);
 		}

src/main.h

@@ -72,12 +72,17 @@ enum {
 	PS_AUTH_COMPLETED, /* successful authentication */
 };
 
+#define COOKIE_HASH_SIZE 20
+#define COOKIE_HASH GNUTLS_DIG_SHA1
+
 typedef struct cookie_entry_st {
 	struct proc_st *proc; /* may be null, otherwise the proc that uses that cookie */
 	time_t expiration; /* -1 or the time it should expire */
 
-	uint8_t *cookie; /* the cookie associated with the session */
-	unsigned cookie_size;
+	/* We store the hash of the cookie that is associated with a particular session.
+	 * The reason is to avoid a memory leak to an unprivileged process to expose
+	 * data that can be used to authenticate as another use */
+	uint8_t cookie_hash[COOKIE_HASH_SIZE];
 } cookie_st;
 
 /* Each worker process maps to a unique proc_st structure.