Обсуждение: Including kerberos realm

Поиск
Список
Период
Сортировка

Including kerberos realm

От
Magnus Hagander
Дата:
Here's the patch allowing for the parameter include_realm on
pg_hba.conf, that makes the authentication system pass the user@realm
format username to the identmap, instead of stripping the realm. This
was the original reason for having regexp support in the ident maps..

The idea is to make it a lot easier to run with multiple kerberos realms
in the same installation. Hello Stephen :-)

Comments?

//Magnus
*** a/doc/src/sgml/client-auth.sgml
--- b/doc/src/sgml/client-auth.sgml
***************
*** 786,791 **** omicron       bryanh            guest1
--- 786,803 ----
       </varlistentry>

       <varlistentry>
+       <term>include_realm</term>
+       <listitem>
+        <para>
+         Include the realm name from the authenticated user principal. This is useful
+         in combination with Username maps (See <xref linkend="auth-username-maps">
+         for details), especially with regular expressions, to map users from
+         multiple realms.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
        <term>krb_realm</term>
        <listitem>
         <para>
***************
*** 847,852 **** omicron       bryanh            guest1
--- 859,876 ----
       </varlistentry>

       <varlistentry>
+       <term>include_realm</term>
+       <listitem>
+        <para>
+         Include the realm name from the authenticated user principal. This is useful
+         in combination with Username maps (See <xref linkend="auth-username-maps">
+         for details), especially with regular expressions, to map users from
+         multiple realms.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
        <term>krb_realm</term>
        <listitem>
         <para>
