[BUGS] BUG #13854: SSPI authentication failure: wrong realm name used

[ch...@chrullrich.net]

The following bug has been logged on the website:

Bug reference: 13854
Logged by: Christian Ullrich
Email address: ch...@chrullrich.net
PostgreSQL version: 9.5.0
Operating system: Windows
Description:

According to the release notes, the default for the "include_realm" option
in SSPI authentication was changed from off to on in 9.5 for improved
security. However, the authenticated user name, with the option enabled, now
includes the NetBIOS domain name, *not* the Kerberos realm name:

[chul@itdb 2016-01-08 00:31:56 CET] ([unknown]) LOG: provided user name
(chul) and authenticated user name (chul@LOCAL-DOM) do not match
[chul@itdb 2016-01-08 00:31:56 CET] ([unknown]) FATAL: SSPI authentication
failed for user "chul"
[chul@itdb 2016-01-08 00:31:56 CET] ([unknown]) DETAIL: Connection matched
pg_hba.conf line 101: "host all all
192.168.0.1/32 sspi"

"LOCAL-DOM" is the domain short name/NetBIOS name. The realm name is
(typically, and in this case) the domain DNS name in uppercase. The string
used for the realm name is retrieved from the LookupAccountSid() function,
which will always return the short name:

(Python 3.5 on a Windows 10 client in the same domain):
>>> from win32security import LookupAccountName, LookupAccountSid
>>> sid = LookupAccountName(None, "chul")[0]
>>> LookupAccountSid(None, sid)
('chul', 'LOCAL-DOM', 1)

Login is successful if I add "include_realm=0 krb_realm=LOCAL-DOM" to
pg_hba.conf. If I use the actual Kerberos realm name instead, I simply get
"SSPI authentication failed" with no further information in the log.

I am aware of the option of using pg_ident.conf to map authenticated user
names with realm to bare database role names, but I would have to put the
wrong realm name string in there as well, so it is not a fix.

A possible fix might be to convert the user name/domain name retrieved from
LookupAccountSid() using TranslateName()/IADsNameTranslate to get the
Kerberos UPN, which includes the actual realm name. There may be
compatibility issues with that, because the first part of the UPN need not
equal sAMAccountName (the logon user name). Apparently [1] you can also get
an explicit mapping (look up dnsRoot by nETBIOSName) from AD, but whether
that is the correct approach, I don't know.

My distribution is from
<http://get.enterprisedb.com/postgresql/postgresql-9.5.0-1-windows-x64-binaries.zip>.

[1] https://stackoverflow.com/questions/12606466

--
Christian


