[Dovecot] [PATCH] Support for GSSAPI SASL Mechanism

[pod at herald.ox.ac.uk]

I believe there is a significant privacy issue with the recently posted
updated GSSAPI patch when used on multiuser systems (indeed this problem
is present in the previous patch as well).

The username, and consequently the uid that the imap process is eventually
run as, is taken from the authorization id in the (unwrapped) buffer
supplied by the client as part of the SASL security layer negotiation.

The short of it is that with the patch as it stands user1 can authenticate
as user1 but supply user2 as the authorization id and dovecot will fire up
an imap process running as user2 looking at user2's mail.

I'm attaching a patch that I believe fixes the immediate problem but it is
really only a bandaid over more general SASL issues.

The patch does the following:

 - Adds in a QOP negotiation flags enum (somewhat cosmetic really)
 - Renames src_name in gssapi_auth_request to authn_name and adds
   authz_name
 - Adds a helper function to import a gss_name_t from a buffer
 - Imports the authorization id (authz_name) and compares it with the
   authentication id (authn_name).  If they are different authentication
   is declined.

I'm not entirely happy that the username passed back from the auth process
is still copied from the authorization id buffer because it feels like the
username should come from gss_display_name.  Unfortunately that will tend
to return names like user at REALM which opens up another can of worms and is
too much for my little brain to cope with right now.
-------------- next part --------------
diff -ru dovecot-jv/src/auth/mech-gssapi.c dovecot-jv-pod/src/auth/mech-gssapi.c
--- dovecot-jv/src/auth/mech-gssapi.c	2005-10-19 18:58:34.000000000 +0100
+++ dovecot-jv-pod/src/auth/mech-gssapi.c	2005-10-20 16:03:30.000000000 +0100
@@ -28,6 +28,14 @@
 
 #include <gssapi/gssapi.h>
 
+/* Non-zero flags defined in RFC 2222 */
+enum sasl_gssapi_qop {
+	SASL_GSSAPI_QOP_UNSPECIFIED = 0x00,
+	SASL_GSSAPI_QOP_AUTH_ONLY   = 0x01,
+	SASL_GSSAPI_QOP_AUTH_INT    = 0x02,
+	SASL_GSSAPI_QOP_AUTH_CONF   = 0x04
+};
+
 struct gssapi_auth_request {
 	struct auth_request auth_request;
 	gss_ctx_id_t gss_ctx;
@@ -39,7 +47,8 @@
 		GSS_STATE_UNWRAP
 	} sasl_gssapi_state;
 
-	gss_name_t src_name;
+	gss_name_t authn_name;
+	gss_name_t authz_name;
 		
 	pool_t pool;
 };
@@ -122,6 +131,28 @@
 	return major_status;
 }
 
+static gss_name_t
+import_name(struct auth_request *request, void *str, size_t len)
+{
+	OM_uint32 major_status, minor_status;
+	gss_buffer_desc name_buf;
+	gss_name_t name;
+
+	name_buf.value=str;
+	name_buf.length=len;
+	major_status = gss_import_name(&minor_status,
+				       &name_buf,
+				       GSS_C_NO_OID,
+				       &name);
+	if (GSS_ERROR(major_status)) {
+		auth_request_log_gss_error(request, major_status,
+					   GSS_C_GSS_CODE, "gss_import_name");
+		return GSS_C_NO_NAME;
+	}
+
+	return name;
+}
+
 static void gssapi_sec_context(struct gssapi_auth_request *request,
 				  gss_buffer_desc inbuf)
 {
@@ -134,7 +165,7 @@
 		request->service_cred,
 		&inbuf,
 		GSS_C_NO_CHANNEL_BINDINGS,
-		&request->src_name, 
+		&request->authn_name, 
 		NULL, /* mech_type */
 		&outbuf,
 		NULL, /* ret_flags */
@@ -177,7 +208,9 @@
 
 	/* The clients return data should be empty here */
 	
-	ret[0] = 0x01; /* Only authentication, no integrity or confidentiality protection (yet?) */
+        /* Only authentication, no integrity or confidentiality protection (yet?) */
+	ret[0] = (SASL_GSSAPI_QOP_UNSPECIFIED|
+                  SASL_GSSAPI_QOP_AUTH_ONLY);
 	ret[1] = 0xFF;
 	ret[2] = 0xFF;
 	ret[3] = 0xFF;
@@ -215,6 +248,7 @@
 {
 	OM_uint32 major_status, minor_status;
 	gss_buffer_desc outbuf;
+	int equal_authn_authz=0;
 
 	major_status = gss_unwrap(&minor_status, request->gss_ctx, 
 							  &inbuf, &outbuf, NULL, NULL);
@@ -233,6 +267,27 @@
 		return;
 	}
 
+	request->authz_name = import_name(&request->auth_request,
+					  (unsigned char *)outbuf.value+4,
+					  outbuf.length-4);
+	if ((request->authn_name == GSS_C_NO_NAME) ||
+	    (request->authz_name == GSS_C_NO_NAME)) {
+		/* XXX (pod): is this check necessary? */
+		auth_request_log_error(&request->auth_request, "gssapi", "one of authn_name or authz_name not determined");
+		auth_request_fail(&request->auth_request);
+		return;
+	}
+	major_status = gss_compare_name(&minor_status,
+					request->authn_name,
+					request->authz_name,
+					&equal_authn_authz);
+	if (equal_authn_authz == 0) {
+		auth_request_log_error(&request->auth_request, "gssapi",
+				       "authn_name and authz_name differ: not supported");
+		auth_request_fail(&request->auth_request);
+		return;
+	}
+
 	request->auth_request.user = p_strndup(request->auth_request.pool,
 			((unsigned char*) outbuf.value)+4,
 			outbuf.length-4);
@@ -272,7 +327,8 @@
 		auth_request_internal_failure(request);
 		return;
 	}
-	gssapi_request->src_name = GSS_C_NO_NAME;
+	gssapi_request->authn_name = GSS_C_NO_NAME;
+	gssapi_request->authz_name = GSS_C_NO_NAME;
 
 	gssapi_request->sasl_gssapi_state = GSS_STATE_SEC_CONTEXT;
 
@@ -299,6 +355,8 @@
 										  GSS_C_NO_BUFFER);
 
 	major_status = gss_release_cred(&minor_status, &gssapi_request->service_cred);
+	major_status = gss_release_name(&minor_status, &gssapi_request->authn_name);
+	major_status = gss_release_name(&minor_status, &gssapi_request->authz_name);
 
 	pool_unref(request->pool);
 }