From 3d55134215e24f107b050cba92fa24e018b2ca52 Mon Sep 17 00:00:00 2001
From: Nikos Mavrogiannopoulos <nmav@gnutls.org>
Date: Thu, 12 Feb 2015 21:44:29 +0100
Subject: [PATCH] when opening a session forward the received cookie to
 sec-module

That allows to verify that the cookie hasn't been tampered
without relying only on the MAC.
---
 src/cookies.c      | 17 ++++++++++++-----
 src/ipc.proto      |  2 ++
 src/main-auth.c    |  2 +-
 src/main-misc.c    | 18 ++++++++++++------
 src/main.h         |  2 +-
 src/sec-mod-auth.c |  6 ++++++
 6 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/src/cookies.c b/src/cookies.c
index 55bcf0de..cef622af 100644
--- a/src/cookies.c
+++ b/src/cookies.c
@@ -46,7 +46,7 @@ gnutls_datum_t iv = { (void*)cookie, COOKIE_IV_SIZE };
 int ret;
 uint8_t tag[COOKIE_MAC_SIZE];
 gnutls_cipher_hd_t h;
-uint8_t *p;
+uint8_t *p, *decrypted = NULL;
 unsigned p_size;
 
 	if (cookie_size <= COOKIE_IV_SIZE+COOKIE_MAC_SIZE)
@@ -56,16 +56,22 @@ unsigned p_size;
 	if (ret < 0)
 		return -1;
 
+	decrypted = talloc_size(pa->allocator_data, cookie_size);
+	if (decrypted == NULL) {
+		ret = -1;
+		goto cleanup;
+	}
+
 	cookie += COOKIE_IV_SIZE;
 	cookie_size -= (COOKIE_IV_SIZE + COOKIE_MAC_SIZE);
 
-	ret = gnutls_cipher_decrypt2(h, cookie, cookie_size, cookie, cookie_size);
+	ret = gnutls_cipher_decrypt2(h, cookie, cookie_size, decrypted, cookie_size);
 	if (ret < 0) {
 		ret = -1;
 		goto cleanup;
 	}
 
-	p = cookie;
+	p = decrypted;
 	p_size = cookie_size;
 
 	ret = gnutls_cipher_tag(h, tag, sizeof(tag));
@@ -73,8 +79,8 @@ unsigned p_size;
 		ret = -1;
 		goto cleanup;
 	}
-	cookie += cookie_size;
-	if (memcmp(tag, cookie, COOKIE_MAC_SIZE) != 0) {
+
+	if (memcmp(tag, cookie+cookie_size, COOKIE_MAC_SIZE) != 0) {
 		ret = -1;
 		goto cleanup;
 	}
@@ -90,6 +96,7 @@ unsigned p_size;
 
 cleanup:
 	gnutls_cipher_deinit(h);
+	talloc_free(decrypted);
 
 	return ret;
 }