--
Sent via pgsql-bugs mailing list (pgsql...@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

[Tom Lane]

ch...@chrullrich.net writes:
> According to the release notes, the default for the "include_realm" option
> in SSPI authentication was changed from off to on in 9.5 for improved
> security. However, the authenticated user name, with the option enabled, now
> includes the NetBIOS domain name, *not* the Kerberos realm name:

Is this new breakage, or did include_realm=1 fail in the same way for
your configuration in prior releases?

regards, tom lane

[Christian Ullrich]

* From: Tom Lane [mailto:t...@sss.pgh.pa.us]

> ch...@chrullrich.net writes:

> > According to the release notes, the default for the "include_realm"
> > option in SSPI authentication was changed from off to on in 9.5 for
> > improved security. However, the authenticated user name, with the
> > option enabled, now includes the NetBIOS domain name, *not* the
> > Kerberos realm name:

> Is this new breakage, or did include_realm=1 fail in the same way for
> your configuration in prior releases?

s/now includes/includes/

I did not use that option before, the same as everyone else, but I checked
9.4.5 just now and it fails in the same way there. The code in auth.c has
not changed significantly since it was introduced, so I assume that it
has behaved like this from the start.

--
Christian

[Christian Ullrich]

* Christian Ullrich wrote:

> According to the release notes, the default for the "include_realm" option
> in SSPI authentication was changed from off to on in 9.5 for improved
> security. However, the authenticated user name, with the option enabled,

> includes the NetBIOS domain name, *not* the Kerberos realm name:

This has been true ever since the include_realm option was added in
2009 (so presumably nobody has ever used it).

Below is a patch to correct this behavior. I suspect it has some
serious compatibility issues, so I would appreciate feedback.

The patch adds:

- a pg_hba.conf option "real_realm" (defaults to off) that uses the
"real" realm name from the Kerberos UPN instead of the domain short
name. It does this by converting the SAM-compatible user name that is
the result of the SSPI authentication to its UPN equivalent, using
domain controller knowledge in the form of the TranslateName()
function. The option defaults to off for backward compatibility with
existing pg_ident.conf files.

- a pg_hba.conf option "upn_username" (defaults to off) that uses
the user name from the UPN (the part before the "@") instead of the
SAM-compatible user name from the authentication result. It only
applies if real_realm is enabled. This is a separate option to keep
compatibility with libpq, which uses the SAM-compatible user name if
none is specified, either from the USERNAME environment variable or
from the GetUserName() function, not sure which. The SAM-compatible
user name (AD attribute sAMAccountName) and the UPN user name
(userPrincipalName) can be different, and there is even UI for that
in the default AD user management tool, so I would expect that there
are environments where this is true.

To see the effect of this option, create an AD user where the two
names are different and look at the "authentication failed" message
in the server log.

- documentation for the above.

I will add the patch to the open CF shortly.


diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
new file mode 100644
index 3b2935c..7236459
*** a/doc/src/sgml/client-auth.sgml
--- b/doc/src/sgml/client-auth.sgml
*************** omicron bryanh
*** 1097,1102 ****
--- 1097,1132 ----
</varlistentry>

<varlistentry>
+ <term><literal>real_realm</literal></term>
+ <listitem>
+ <para>
+ If set to 0, the domain's SAM-compatible name (also known as the
+ NetBIOS name) is used for the <literal>include_realm</literal>
+ option. This is the default. If set to 1, the true realm name from
+ the Kerberos user principal name is used. If you used the
+ <literal>include_realm</literal> option, you can leave this option
+ disabled to maintain compatibility with existing
+ <filename>pg_ident.conf</filename> files.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>upn_username</literal></term>
+ <listitem>
+ <para>
+ If this option is enabled along with <literal>real_realm</literal>,
+ the user name from the Kerberos UPN is used for authentication. If
+ it is disabled (the default), the SAM-compatible user name is used.
+ Note that <application>libpq</> uses the SAM-compatible name if no
+ explicit user name is specified. If you use
+ <application>libpq</> (e.g. through the ODBC driver), you should
+ leave this option disabled.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>map</literal></term>
<listitem>
<para>
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
new file mode 100644
index 0131bfd..5d84c19
*** a/src/backend/libpq/auth.c
--- b/src/backend/libpq/auth.c
*************** typedef SECURITY_STATUS
*** 155,160 ****
--- 155,165 ----
(WINAPI * QUERY_SECURITY_CONTEXT_TOKEN_FN) (
PCtxtHandle, void **);
static int pg_SSPI_recvauth(Port *port);
+ static int pg_SSPI_make_upn(char *accountname,
+ size_t accountnamesize,
+ char *domainname,
+ size_t domainnamesize,
+ bool update_accountname);
#endif

