* Christian Ullrich wrote:

* From: Magnus Hagander [mailto:mag...@hagander.net]


I don't like the name "real_realm" as a parameter name. I'm wondering if
it might be better to reverse the meaning, and call it sspi_netbios_realm
(and then change the default to on, to be backwards compatible).

What about "compat_realm" instead? "sspi_netbios_realm" is a bit long.
Also, the domain short name is not really a realm name, and this would
feel like implying that it is.

I changed it to "compat_realm".

Code uses a mix of malloc() and palloc() (through sprintf). Is there a
reason for that?

I wasn't sure which to prefer, so I looked around in auth.c, and other than
RADIUS, everything seems to use malloc() (although the sample size is not
too great). Should I use palloc() instead?

The single instance of malloc() has been replaced with palloc().

Another thing: This business of getting a SID, turning it into a user
name/domain pair, then using that to get the UPN (which probably involves
converting it to the SID again somewhere in between) does not look very good
to me. I'll have to see if it works, but what do you think about briefly
impersonating the client, just long enough to get the SAM and UPN names?

I did not pursue this further; it involves quite a bit more code including several more functions from secur32.dll. These might be a problem on MinGW according to the comment in auth.c regarding QuerySecurityContextToken() (if that is still accurate, because my mingw\lib\libsecur32.a apparently has the export).

Updated patch attached.

--
Christian

diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
new file mode 100644
index 3b2935c..f7d7b5a
*** a/doc/src/sgml/client-auth.sgml
--- b/doc/src/sgml/client-auth.sgml
*************** omicron         bryanh
*** 1097,1102 ****
--- 1097,1140 ----
       </varlistentry>
  
       <varlistentry>
+       <term><literal>compat_realm</literal></term>
+       <listitem>
+        <para>
+         If set to 1, 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 0, the true realm name from
+         the Kerberos user principal name is used. Leave this option
+         disabled to maintain compatibility with existing 
+         <filename>pg_ident.conf</filename> files.
+        </para>
+        <para>
+         Do not enable this option unless your server runs under a domain
+         account (this includes virtual service accounts on a domain member
+         system) and all clients authenticating through SSPI are also using
+         domain accounts, or authentication will fail.
+        </para>
+       </listitem>
+      </varlistentry>
+ 
+      <varlistentry>
+       <term><literal>upn_username</literal></term>
+       <listitem>
+        <para>
+         If this option is enabled along with <literal>compat_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.
+         By default, these two names are identical for new user accounts.
+        </para>
+        <para>
+         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 7f1ae8c..f31b06f
*** 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)
*** 1263,1268 ****
--- 1269,1283 ----
  
        free(tokenuser);
  
+       if (!port->hba->compat_realm)
+       {
+               status = pg_SSPI_make_upn(accountname, sizeof(accountname),
+                                                                 domainname, 
sizeof(domainname),
+                                                                 
port->hba->upn_username);
+               if (status != STATUS_OK)
+                       return status;
+       }
+ 
        /*
         * Compare realm/domain if requested. In SSPI, always compare case
         * insensitive.
*************** pg_SSPI_recvauth(Port *port)
*** 1298,1303 ****
--- 1313,1418 ----
        else
                return check_usermap(port->hba->usermap, port->user_name, 
accountname, true);
  }
+ 
+ /*
+  * Replaces the domainname with the Kerberos realm name,
+  * and optionally the accountname with the Kerberos user name.
+  */
+ 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
+          (user@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*)palloc(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 = strchr(upname, '@');
+ 
+       if (!res || p == NULL)
+       {
+               pfree(upname);
+               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)
+       {
+               pfree(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)
+               {
+                       pfree(upname);
+                       ereport(LOG,
+                                       
(errcode(ERRCODE_INVALID_ROLE_SPECIFICATION),
+                                        errmsg("translated account name too 
long")));
+                                        return STATUS_ERROR;
+               }
+ 
+               *p = 0;
+               strcpy(accountname, upname);
+       }
+ 
+       pfree(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..9ca6925
*** 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->compat_realm = true;
+               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, "compat_realm") == 0)
+       {
+               if (hbaline->auth_method != uaSSPI)
+                       INVALID_AUTH_OPTION("compat_realm", 
gettext_noop("sspi"));
+               if (strcmp(val, "1") == 0)
+                       hbaline->compat_realm = true;
+               else
+                       hbaline->compat_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..f3868f7
*** 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            compat_realm;
+       bool            upn_username;
        char       *radiusserver;
        char       *radiussecret;
        char       *radiusidentifier;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to