[I sent this a while ago, but it seems not to have made it to the list.
I'm resending it having subscribed first; I apologise if anyone get it
twice.]
I have been trying to get a Postfix mail server using Dovecot SASL to
accept GSSAPI AUTH from another Postfix server using Cyrus SASL, and I
believe I have found a couple of bugs in Dovecot's GSSAPI
implementation.
The first problem is that, because of the way the client invokes
libsasl, it sends a GSSAPI request which does not ask for mutual
authentication. This means that on the server gss_accept_sec_context
returns GSS_S_COMPLETE with a zero-length output token. Dovecot
currently sends this to the client as a zero-length continuation
response, but this is incorrect according to RFC 4752: what it ought to
do instead is proceed straight to the security layer negotiations, and
send a gss_wrap packet.
The second is that Cyrus sends an empty authz identity; that is, the
security layer negotiation packet, when gss_unwrapped, is exactly 4
bytes long. Dovecot objects to this, but in RFC 4422 this is explicitly
allowed, and means the authz identity is identical to the authn
identity.
I believe the attached patches (for the 1.2 and 2.1 branches) fix the
problem, though I'm not entirely sure if the difference between the
p_strndup in mech_gssapi_unwrap and the t_strndup in get_display_name
is important.
Ben
--- src/auth/mech-gssapi.c~ 2010-01-24 23:14:17.000000000 +0000
+++ src/auth/mech-gssapi.c 2012-06-21 21:03:43.515476193 +0100
@@ -213,6 +213,21 @@
return name;
}
+static gss_name_t
+duplicate_name(struct auth_request *request, gss_name_t old)
+{
+ OM_uint32 major_status, minor_status;
+ gss_name_t new;
+
+ major_status = gss_duplicate_name(&minor_status, old, &new);
+ if (GSS_ERROR(major_status)) {
+ mech_gssapi_log_error(request, major_status, GSS_C_GSS_CODE,
+ "gss_duplicate_name");
+ return GSS_C_NO_NAME;
+ }
+ return new;
+}
+
static bool data_has_nuls(const void *data, unsigned int len)
{
const unsigned char *c = data;
@@ -260,6 +275,9 @@
}
static int
+mech_gssapi_wrap(struct gssapi_auth_request *request, gss_buffer_desc inbuf);
+
+static int
mech_gssapi_sec_context(struct gssapi_auth_request *request,
gss_buffer_desc inbuf)
{
@@ -327,9 +345,15 @@
}
if (ret == 0) {
- auth_request->callback(auth_request,
- AUTH_CLIENT_RESULT_CONTINUE,
- output_token.value, output_token.length);
+ if (output_token.length) {
+ auth_request->callback(auth_request,
+ AUTH_CLIENT_RESULT_CONTINUE,
+ output_token.value, output_token.length);
+ } else {
+ /* If there is no output token, go straight to wrap, which is
+ expecting an empty input token. */
+ ret = mech_gssapi_wrap(request, output_token);
+ }
}
(void)gss_release_buffer(&minor_status, &output_token);
return ret;
@@ -510,22 +534,35 @@
/* outbuf[0] contains bitmask for selected security layer,
outbuf[1..3] contains maximum output_message size */
- if (outbuf.length <= 4) {
+ if (outbuf.length < 4) {
auth_request_log_error(auth_request, "gssapi",
"Invalid response length");
return -1;
}
- name = (unsigned char *)outbuf.value + 4;
- name_len = outbuf.length - 4;
- if (data_has_nuls(name, name_len)) {
- auth_request_log_info(auth_request, "gssapi",
- "authz_name has NULs");
- return -1;
+ if (outbuf.length > 4) {
+ name = (unsigned char *)outbuf.value + 4;
+ name_len = outbuf.length - 4;
+
+ if (data_has_nuls(name, name_len)) {
+ auth_request_log_info(auth_request, "gssapi",
+ "authz_name has NULs");
+ return -1;
+ }
+
+ login_user = p_strndup(auth_request->pool, name, name_len);
+ request->authz_name = import_name(auth_request, name, name_len);
+ } else {
+ int ret;
+
+ request->authz_name = duplicate_name(auth_request,
+ request->authn_name);
+ ret = get_display_name(auth_request, request->authz_name,
+ NULL, &login_user);
+ if (ret < 0)
+ return -1;
}
- login_user = p_strndup(auth_request->pool, name, name_len);
- request->authz_name = import_name(auth_request, name, name_len);
if (request->authz_name == GSS_C_NO_NAME) {
auth_request_log_info(auth_request, "gssapi", "no authz_name");
return -1;
--- src/auth/mech-gssapi.c~ 2011-12-13 11:35:28.000000000 +0000
+++ src/auth/mech-gssapi.c 2012-06-21 21:23:42.786661107 +0100
@@ -214,6 +214,21 @@
return name;
}
+static gss_name_t
+duplicate_name(struct auth_request *request, gss_name_t old)
+{
+ OM_uint32 major_status, minor_status;
+ gss_name_t new;
+
+ major_status = gss_duplicate_name(&minor_status, old, &new);
+ if (GSS_ERROR(major_status)) {
+ mech_gssapi_log_error(request, major_status, GSS_C_GSS_CODE,
+ "gss_duplicate_name");
+ return GSS_C_NO_NAME;
+ }
+ return new;
+}
+
static bool data_has_nuls(const void *data, unsigned int len)
{
const unsigned char *c = data;
@@ -261,6 +276,9 @@
}
static int
+mech_gssapi_wrap(struct gssapi_auth_request *request, gss_buffer_desc inbuf);
+
+static int
mech_gssapi_sec_context(struct gssapi_auth_request *request,
gss_buffer_desc inbuf)
{
@@ -328,9 +346,15 @@
}
if (ret == 0) {
- auth_request_handler_reply_continue(auth_request,
- output_token.value,
- output_token.length);
+ if (output_token.length) {
+ auth_request_handler_reply_continue(auth_request,
+ output_token.value,
+ output_token.length);
+ } else {
+ /* If there is no output token, go straight to wrap, which is
+ expecting an empty input token. */
+ ret = mech_gssapi_wrap(request, output_token);
+ }
}
(void)gss_release_buffer(&minor_status, &output_token);
return ret;
@@ -510,22 +534,35 @@
/* outbuf[0] contains bitmask for selected security layer,
outbuf[1..3] contains maximum output_message size */
- if (outbuf.length <= 4) {
+ if (outbuf.length < 4) {
auth_request_log_error(auth_request, "gssapi",
"Invalid response length");
return -1;
}
- name = (unsigned char *)outbuf.value + 4;
- name_len = outbuf.length - 4;
- if (data_has_nuls(name, name_len)) {
- auth_request_log_info(auth_request, "gssapi",
- "authz_name has NULs");
- return -1;
+ if (outbuf.length > 4) {
+ name = (unsigned char *)outbuf.value + 4;
+ name_len = outbuf.length - 4;
+
+ if (data_has_nuls(name, name_len)) {
+ auth_request_log_info(auth_request, "gssapi",
+ "authz_name has NULs");
+ return -1;
+ }
+
+ login_user = p_strndup(auth_request->pool, name, name_len);
+ request->authz_name = import_name(auth_request, name, name_len);
+ } else {
+ int ret;
+
+ request->authz_name = duplicate_name(auth_request,
+ request->authn_name);
+ ret = get_display_name(auth_request, request->authz_name,
+ NULL, &login_user);
+ if (ret < 0)
+ return -1;
}
- login_user = p_strndup(auth_request->pool, name, name_len);
- request->authz_name = import_name(auth_request, name, name_len);
if (request->authz_name == GSS_C_NO_NAME) {
auth_request_log_info(auth_request, "gssapi", "no authz_name");
return -1;