/*----------------------------------------------------------------
*************** static int
*** 1026,1031 ****
--- 1031,1037 ----
pg_SSPI_recvauth(Port *port)
{
int mtype;
+ int status;
StringInfoData buf;
SECURITY_STATUS r;
CredHandle sspicred;
*************** pg_SSPI_recvauth(Port *port)
*** 1261,1266 ****
--- 1267,1281 ----

free(tokenuser);

+ if (port->hba->real_realm) {
+ status = pg_SSPI_make_upn(accountname, sizeof(accountname),
+ domainname, sizeof(domainname),
+ port->hba->upn_username) != STATUS_OK;
+ if (status != STATUS_OK) {
+ return status;
+ }
+ }
+
/*
* Compare realm/domain if requested. In SSPI, always compare case
* insensitive.
*************** pg_SSPI_recvauth(Port *port)
*** 1296,1301 ****
--- 1311,1407 ----
else
return check_usermap(port->hba->usermap, port->user_name, accountname, true);
}
+
+ static int pg_SSPI_make_upn(char *accountname,
+ size_t accountnamesize,
+ char *domainname,
+ size_t domainnamesize,
+ bool update_accountname)
+ {
+ char *samname;
+ char *upname = NULL;
+ char *p = NULL;
+ ULONG upnamesize = 0;
+ size_t upnamerealmsize;
+ BOOLEAN res;
+
+ /* Build SAM name (DOMAIN\\user), then translate to UPN
+ (us...@kerberos.realm). The realm name is returned in
+ lower case, but that is fine because in SSPI auth,
+ string comparisons are always case-insensitive. */
+
+ samname = psprintf("%s\\%s", domainname, accountname);
+ res = TranslateName(samname, NameSamCompatible, NameUserPrincipal,
+ NULL, &upnamesize);
+
+ if ((!res && GetLastError() != ERROR_INSUFFICIENT_BUFFER) || upnamesize == 0) {
+ pfree(samname);
+ ereport(LOG,
+ (errcode(ERRCODE_INVALID_ROLE_SPECIFICATION),
+ errmsg("could not translate name")));
+ return STATUS_ERROR;
+ }
+
+ /* upnamesize includes the NUL. */
+ upname = (char*)malloc(upnamesize);
+
+ if (!upname) {
+ pfree(samname);
+ ereport(LOG,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+ return STATUS_ERROR;
+ }
+
+ res = TranslateName(samname, NameSamCompatible, NameUserPrincipal,
+ upname, &upnamesize);
+
+ pfree(samname);
+ if (res) {
+ p = strrchr(upname, '@');
+ }
+
+ if (!res || p == NULL) {
+ ereport(LOG,
+ (errcode(ERRCODE_INVALID_ROLE_SPECIFICATION),
+ errmsg("could not translate name")));
+ return STATUS_ERROR;
+ }
+
+ /* Length of realm name after the '@', including the NUL. */
+ upnamerealmsize = upnamesize - (p - upname + 1);
+
+ /* Replace domainname with realm name. */
+ if (upnamerealmsize > domainnamesize) {
+ free(upname);
+ ereport(LOG,
+ (errcode(ERRCODE_INVALID_ROLE_SPECIFICATION),
+ errmsg("realm name too long")));
+ return STATUS_ERROR;
+ }
+
+ /* Length is now safe. */
+ strcpy(domainname, p+1);
+
+ /* Replace account name as well (in case UPN != SAM)? */
+ if (update_accountname) {
+ if ((p - upname + 1) > accountnamesize) {
+ free(upname);
+ ereport(LOG,
+ (errcode(ERRCODE_INVALID_ROLE_SPECIFICATION),
+ errmsg("translated account name too long")));
+ return STATUS_ERROR;
+ }
+
+ *p = 0;
+ strcpy(accountname, upname);
+ }
+
+ free(upname);
+ return STATUS_OK;
+ }
+
+
#endif /* ENABLE_SSPI */


diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
new file mode 100644
index 28f9fb5..f8defab
*** a/src/backend/libpq/hba.c
--- b/src/backend/libpq/hba.c
*************** parse_hba_line(List *line, int line_num,
*** 1287,1292 ****
--- 1287,1302 ----
parsedline->auth_method == uaSSPI)
parsedline->include_realm = true;

+ /*
+ * For SSPI, include_realm defaults to the SAM-compatible domain
+ * (aka NetBIOS name) and user names instead of the Kerberos
+ * principal name for compatibility.
+ */
+ if (parsedline->auth_method == uaSSPI) {
+ parsedline->real_realm = false;
+ parsedline->upn_username = false;
+ }
+
/* Parse remaining arguments */
while ((field = lnext(field)) != NULL)
{
*************** parse_hba_auth_opt(char *name, char *val
*** 1570,1575 ****
--- 1580,1603 ----
else
hbaline->include_realm = false;
}
+ else if (strcmp(name, "real_realm") == 0)
+ {
+ if (hbaline->auth_method != uaSSPI)
+ INVALID_AUTH_OPTION("real_realm", gettext_noop("sspi"));
+ if (strcmp(val, "1") == 0)
+ hbaline->real_realm = true;
+ else
+ hbaline->real_realm = false;
+ }
+ else if (strcmp(name, "upn_username") == 0)
+ {
+ if (hbaline->auth_method != uaSSPI)
+ INVALID_AUTH_OPTION("upn_username", gettext_noop("sspi"));
+ if (strcmp(val, "1") == 0)
+ hbaline->upn_username = true;
+ else
+ hbaline->upn_username = false;
+ }
else if (strcmp(name, "radiusserver") == 0)
{
struct addrinfo *gai_result;
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
new file mode 100644
index 68a953a..9e4ad8e
*** a/src/include/libpq/hba.h
--- b/src/include/libpq/hba.h
*************** typedef struct HbaLine
*** 77,82 ****
--- 77,84 ----
bool clientcert;
char *krb_realm;
bool include_realm;
+ bool real_realm;
+ bool upn_username;
char *radiusserver;
char *radiussecret;
char *radiusidentifier;

--
Christian Ullrich