*** a/src/backend/libpq/auth.c
--- b/src/backend/libpq/auth.c
***************
*** 748,754 **** pg_krb5_recvauth(Port *port)
      cp = strchr(kusername, '@');
      if (cp)
      {
!         *cp = '\0';
          cp++;

          if (realmmatch != NULL && strlen(realmmatch))
--- 748,760 ----
      cp = strchr(kusername, '@');
      if (cp)
      {
!         /*
!          * If we are not going to include the realm in the username that is passed
!          * to the ident map, destructively modify it here to remove the realm. Then
!          * advance past the separator to check the realm.
!          */
!         if (!port->hba->include_realm)
!             *cp = '\0';
          cp++;

          if (realmmatch != NULL && strlen(realmmatch))
***************
*** 1040,1046 **** pg_GSS_recvauth(Port *port)
      {
          char       *cp = strchr(gbuf.value, '@');

!         *cp = '\0';
          cp++;

          if (realmmatch != NULL && strlen(realmmatch))
--- 1046,1058 ----
      {
          char       *cp = strchr(gbuf.value, '@');

!         /*
!          * If we are not going to include the realm in the username that is passed
!          * to the ident map, destructively modify it here to remove the realm. Then
!          * advance past the separator to check the realm.
!          */
!         if (!port->hba->include_realm)
!             *cp = '\0';
          cp++;

          if (realmmatch != NULL && strlen(realmmatch))
***************
*** 1361,1368 **** pg_SSPI_recvauth(Port *port)
      /*
       * We have the username (without domain/realm) in accountname, compare to
       * the supplied value. In SSPI, always compare case insensitive.
       */
!     return check_usermap(port->hba->usermap, port->user_name, accountname, true);
  }
  #endif   /* ENABLE_SSPI */

--- 1373,1394 ----
      /*
       * We have the username (without domain/realm) in accountname, compare to
       * the supplied value. In SSPI, always compare case insensitive.
+      *
+      * If set to include realm, append it in <username>@<realm> format.
       */
!     if (port->hba->include_realm)
!     {
!         char   *namebuf;
!         int        retval;
!
!         namebuf = palloc(strlen(accountname) + strlen(domainname) + 2);
!         sprintf(namebuf, "%s@%s", accountname, domainname);
!         retval = check_usermap(port->hba->usermap, port->user_name, namebuf, true);
!         pfree(namebuf);
!         return retval;
!     }
!     else
!         return check_usermap(port->hba->usermap, port->user_name, accountname, true);
  }
  #endif   /* ENABLE_SSPI */

*** a/src/backend/libpq/hba.c
--- b/src/backend/libpq/hba.c
***************
*** 1055,1060 **** parse_hba_line(List *line, int line_num, HbaLine *parsedline)
--- 1055,1071 ----
                      INVALID_AUTH_OPTION("krb_realm", "krb5, gssapi and sspi");
                  parsedline->krb_realm = pstrdup(c);
              }
+             else if (strcmp(token, "include_realm") == 0)
+             {
+                 if (parsedline->auth_method != uaKrb5 &&
+                     parsedline->auth_method != uaGSS &&
+                     parsedline->auth_method != uaSSPI)
+                     INVALID_AUTH_OPTION("include_realm", "krb5, gssapi and sspi");
+                 if (strcmp(c, "1") == 0)
+                     parsedline->include_realm = true;
+                 else
+                     parsedline->include_realm = false;
+             }
              else
              {
                  ereport(LOG,
*** a/src/include/libpq/hba.h
--- b/src/include/libpq/hba.h
***************
*** 58,63 **** typedef struct
--- 58,64 ----
      bool        clientcert;
      char       *krb_server_hostname;
      char       *krb_realm;
+     bool        include_realm;
  } HbaLine;

  typedef struct Port hbaPort;

Re: Including kerberos realm

От
Alvaro Herrera
Дата:
Magnus Hagander wrote:
> Here's the patch allowing for the parameter include_realm on
> pg_hba.conf, that makes the authentication system pass the user@realm
> format username to the identmap, instead of stripping the realm.

Not that this affects me in any way, but should there be a GUC variable
to set the default behavior system-wide?

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


Re: Including kerberos realm

От
Magnus Hagander
Дата:
Alvaro Herrera wrote:
> Magnus Hagander wrote:
>> Here's the patch allowing for the parameter include_realm on
>> pg_hba.conf, that makes the authentication system pass the user@realm
>> format username to the identmap, instead of stripping the realm.
> 
> Not that this affects me in any way, but should there be a GUC variable
> to set the default behavior system-wide?

I thought about that, but I don't want to add extra gucs without a good
reason. You'd typically not have very many different lines in pg_hba for
this, and just duplicating the parameter there would be ok I think.

I'd rather move more of the krb parameters to be *just* in pg_hba.conf,
but for now I left those in postgresql.conf as fallbacks..

//Magnus


Re: Including kerberos realm

От
Tom Lane
Дата:
Magnus Hagander <magnus@hagander.net> writes:
> Alvaro Herrera wrote:
>> Not that this affects me in any way, but should there be a GUC variable
>> to set the default behavior system-wide?

> I thought about that, but I don't want to add extra gucs without a good
> reason. You'd typically not have very many different lines in pg_hba for
> this, and just duplicating the parameter there would be ok I think.

> I'd rather move more of the krb parameters to be *just* in pg_hba.conf,
> but for now I left those in postgresql.conf as fallbacks..

If you think those parameters would make more sense in pg_hba.conf,
let's just move them and be done with it.  There has never been any
intention that administrator-only GUCs would be promised compatible
across versions.  And the GUC mechanism is really rather a lot of
overhead compared to options on a pg_hba line ...
        regards, tom lane


Re: Including kerberos realm

От
Magnus Hagander
Дата:
Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> Alvaro Herrera wrote:
>>> Not that this affects me in any way, but should there be a GUC variable
>>> to set the default behavior system-wide?
> 
>> I thought about that, but I don't want to add extra gucs without a good
>> reason. You'd typically not have very many different lines in pg_hba for
>> this, and just duplicating the parameter there would be ok I think.
> 
>> I'd rather move more of the krb parameters to be *just* in pg_hba.conf,
>> but for now I left those in postgresql.conf as fallbacks..
> 
> If you think those parameters would make more sense in pg_hba.conf,
> let's just move them and be done with it.  There has never been any
> intention that administrator-only GUCs would be promised compatible
> across versions.  And the GUC mechanism is really rather a lot of
> overhead compared to options on a pg_hba line ...

Well, it does make sense to have defaults in postgresql.conf - but I
don't think it's worth the overhead.

I'll commit the stuff I have for now and put it on my TODO to remove
them completely from postgresql.conf later. I'll see if I have time to
get it done for 8.4.

//Magnus



Re: Including kerberos realm

От
Magnus Hagander
Дата:
Magnus Hagander wrote:
> Tom Lane wrote:
>> Magnus Hagander <magnus@hagander.net> writes:
>>> Alvaro Herrera wrote:
>>>> Not that this affects me in any way, but should there be a GUC variable
>>>> to set the default behavior system-wide?
>>> I thought about that, but I don't want to add extra gucs without a good
>>> reason. You'd typically not have very many different lines in pg_hba for
>>> this, and just duplicating the parameter there would be ok I think.
>>> I'd rather move more of the krb parameters to be *just* in pg_hba.conf,
>>> but for now I left those in postgresql.conf as fallbacks..
>> If you think those parameters would make more sense in pg_hba.conf,
>> let's just move them and be done with it.  There has never been any
>> intention that administrator-only GUCs would be promised compatible
>> across versions.  And the GUC mechanism is really rather a lot of
>> overhead compared to options on a pg_hba line ...
> 
> Well, it does make sense to have defaults in postgresql.conf - but I
> don't think it's worth the overhead.
> 
> I'll commit the stuff I have for now and put it on my TODO to remove
> them completely from postgresql.conf later. I'll see if I have time to
> get it done for 8.4.

Ok, I've applied a patch for this for the parameter krb_realm and
krb_server_hostname, which are the ones that currently supported both.

Should we also consider moving the remaining ones there?
(krb_server_keyfile, krb_srvname, krb_caseinsens_users)

They do make sense to set on a per-server basis, on the other hand they
are the only remaining authentication-method-specific parameters left...

//Magnus


Re: Including kerberos realm

От
Bruce Momjian
Дата:
Magnus Hagander wrote:
> Magnus Hagander wrote:
> > Tom Lane wrote:
> >> Magnus Hagander <magnus@hagander.net> writes:
> >>> Alvaro Herrera wrote:
> >>>> Not that this affects me in any way, but should there be a GUC variable
> >>>> to set the default behavior system-wide?
> >>> I thought about that, but I don't want to add extra gucs without a good
> >>> reason. You'd typically not have very many different lines in pg_hba for
> >>> this, and just duplicating the parameter there would be ok I think.
> >>> I'd rather move more of the krb parameters to be *just* in pg_hba.conf,
> >>> but for now I left those in postgresql.conf as fallbacks..
> >> If you think those parameters would make more sense in pg_hba.conf,
> >> let's just move them and be done with it.  There has never been any
> >> intention that administrator-only GUCs would be promised compatible
> >> across versions.  And the GUC mechanism is really rather a lot of
> >> overhead compared to options on a pg_hba line ...
> > 
> > Well, it does make sense to have defaults in postgresql.conf - but I
> > don't think it's worth the overhead.
> > 
> > I'll commit the stuff I have for now and put it on my TODO to remove
> > them completely from postgresql.conf later. I'll see if I have time to
> > get it done for 8.4.
> 
> Ok, I've applied a patch for this for the parameter krb_realm and
> krb_server_hostname, which are the ones that currently supported both.
> 
> Should we also consider moving the remaining ones there?
> (krb_server_keyfile, krb_srvname, krb_caseinsens_users)
> 
> They do make sense to set on a per-server basis, on the other hand they
> are the only remaining authentication-method-specific parameters left...

If they make more sense in postgresql.conf, I would just leave them
there.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +