From ddbdabe37fc66f6881a22e20db8aebbd46385f84 Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Tue, 8 Mar 2016 17:16:29 -0500 Subject: [PATCH 3/3] GSSAPI authentication cleanup Become more fussy about what flags we need. Now that we want to do encryption, protection and integrity are needed for that to work. Other protections are desirable as well, and we should check the flags GSSAPI returns to us. Also remove the (now-)redundant definitions that worked around an old bug with MIT Kerberos on Windows. Finally, resolve a pre-existing action item for acquiring all possible GSSAPI error messages on the backend, rather than just the first major and first minor statuses. --- src/backend/libpq/auth.c | 20 ++++++++++++- src/backend/libpq/be-gssapi-common.c | 50 ++++++++++++++++----------------- src/interfaces/libpq/fe-auth.c | 19 +++++++++++-- src/interfaces/libpq/fe-gssapi-common.c | 11 -------- 4 files changed, 59 insertions(+), 41 deletions(-) diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 94d95bd..83f7fd1 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -719,7 +719,8 @@ pg_GSS_recvauth(Port *port) OM_uint32 maj_stat, min_stat, lmin_s, - gflags; + gflags, + target_flags; int mtype; int ret; StringInfoData buf; @@ -875,6 +876,23 @@ pg_GSS_recvauth(Port *port) } /* + * GSS_C_REPLAY_FLAG (request replay detection) and GSS_C_SEQUENCE_FLAG + * (out-of-sequence detection) are missing for compatability with older + * clients which do not request these flags. They should be added in as + * soon as we no longer care about pre-9.6 clients. + * + * Newer clients will request both GSS_C_REPLAY_FLAGS and + * GSS_C_SEQUENCE_FLAGS as well as the flags we check for below, so lack + * of these two flags is not the end of the world. + */ + target_flags = GSS_C_MUTUAL_FLAG | GSS_C_CONF_FLAG | GSS_C_INTEG_FLAG; + if ((gflags & target_flags) != target_flags) + { + ereport(FATAL, (errmsg("GSSAPI did not provide required flags"))); + return STATUS_ERROR; + } + + /* * GSS_S_COMPLETE indicates that authentication is now complete. * * Get the name of the user that authenticated, and compare it to the pg diff --git a/src/backend/libpq/be-gssapi-common.c b/src/backend/libpq/be-gssapi-common.c index eab68a5..843d449 100644 --- a/src/backend/libpq/be-gssapi-common.c +++ b/src/backend/libpq/be-gssapi-common.c @@ -17,17 +17,6 @@ #include "postgres.h" -#if defined(WIN32) && !defined(WIN32_ONLY_COMPILER) -/* - * MIT Kerberos GSSAPI DLL doesn't properly export the symbols for MingW - * that contain the OIDs required. Redefine here, values copied - * from src/athena/auth/krb5/src/lib/gssapi/generic/gssapi_generic.c - */ -static const gss_OID_desc GSS_C_NT_USER_NAME_desc = -{10, (void *) "\x2a\x86\x48\x86\xf7\x12\x01\x02\x01\x02"}; -static GSS_DLLIMP gss_OID GSS_C_NT_USER_NAME = &GSS_C_NT_USER_NAME_desc; -#endif - void pg_GSS_error(int severity, char *errmsg, OM_uint32 maj_stat, OM_uint32 min_stat) { @@ -36,31 +25,40 @@ pg_GSS_error(int severity, char *errmsg, OM_uint32 maj_stat, OM_uint32 min_stat) msg_ctx; char msg_major[128], msg_minor[128]; + short i; + + gmsg.value = NULL; + gmsg.length = 0; /* Fetch major status message */ msg_ctx = 0; - gss_display_status(&lmin_s, maj_stat, GSS_C_GSS_CODE, - GSS_C_NO_OID, &msg_ctx, &gmsg); - strlcpy(msg_major, gmsg.value, sizeof(msg_major)); - gss_release_buffer(&lmin_s, &gmsg); + i = 0; + do + { + gss_display_status(&lmin_s, maj_stat, GSS_C_GSS_CODE, + GSS_C_NO_OID, &msg_ctx, &gmsg); + strlcpy(msg_major + i, gmsg.value, sizeof(msg_major) - i); + gss_release_buffer(&lmin_s, &gmsg); + } + while (msg_ctx && i < sizeof(msg_major)); - if (msg_ctx) - - /* - * More than one message available. XXX: Should we loop and read all - * messages? (same below) - */ + if (msg_ctx || i == sizeof(msg_major)) ereport(WARNING, (errmsg_internal("incomplete GSS error report"))); /* Fetch mechanism minor status message */ msg_ctx = 0; - gss_display_status(&lmin_s, min_stat, GSS_C_MECH_CODE, - GSS_C_NO_OID, &msg_ctx, &gmsg); - strlcpy(msg_minor, gmsg.value, sizeof(msg_minor)); - gss_release_buffer(&lmin_s, &gmsg); + i = 0; + do + { + gss_display_status(&lmin_s, min_stat, GSS_C_MECH_CODE, + GSS_C_NO_OID, &msg_ctx, &gmsg); + strlcpy(msg_minor + i, gmsg.value, sizeof(msg_minor) - i); + gss_release_buffer(&lmin_s, &gmsg); + } + while (msg_ctx && i < sizeof(msg_minor)); - if (msg_ctx) + if (msg_ctx || i == sizeof(msg_minor)) ereport(WARNING, (errmsg_internal("incomplete GSS minor error report"))); diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c index 56528f2..76bc3ec 100644 --- a/src/interfaces/libpq/fe-auth.c +++ b/src/interfaces/libpq/fe-auth.c @@ -57,20 +57,24 @@ pg_GSS_continue(PGconn *conn) { OM_uint32 maj_stat, min_stat, - lmin_s; + lmin_s, + req_flags, + ret_flags; + req_flags = GSS_C_MUTUAL_FLAG | GSS_C_REPLAY_FLAG | GSS_C_SEQUENCE_FLAG | + GSS_C_CONF_FLAG | GSS_C_INTEG_FLAG; maj_stat = gss_init_sec_context(&min_stat, GSS_C_NO_CREDENTIAL, &conn->gctx, conn->gtarg_nam, GSS_C_NO_OID, - GSS_C_MUTUAL_FLAG, + req_flags, 0, GSS_C_NO_CHANNEL_BINDINGS, (conn->gctx == GSS_C_NO_CONTEXT) ? GSS_C_NO_BUFFER : &conn->ginbuf, NULL, &conn->goutbuf, - NULL, + &ret_flags, NULL); if (conn->gctx != GSS_C_NO_CONTEXT) @@ -109,8 +113,17 @@ pg_GSS_continue(PGconn *conn) } if (maj_stat == GSS_S_COMPLETE) + { gss_release_name(&lmin_s, &conn->gtarg_nam); + if ((ret_flags & req_flags) != req_flags) + { + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("GSSAPI did not provide required flags\n")); + return STATUS_ERROR; + } + } + return STATUS_OK; } diff --git a/src/interfaces/libpq/fe-gssapi-common.c b/src/interfaces/libpq/fe-gssapi-common.c index cd7ae5a..0ad09f7 100644 --- a/src/interfaces/libpq/fe-gssapi-common.c +++ b/src/interfaces/libpq/fe-gssapi-common.c @@ -15,17 +15,6 @@ #include "fe-auth.h" #include "fe-gssapi-common.h" -#if defined(WIN32) && !defined(WIN32_ONLY_COMPILER) -/* - * MIT Kerberos GSSAPI DLL doesn't properly export the symbols for MingW - * that contain the OIDs required. Redefine here, values copied - * from src/athena/auth/krb5/src/lib/gssapi/generic/gssapi_generic.c - */ -static const gss_OID_desc GSS_C_NT_HOSTBASED_SERVICE_desc = -{10, (void *) "\x2a\x86\x48\x86\xf7\x12\x01\x02\x01\x04"}; -static GSS_DLLIMP gss_OID GSS_C_NT_HOSTBASED_SERVICE = &GSS_C_NT_HOSTBASED_SERVICE_desc; -#endif - /* * Fetch all errors of a specific type and append to "str". */ -- 2.8.0.rc3