On 2016-03-24 16:35, Christian Ullrich wrote:

* From: Robbie Harwood [mailto:rharw...@redhat.com]

Christian Ullrich <ch...@chrullrich.net> writes:

   pg_SSPI_recvauth(Port *port)
   {
        int                     mtype;
+       int                     status;

The section of this function for include_realm checking already uses an
int status return code (retval).  I would expect to see them share a
variable rather than have both "retval" and "status".

I would not, because retval is local to that last if, but you are right, status
does not need to be in function scope.

Moved declaration.

+       /* 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. */

Since we're already considering changing things: this is not the comment
style for this file (though it is otherwise a good comment).

True. Will fix.

Reformatted.

+       upname = (char*)palloc(upnamesize);

I don't believe this cast is typically included.

Left over from when this was malloc() before Magnus' first look at it.

Removed.

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..6830764
*** 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
  
  /*----------------------------------------------------------------
*************** pg_SSPI_recvauth(Port *port)
*** 1263,1268 ****
--- 1268,1282 ----
  
        free(tokenuser);
  
+       if (!port->hba->compat_realm)
+       {
+               int 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 ****
--- 1312,1419 ----
        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 = 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