diff --git a/src/ipc.proto b/src/ipc.proto
index d3dedba5..3c7e1288 100644
--- a/src/ipc.proto
+++ b/src/ipc.proto
@@ -219,6 +219,8 @@ message cookie
 message sec_auth_session_msg
 {
 	required bytes sid = 1;
+	/* on open */
+	optional bytes cookie = 2;
 	/* on close */
 	optional uint32 uptime = 3;
 	optional uint64 bytes_in = 4;
diff --git a/src/main-auth.c b/src/main-auth.c
index b9d34424..564766c2 100644
--- a/src/main-auth.c
+++ b/src/main-auth.c
@@ -214,7 +214,7 @@ struct proc_st *old_proc;
 	apply_default_sup_config(s->config, proc);
 
 	/* loads sup config */
-	ret = session_open(s, proc);
+	ret = session_open(s, proc, req->cookie.data, req->cookie.len);
 	if (ret < 0) {
 		mslog(s, proc, LOG_INFO, "could not open session");
 		return -1;
diff --git a/src/main-misc.c b/src/main-misc.c
index 5e97ed7d..aeca4ca8 100644
--- a/src/main-misc.c
+++ b/src/main-misc.c
@@ -163,7 +163,7 @@ struct proc_st *ctmp;
 }
 
 static
-int session_cmd(main_server_st * s, struct proc_st *proc, unsigned open)
+int session_cmd(main_server_st * s, struct proc_st *proc, const uint8_t *cookie, unsigned cookie_size)
 {
 	int sd, ret, e;
 	SecAuthSessionMsg ireq = SEC_AUTH_SESSION_MSG__INIT;
@@ -171,7 +171,7 @@ int session_cmd(main_server_st * s, struct proc_st *proc, unsigned open)
 	unsigned type, i;
 	PROTOBUF_ALLOCATOR(pa, proc);
 
-	if (open)
+	if (cookie)
 		type = SM_CMD_AUTH_SESSION_OPEN;
 	else
 		type = SM_CMD_AUTH_SESSION_CLOSE;
@@ -205,6 +205,12 @@ int session_cmd(main_server_st * s, struct proc_st *proc, unsigned open)
 	ireq.sid.data = proc->sid;
 	ireq.sid.len = sizeof(proc->sid);
 
+	if (cookie) {
+		ireq.cookie.data = (void*)cookie;
+		ireq.cookie.len = cookie_size;
+		ireq.has_cookie = 1;
+	}
+
 	mslog(s, proc, LOG_DEBUG, "sending msg %s to sec-mod", cmd_request_to_str(type));
 
 	ret = send_msg(proc, sd, type,
@@ -218,7 +224,7 @@ int session_cmd(main_server_st * s, struct proc_st *proc, unsigned open)
 		return -1;
 	}
 
-	if (open) {
+	if (type == SM_CMD_AUTH_SESSION_OPEN) {
 		ret = recv_msg(proc, sd, SM_CMD_AUTH_SESSION_REPLY,
 		       (void *)&msg, (unpack_func) sec_auth_session_reply_msg__unpack);
 		close(sd);
@@ -326,14 +332,14 @@ int session_cmd(main_server_st * s, struct proc_st *proc, unsigned open)
 	return 0;
 }
 
-int session_open(main_server_st * s, struct proc_st *proc)
+int session_open(main_server_st * s, struct proc_st *proc, const uint8_t *cookie, unsigned cookie_size)
 {
-	return session_cmd(s, proc, 1);
+	return session_cmd(s, proc, cookie, cookie_size);
 }
 
 int session_close(main_server_st * s, struct proc_st *proc)
 {
-	return session_cmd(s, proc, 0);
+	return session_cmd(s, proc, NULL, 0);
 }
 
 /* k: whether to kill the process
diff --git a/src/main.h b/src/main.h
index a9babc5d..bae48367 100644
--- a/src/main.h
+++ b/src/main.h
@@ -231,7 +231,7 @@ int handle_resume_fetch_req(main_server_st* s, struct proc_st * proc,
 int handle_resume_store_req(main_server_st* s, struct proc_st *proc,
   			   const SessionResumeStoreReqMsg *);
 
-int session_open(main_server_st * s, struct proc_st *proc);
+int session_open(main_server_st * s, struct proc_st *proc, const uint8_t *cookie, unsigned cookie_size);
 int session_close(main_server_st * s, struct proc_st *proc);
 
 void 
diff --git a/src/sec-mod-auth.c b/src/sec-mod-auth.c
index d58112b6..566eec4d 100644
--- a/src/sec-mod-auth.c
+++ b/src/sec-mod-auth.c
@@ -334,6 +334,12 @@ int handle_sec_auth_session_cmd(int cfd, sec_mod_st * sec, const SecAuthSessionM
 			return -1;
 		}
 
+		if (req->has_cookie == 0 || (req->cookie.len != e->cookie_size) ||
+		    memcmp(req->cookie.data, e->cookie, e->cookie_size) != 0) {
+			seclog(sec, LOG_ERR, "cookie doesn't match the one sent");
+			return -1;
+		}
+
 		if (module != NULL && module->open_session != NULL) {
 			ret = module->open_session(e->auth_ctx, req->sid.data, req->sid.len);
 			if (ret < 0) {