[pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
32 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Khushboo Vashi
Hi,

Please find the attached patch to support Kerberos Authentication in pgAdmin RM 5457.

The patch introduces a new pluggable option for Kerberos authentication, using SPNEGO to forward kerberos tickets through a browser which will bypass the login page entirely if the Kerberos Authentication succeeds.

The complete setup of the Kerberos Server + pgAdmin Server + Client is documented in a separate file and attached.

This patch also includes the small fix related to logging #5829

Thanks,
Khushboo

RM_5457.patch (35K) Download Attachment
GSSAPI Authentication - pgAdmin.docx (13K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Akshay Joshi
Hi Aditya

Can you please do the code review?

On Tue, Dec 22, 2020 at 3:44 PM Khushboo Vashi <[hidden email]> wrote:
Hi,

Please find the attached patch to support Kerberos Authentication in pgAdmin RM 5457.

The patch introduces a new pluggable option for Kerberos authentication, using SPNEGO to forward kerberos tickets through a browser which will bypass the login page entirely if the Kerberos Authentication succeeds.

The complete setup of the Kerberos Server + pgAdmin Server + Client is documented in a separate file and attached.

This patch also includes the small fix related to logging #5829

Thanks,
Khushboo


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Aditya Toshniwal
Hi Khushboo,

I've just done the code review. Apart from below, the patch looks good to me:

1) Move the auth source constants -ldap, kerberos out of app object. They don't belong there. You can create the constants somewhere else and import them.

+app.PGADMIN_LDAP_AUTH_SOURCE = 'ldap'

+app.PGADMIN_KERBEROS_AUTH_SOURCE = 'kerberos'


2) Are we going to make kerberos default for wsgi ?

--- a/web/pgAdmin4.wsgi

+++ b/web/pgAdmin4.wsgi

@@ -24,6 +24,10 @@ builtins.SERVER_MODE = True

 

 import config

 

+

+config.AUTHENTICATION_SOURCES = ['kerberos']

+config.KERBEROS_AUTO_CREATE_USER = True

+


3) Remove the commented code.

+            # if self.form.data['email'] and self.form.data['password'] and \

+            #         source.get_source_name() ==\

+            #         current_app.PGADMIN_KERBEROS_AUTH_SOURCE:

+            #     continue


4) KERBEROSAuthentication could be KerberosAuthentication

class KERBEROSAuthentication(BaseAuthentication):


5) You can use the constants (ldap, kerberos) you had defined when creating a user.

+                    'auth_source': 'kerberos'


6) The below URLs belong to the authenticate module. Currently they are in the browser module. I would also suggest rephrasing the URL from /kerberos_login to /login/kerberos. Same for logout. Also, even though the method GET works, we should use the POST method for login and DELETE for logout.

+@blueprint.route("/kerberos_login",

+                 endpoint="kerberos_login", methods=["GET"])


+@blueprint.route("/kerberos_logout",

+                 endpoint="kerberos_logout", methods=["GET"])



On Tue, Dec 22, 2020 at 6:07 PM Akshay Joshi <[hidden email]> wrote:
Hi Aditya

Can you please do the code review?

On Tue, Dec 22, 2020 at 3:44 PM Khushboo Vashi <[hidden email]> wrote:
Hi,

Please find the attached patch to support Kerberos Authentication in pgAdmin RM 5457.

The patch introduces a new pluggable option for Kerberos authentication, using SPNEGO to forward kerberos tickets through a browser which will bypass the login page entirely if the Kerberos Authentication succeeds.

The complete setup of the Kerberos Server + pgAdmin Server + Client is documented in a separate file and attached.

This patch also includes the small fix related to logging #5829

Thanks,
Khushboo


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



--
Thanks,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | edbpostgres.com
"Don't Complain about Heat, Plant a TREE"
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Stephen Frost
In reply to this post by Khushboo Vashi
Greetings,

* Khushboo Vashi ([hidden email]) wrote:
> Please find the attached patch to support Kerberos Authentication in
> pgAdmin RM 5457.
>
> The patch introduces a new pluggable option for Kerberos authentication,
> using SPNEGO to forward kerberos tickets through a browser which will
> bypass the login page entirely if the Kerberos Authentication succeeds.

I've taken a (very short) look at this as it's certainly something that
I'm interested in and glad to see work is being done on it.

I notice that 'delegated_creds' is being set but it's unclear to me how
they're actually being used (if at all), which is a very important part
of Kerberos.

What's commonly done with mod_auth_kerb/mod_auth_gss is that the
delegated credentials are stored on the filesystem in a temporary
directory and then an environment variable is set to signal to libpq /
the Kerberos libraries that the delegated credentials can be found in
the temporary file.  I don't see any of that happening in this patch- is
that already handled in some way?  If not, what's the plan for making
that work?  Also important is to make sure that this approach will work
for constrainted delegation implementations.

Thanks!

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Dave Page-7
Hi Stephen 

On Sat, 2 Jan 2021 at 15:41, Stephen Frost <[hidden email]> wrote:
Greetings,

* Khushboo Vashi ([hidden email]) wrote:
> Please find the attached patch to support Kerberos Authentication in
> pgAdmin RM 5457.
>
> The patch introduces a new pluggable option for Kerberos authentication,
> using SPNEGO to forward kerberos tickets through a browser which will
> bypass the login page entirely if the Kerberos Authentication succeeds.

I've taken a (very short) look at this as it's certainly something that
I'm interested in and glad to see work is being done on it.

I notice that 'delegated_creds' is being set but it's unclear to me how
they're actually being used (if at all), which is a very important part
of Kerberos.

What's commonly done with mod_auth_kerb/mod_auth_gss is that the
delegated credentials are stored on the filesystem in a temporary
directory and then an environment variable is set to signal to libpq /
the Kerberos libraries that the delegated credentials can be found in
the temporary file.  I don't see any of that happening in this patch- is
that already handled in some way?  If not, what's the plan for making
that work?  Also important is to make sure that this approach will work
for constrainted delegation implementations.

Phase 1 of this project (which this patch aims to implement) is to handle Kerberos logins to pgAdmin when running in server mode (as we’ve already done for LDAP, except KRB authenticated users don’t see a login page of course). Phase 2 will add support for logging into the PostgreSQL servers - I believe that is where we’ll need to handle delegated credentials, correct?
--
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Stephen Frost
Greetings Dave!

* Dave Page ([hidden email]) wrote:

> On Sat, 2 Jan 2021 at 15:41, Stephen Frost <[hidden email]> wrote:
> > * Khushboo Vashi ([hidden email]) wrote:
> > > Please find the attached patch to support Kerberos Authentication in
> > > pgAdmin RM 5457.
> > >
> > > The patch introduces a new pluggable option for Kerberos authentication,
> > > using SPNEGO to forward kerberos tickets through a browser which will
> > > bypass the login page entirely if the Kerberos Authentication succeeds.
> >
> > I've taken a (very short) look at this as it's certainly something that
> > I'm interested in and glad to see work is being done on it.
> >
> > I notice that 'delegated_creds' is being set but it's unclear to me how
> > they're actually being used (if at all), which is a very important part
> > of Kerberos.
> >
> > What's commonly done with mod_auth_kerb/mod_auth_gss is that the
> > delegated credentials are stored on the filesystem in a temporary
> > directory and then an environment variable is set to signal to libpq /
> > the Kerberos libraries that the delegated credentials can be found in
> > the temporary file.  I don't see any of that happening in this patch- is
> > that already handled in some way?  If not, what's the plan for making
> > that work?  Also important is to make sure that this approach will work
> > for constrainted delegation implementations.
>
> Phase 1 of this project (which this patch aims to implement) is to handle
> Kerberos logins to pgAdmin when running in server mode (as we’ve already
> done for LDAP, except KRB authenticated users don’t see a login page of
> course). Phase 2 will add support for logging into the PostgreSQL servers -
> I believe that is where we’ll need to handle delegated credentials, correct?
Yes, though I sure hope there isn't a plan to release just 'phase 1'
since that would imply that the user is still sending their password to
pgAdmin in some form that pgAdmin then turns around and impersonates the
user, basically completely against how Kerberos auth should be working
in this kind of a intermediate service arrangement.

In other words, if just 'phase 1' is released, it'd probably be CVE
worthy right out of the gate...

Thanks,

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Dave Page-7
Hi

On Sat, 2 Jan 2021 at 15:59, Stephen Frost <[hidden email]> wrote:
Greetings Dave!

* Dave Page ([hidden email]) wrote:
> On Sat, 2 Jan 2021 at 15:41, Stephen Frost <[hidden email]> wrote:
> > * Khushboo Vashi ([hidden email]) wrote:
> > > Please find the attached patch to support Kerberos Authentication in
> > > pgAdmin RM 5457.
> > >
> > > The patch introduces a new pluggable option for Kerberos authentication,
> > > using SPNEGO to forward kerberos tickets through a browser which will
> > > bypass the login page entirely if the Kerberos Authentication succeeds.
> >
> > I've taken a (very short) look at this as it's certainly something that
> > I'm interested in and glad to see work is being done on it.
> >
> > I notice that 'delegated_creds' is being set but it's unclear to me how
> > they're actually being used (if at all), which is a very important part
> > of Kerberos.
> >
> > What's commonly done with mod_auth_kerb/mod_auth_gss is that the
> > delegated credentials are stored on the filesystem in a temporary
> > directory and then an environment variable is set to signal to libpq /
> > the Kerberos libraries that the delegated credentials can be found in
> > the temporary file.  I don't see any of that happening in this patch- is
> > that already handled in some way?  If not, what's the plan for making
> > that work?  Also important is to make sure that this approach will work
> > for constrainted delegation implementations.
>
> Phase 1 of this project (which this patch aims to implement) is to handle
> Kerberos logins to pgAdmin when running in server mode (as we’ve already
> done for LDAP, except KRB authenticated users don’t see a login page of
> course). Phase 2 will add support for logging into the PostgreSQL servers -
> I believe that is where we’ll need to handle delegated credentials, correct?

Yes, though I sure hope there isn't a plan to release just 'phase 1'
since that would imply that the user is still sending their password to
pgAdmin in some form that pgAdmin then turns around and impersonates the
user, basically completely against how Kerberos auth should be working
in this kind of a intermediate service arrangement.

In other words, if just 'phase 1' is released, it'd probably be CVE
worthy right out of the gate...

It wouldn’t impersonate the user at all, it would just work as it does now, requiring the user to supply a username/password for scram/md5/ldap etc, or a cert for SSL auth. Connection to a PostgreSQL server which required gss or sspi simply wouldn’t work (unless the service account under which the pgAdmin server is running has a valid ticket through other means).
--
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Stephen Frost
Greetings,

* Dave Page ([hidden email]) wrote:

> On Sat, 2 Jan 2021 at 15:59, Stephen Frost <[hidden email]> wrote:
> > * Dave Page ([hidden email]) wrote:
> > > On Sat, 2 Jan 2021 at 15:41, Stephen Frost <[hidden email]> wrote:
> > > > * Khushboo Vashi ([hidden email]) wrote:
> > > > > Please find the attached patch to support Kerberos Authentication in
> > > > > pgAdmin RM 5457.
> > > > >
> > > > > The patch introduces a new pluggable option for Kerberos
> > authentication,
> > > > > using SPNEGO to forward kerberos tickets through a browser which will
> > > > > bypass the login page entirely if the Kerberos Authentication
> > succeeds.
> > > >
> > > > I've taken a (very short) look at this as it's certainly something that
> > > > I'm interested in and glad to see work is being done on it.
> > > >
> > > > I notice that 'delegated_creds' is being set but it's unclear to me how
> > > > they're actually being used (if at all), which is a very important part
> > > > of Kerberos.
> > > >
> > > > What's commonly done with mod_auth_kerb/mod_auth_gss is that the
> > > > delegated credentials are stored on the filesystem in a temporary
> > > > directory and then an environment variable is set to signal to libpq /
> > > > the Kerberos libraries that the delegated credentials can be found in
> > > > the temporary file.  I don't see any of that happening in this patch-
> > is
> > > > that already handled in some way?  If not, what's the plan for making
> > > > that work?  Also important is to make sure that this approach will work
> > > > for constrainted delegation implementations.
> > >
> > > Phase 1 of this project (which this patch aims to implement) is to handle
> > > Kerberos logins to pgAdmin when running in server mode (as we’ve already
> > > done for LDAP, except KRB authenticated users don’t see a login page of
> > > course). Phase 2 will add support for logging into the PostgreSQL
> > servers -
> > > I believe that is where we’ll need to handle delegated credentials,
> > correct?
> >
> > Yes, though I sure hope there isn't a plan to release just 'phase 1'
> > since that would imply that the user is still sending their password to
> > pgAdmin in some form that pgAdmin then turns around and impersonates the
> > user, basically completely against how Kerberos auth should be working
> > in this kind of a intermediate service arrangement.
> >
> > In other words, if just 'phase 1' is released, it'd probably be CVE
> > worthy right out of the gate...
>
> It wouldn’t impersonate the user at all, it would just work as it does now,
> requiring the user to supply a username/password for scram/md5/ldap etc, or
> a cert for SSL auth. Connection to a PostgreSQL server which required gss
> or sspi simply wouldn’t work (unless the service account under which the
> pgAdmin server is running has a valid ticket through other means).
That *is* impersonating the user..

Kerberized services really should *not* be accepting a cleartext
password to use to authenticate as the user against another service,
which is why I'd strongly recommend against releasing with just
'phase 1' done.. or at least heavily caveat'ing it that this isn't
actually real Kerberos support but is just an intermediate step that no
one should really deploy...

Thanks,

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Dave Page-7


On Sun, 3 Jan 2021 at 17:31, Stephen Frost <[hidden email]> wrote:
Greetings,

* Dave Page ([hidden email]) wrote:
> On Sat, 2 Jan 2021 at 15:59, Stephen Frost <[hidden email]> wrote:
> > * Dave Page ([hidden email]) wrote:
> > > On Sat, 2 Jan 2021 at 15:41, Stephen Frost <[hidden email]> wrote:
> > > > * Khushboo Vashi ([hidden email]) wrote:
> > > > > Please find the attached patch to support Kerberos Authentication in
> > > > > pgAdmin RM 5457.
> > > > >
> > > > > The patch introduces a new pluggable option for Kerberos
> > authentication,
> > > > > using SPNEGO to forward kerberos tickets through a browser which will
> > > > > bypass the login page entirely if the Kerberos Authentication
> > succeeds.
> > > >
> > > > I've taken a (very short) look at this as it's certainly something that
> > > > I'm interested in and glad to see work is being done on it.
> > > >
> > > > I notice that 'delegated_creds' is being set but it's unclear to me how
> > > > they're actually being used (if at all), which is a very important part
> > > > of Kerberos.
> > > >
> > > > What's commonly done with mod_auth_kerb/mod_auth_gss is that the
> > > > delegated credentials are stored on the filesystem in a temporary
> > > > directory and then an environment variable is set to signal to libpq /
> > > > the Kerberos libraries that the delegated credentials can be found in
> > > > the temporary file.  I don't see any of that happening in this patch-
> > is
> > > > that already handled in some way?  If not, what's the plan for making
> > > > that work?  Also important is to make sure that this approach will work
> > > > for constrainted delegation implementations.
> > >
> > > Phase 1 of this project (which this patch aims to implement) is to handle
> > > Kerberos logins to pgAdmin when running in server mode (as we’ve already
> > > done for LDAP, except KRB authenticated users don’t see a login page of
> > > course). Phase 2 will add support for logging into the PostgreSQL
> > servers -
> > > I believe that is where we’ll need to handle delegated credentials,
> > correct?
> >
> > Yes, though I sure hope there isn't a plan to release just 'phase 1'
> > since that would imply that the user is still sending their password to
> > pgAdmin in some form that pgAdmin then turns around and impersonates the
> > user, basically completely against how Kerberos auth should be working
> > in this kind of a intermediate service arrangement.
> >
> > In other words, if just 'phase 1' is released, it'd probably be CVE
> > worthy right out of the gate...
>
> It wouldn’t impersonate the user at all, it would just work as it does now,
> requiring the user to supply a username/password for scram/md5/ldap etc, or
> a cert for SSL auth. Connection to a PostgreSQL server which required gss
> or sspi simply wouldn’t work (unless the service account under which the
> pgAdmin server is running has a valid ticket through other means).

That *is* impersonating the user..

Kerberized services really should *not* be accepting a cleartext
password to use to authenticate as the user against another service,
which is why I'd strongly recommend against releasing with just
'phase 1' done.. or at least heavily caveat'ing it that this isn't
actually real Kerberos support but is just an intermediate step that no
one should really deploy...

By that argument, one should not be able to login to a kerberised SSH server and then use a mail client to access Gmail (or for that matter, the web interface), neither should one be able to login to a Postgres server using Kerberos, and then use a non-kerberised FDW. 

Why aren’t those cases CVE worthy?
--
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Magnus Hagander-2
In reply to this post by Stephen Frost
On Sun, Jan 3, 2021 at 6:31 PM Stephen Frost <[hidden email]> wrote:

>
> Greetings,
>
> * Dave Page ([hidden email]) wrote:
> > On Sat, 2 Jan 2021 at 15:59, Stephen Frost <[hidden email]> wrote:
> > > * Dave Page ([hidden email]) wrote:
> > > > On Sat, 2 Jan 2021 at 15:41, Stephen Frost <[hidden email]> wrote:
> > > > > * Khushboo Vashi ([hidden email]) wrote:
> > > > > > Please find the attached patch to support Kerberos Authentication in
> > > > > > pgAdmin RM 5457.
> > > > > >
> > > > > > The patch introduces a new pluggable option for Kerberos
> > > authentication,
> > > > > > using SPNEGO to forward kerberos tickets through a browser which will
> > > > > > bypass the login page entirely if the Kerberos Authentication
> > > succeeds.
> > > > >
> > > > > I've taken a (very short) look at this as it's certainly something that
> > > > > I'm interested in and glad to see work is being done on it.
> > > > >
> > > > > I notice that 'delegated_creds' is being set but it's unclear to me how
> > > > > they're actually being used (if at all), which is a very important part
> > > > > of Kerberos.
> > > > >
> > > > > What's commonly done with mod_auth_kerb/mod_auth_gss is that the
> > > > > delegated credentials are stored on the filesystem in a temporary
> > > > > directory and then an environment variable is set to signal to libpq /
> > > > > the Kerberos libraries that the delegated credentials can be found in
> > > > > the temporary file.  I don't see any of that happening in this patch-
> > > is
> > > > > that already handled in some way?  If not, what's the plan for making
> > > > > that work?  Also important is to make sure that this approach will work
> > > > > for constrainted delegation implementations.
> > > >
> > > > Phase 1 of this project (which this patch aims to implement) is to handle
> > > > Kerberos logins to pgAdmin when running in server mode (as we’ve already
> > > > done for LDAP, except KRB authenticated users don’t see a login page of
> > > > course). Phase 2 will add support for logging into the PostgreSQL
> > > servers -
> > > > I believe that is where we’ll need to handle delegated credentials,
> > > correct?
> > >
> > > Yes, though I sure hope there isn't a plan to release just 'phase 1'
> > > since that would imply that the user is still sending their password to
> > > pgAdmin in some form that pgAdmin then turns around and impersonates the
> > > user, basically completely against how Kerberos auth should be working
> > > in this kind of a intermediate service arrangement.
> > >
> > > In other words, if just 'phase 1' is released, it'd probably be CVE
> > > worthy right out of the gate...
> >
> > It wouldn’t impersonate the user at all, it would just work as it does now,
> > requiring the user to supply a username/password for scram/md5/ldap etc, or
> > a cert for SSL auth. Connection to a PostgreSQL server which required gss
> > or sspi simply wouldn’t work (unless the service account under which the
> > pgAdmin server is running has a valid ticket through other means).
>
> That *is* impersonating the user..
>
> Kerberized services really should *not* be accepting a cleartext
> password to use to authenticate as the user against another service,
> which is why I'd strongly recommend against releasing with just
> 'phase 1' done.. or at least heavily caveat'ing it that this isn't
> actually real Kerberos support but is just an intermediate step that no
> one should really deploy...

AIUI that's not what's being proposed.

Correct me if I'm wrong, but I think what's said is that this phase would:

1. Allow kerberos login *to pgadmin*.
2. Do exactly *nothing* to logins to the database server.

So per (2) logins to the db server would work exactly the same as it
does today, and bear no connection to the actual KRB login at all.

One question around that though -- when I click "save password" on a
database connection in pgadmin, it gets stored on the pgadmin server.
Isn't the key used to encrypt that derived from my password?  If I'm
logging into pgadmin without a password (using kerberos),what would
that key be derived from?

--
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/


Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Dave Page-7


On Mon, Jan 11, 2021 at 1:15 PM Magnus Hagander <[hidden email]> wrote:
On Sun, Jan 3, 2021 at 6:31 PM Stephen Frost <[hidden email]> wrote:
>
> Greetings,
>
> * Dave Page ([hidden email]) wrote:
> > On Sat, 2 Jan 2021 at 15:59, Stephen Frost <[hidden email]> wrote:
> > > * Dave Page ([hidden email]) wrote:
> > > > On Sat, 2 Jan 2021 at 15:41, Stephen Frost <[hidden email]> wrote:
> > > > > * Khushboo Vashi ([hidden email]) wrote:
> > > > > > Please find the attached patch to support Kerberos Authentication in
> > > > > > pgAdmin RM 5457.
> > > > > >
> > > > > > The patch introduces a new pluggable option for Kerberos
> > > authentication,
> > > > > > using SPNEGO to forward kerberos tickets through a browser which will
> > > > > > bypass the login page entirely if the Kerberos Authentication
> > > succeeds.
> > > > >
> > > > > I've taken a (very short) look at this as it's certainly something that
> > > > > I'm interested in and glad to see work is being done on it.
> > > > >
> > > > > I notice that 'delegated_creds' is being set but it's unclear to me how
> > > > > they're actually being used (if at all), which is a very important part
> > > > > of Kerberos.
> > > > >
> > > > > What's commonly done with mod_auth_kerb/mod_auth_gss is that the
> > > > > delegated credentials are stored on the filesystem in a temporary
> > > > > directory and then an environment variable is set to signal to libpq /
> > > > > the Kerberos libraries that the delegated credentials can be found in
> > > > > the temporary file.  I don't see any of that happening in this patch-
> > > is
> > > > > that already handled in some way?  If not, what's the plan for making
> > > > > that work?  Also important is to make sure that this approach will work
> > > > > for constrainted delegation implementations.
> > > >
> > > > Phase 1 of this project (which this patch aims to implement) is to handle
> > > > Kerberos logins to pgAdmin when running in server mode (as we’ve already
> > > > done for LDAP, except KRB authenticated users don’t see a login page of
> > > > course). Phase 2 will add support for logging into the PostgreSQL
> > > servers -
> > > > I believe that is where we’ll need to handle delegated credentials,
> > > correct?
> > >
> > > Yes, though I sure hope there isn't a plan to release just 'phase 1'
> > > since that would imply that the user is still sending their password to
> > > pgAdmin in some form that pgAdmin then turns around and impersonates the
> > > user, basically completely against how Kerberos auth should be working
> > > in this kind of a intermediate service arrangement.
> > >
> > > In other words, if just 'phase 1' is released, it'd probably be CVE
> > > worthy right out of the gate...
> >
> > It wouldn’t impersonate the user at all, it would just work as it does now,
> > requiring the user to supply a username/password for scram/md5/ldap etc, or
> > a cert for SSL auth. Connection to a PostgreSQL server which required gss
> > or sspi simply wouldn’t work (unless the service account under which the
> > pgAdmin server is running has a valid ticket through other means).
>
> That *is* impersonating the user..
>
> Kerberized services really should *not* be accepting a cleartext
> password to use to authenticate as the user against another service,
> which is why I'd strongly recommend against releasing with just
> 'phase 1' done.. or at least heavily caveat'ing it that this isn't
> actually real Kerberos support but is just an intermediate step that no
> one should really deploy...

AIUI that's not what's being proposed.

Correct me if I'm wrong, but I think what's said is that this phase would:

1. Allow kerberos login *to pgadmin*.
2. Do exactly *nothing* to logins to the database server.

So per (2) logins to the db server would work exactly the same as it
does today, and bear no connection to the actual KRB login at all.

Correct.
 

One question around that though -- when I click "save password" on a
database connection in pgadmin, it gets stored on the pgadmin server.
Isn't the key used to encrypt that derived from my password?  If I'm
logging into pgadmin without a password (using kerberos),what would
that key be derived from?

Also correct - and right now, the plan is to disable password saving if logged in using Kerberos. 

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com

Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Stephen Frost
Greetings,

* Dave Page ([hidden email]) wrote:
> On Mon, Jan 11, 2021 at 1:15 PM Magnus Hagander <[hidden email]> wrote:
> > One question around that though -- when I click "save password" on a
> > database connection in pgadmin, it gets stored on the pgadmin server.
> > Isn't the key used to encrypt that derived from my password?  If I'm
> > logging into pgadmin without a password (using kerberos),what would
> > that key be derived from?
>
> Also correct - and right now, the plan is to disable password saving if
> logged in using Kerberos.

Disable password *saving*, or disable password *using*?

If you're saying that, when Kerberos is enabled, users will never be
prompted to provide a password because password-based auth has been
disabled, then perhaps that's reasonable.  I don't know how useful such
a pgadmin setup would be, but at least it wouldn't be violating one of
the core values that using Kerberos brings.

If you're saying that this is just disabling password *saving*, then
that implies that if someone actually wants to use pgadmin to, uh, log
into a PostgreSQL server which is configured for md5 or SCRAM auth or
LDAP based auth that the way that'll work is that pgadmin will prompt
the user for a password, which the user will provide and which will
then be sent from the client to the pgadmin system in the clear, and
which pgadmin will turn around and use to log into PG with, right?

It's the latter than I'm concerned with because it just wouldn't be
appropriate for a Kerberized service which is set up to use Kerberos to
then prompt the user for a password.

In any case, I have a really hard time seeing this as being something
that it'd be good for the pgAdmin team to publish as "we now have
Kerberos support!" because, either way, it doesn't seem like it would be
usable in a secure manner in a Kerberized environment.  Once "phase 2"
is done (which hopefully will include both traditional credential
delegating and constrainted delegation support...), then it'll be a game
changer imv and something that everyone should be shouting from the
rooftops about and I'll be right there cheering it on too..

Thanks,

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Dave Page-7


On Mon, Jan 11, 2021 at 4:50 PM Stephen Frost <[hidden email]> wrote:
Greetings,

* Dave Page ([hidden email]) wrote:
> On Mon, Jan 11, 2021 at 1:15 PM Magnus Hagander <[hidden email]> wrote:
> > One question around that though -- when I click "save password" on a
> > database connection in pgadmin, it gets stored on the pgadmin server.
> > Isn't the key used to encrypt that derived from my password?  If I'm
> > logging into pgadmin without a password (using kerberos),what would
> > that key be derived from?
>
> Also correct - and right now, the plan is to disable password saving if
> logged in using Kerberos.

Disable password *saving*, or disable password *using*?

I'm pretty sure I wrote "saving".
 

If you're saying that, when Kerberos is enabled, users will never be
prompted to provide a password because password-based auth has been
disabled, then perhaps that's reasonable.  I don't know how useful such
a pgadmin setup would be, but at least it wouldn't be violating one of
the core values that using Kerberos brings.

If you're saying that this is just disabling password *saving*, then
that implies that if someone actually wants to use pgadmin to, uh, log
into a PostgreSQL server which is configured for md5 or SCRAM auth or
LDAP based auth that the way that'll work is that pgadmin will prompt
the user for a password, which the user will provide and which will
then be sent from the client to the pgadmin system in the clear, and
which pgadmin will turn around and use to log into PG with, right?

Yes.
 

It's the latter than I'm concerned with because it just wouldn't be
appropriate for a Kerberized service which is set up to use Kerberos to
then prompt the user for a password.

Well you never answered my previous question about that. Why is it appropriate for an FDW to do that, but not pgAdmin? Or for a user on a kerberised machine to use a web browser to access a non-kerberised site? Or frankly pretty much anything outside of a windows domain or kerberos environment that a user inside the environment might want to use?

You basically seem to be saying that once a user logs into something using Kerberos, *everything* else they login to from there must also be done using Kerberos - which clearly will not be the case in the vast majority of deployments.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com

Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Stephen Frost
Greetings,

* Dave Page ([hidden email]) wrote:

> On Mon, Jan 11, 2021 at 4:50 PM Stephen Frost <[hidden email]> wrote:
> > If you're saying that, when Kerberos is enabled, users will never be
> > prompted to provide a password because password-based auth has been
> > disabled, then perhaps that's reasonable.  I don't know how useful such
> > a pgadmin setup would be, but at least it wouldn't be violating one of
> > the core values that using Kerberos brings.
> >
> > If you're saying that this is just disabling password *saving*, then
> > that implies that if someone actually wants to use pgadmin to, uh, log
> > into a PostgreSQL server which is configured for md5 or SCRAM auth or
> > LDAP based auth that the way that'll work is that pgadmin will prompt
> > the user for a password, which the user will provide and which will
> > then be sent from the client to the pgadmin system in the clear, and
> > which pgadmin will turn around and use to log into PG with, right?
>
> Yes.
Alright, glad I wasn't completely misunderstanding something.

> > It's the latter than I'm concerned with because it just wouldn't be
> > appropriate for a Kerberized service which is set up to use Kerberos to
> > then prompt the user for a password.
>
> Well you never answered my previous question about that. Why is it
> appropriate for an FDW to do that, but not pgAdmin? Or for a user on a
> kerberised machine to use a web browser to access a non-kerberised site? Or
> frankly pretty much anything outside of a windows domain or kerberos
> environment that a user inside the environment might want to use?

Pretty sure I didn't say it was appropriate for an FDW to do that, it
really isn't, but FDWs are also a side feature of the overall product,
not a core component, and you have to be granted specific rights to be
able to use an FDW.

Accessing systems outside of the Kerberized environment is obviously a
different situation as you *can't* use the Kerberos credentials- and,
hopefully, everyone is using password managers and has a distinct and
different password for every service they do use outside of the
Kerberized environment.  When you're talking about a set of systems
which live inside of the Kerberized environment, however, it's simply
not sensible to ask the user to provide their *domain-level* credentials
which an attacker could use to log in as that user to the entire domain
and have complete access over their account and that's exactly what is
likely to end up being the case here because the only way to set this up
would be Kerberos for pgAdmin and LDAP for PG- at least until delegated
credentials are implemented.

> You basically seem to be saying that once a user logs into something using
> Kerberos, *everything* else they login to from there must also be done
> using Kerberos - which clearly will not be the case in the vast majority of
> deployments.

Everything else they login to from there in the same Kerberized
environment absolutely should be done using Kerberos delegated
credentials.  That's the point of Kerberos delegation.  Are you modeling
this approach based on some existing system which accepts Kerberos
logins but then *doesn't* allow use of delegated credentials to log into
other Kerberized systems from there?  Surely SSH works great with
delegated credentials, as does any website that uses mod_auth_kerb or
mod_auth_gss, or IIS..

I sure hope that the vast majority of deployments where pgAdmin is set
up with Kerberos will be using Kerberos for logging into PG with
delegated credentials, and further, that we will be *strongly*
encouraging that as otherwise you might as well use LDAP auth for all of
it and accept that any compromise of the web server or of PG will result
in complete compromise of any user's account who accesses the system.

I don't understand all this push-back.

The intent is to do the 'phase 2', right?  And it hopefully will happen
in relatively short order, no?  At least, I'd think it would make sense,
while people have developer environments set up and working with
Kerberos to go ahead and get that part done.  All I'm saying is that the
'phase 1' part really shouldn't be independently released, or if it is,
it should be *heavily* caveated that it is strongly discouraged for
people to run it in an environment where pgadmin and PG are in the same
Kerberized environment because it's not possible to set that up, with
just phase 1 done, in a manner which would avoid the pgadmin and PG
servers seeing the user's password.

Thanks,

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Dave Page-7
Hi

On Mon, Jan 11, 2021 at 5:42 PM Stephen Frost <[hidden email]> wrote:

Accessing systems outside of the Kerberized environment is obviously a
different situation as you *can't* use the Kerberos credentials- and,
hopefully, everyone is using password managers and has a distinct and
different password for every service they do use outside of the
Kerberized environment.  When you're talking about a set of systems
which live inside of the Kerberized environment, however, it's simply
not sensible to ask the user to provide their *domain-level* credentials
which an attacker could use to log in as that user to the entire domain
and have complete access over their account and that's exactly what is
likely to end up being the case here because the only way to set this up
would be Kerberos for pgAdmin and LDAP for PG- at least until delegated
credentials are implemented.

Which is no worse than the current situation - in fact it's arguably better because there's one less system that isn't Kerberised.

Don't forget, you (as the system administrator) also have the choice of whether or not to use Kerberos. If you're not happy to have the pgAdmin authentication be kerberised whilst the database server access is not, then don't enable Kerberos until phase 2 is complete.
 

> You basically seem to be saying that once a user logs into something using
> Kerberos, *everything* else they login to from there must also be done
> using Kerberos - which clearly will not be the case in the vast majority of
> deployments.

Everything else they login to from there in the same Kerberized
environment absolutely should be done using Kerberos delegated
credentials.  That's the point of Kerberos delegation.  Are you modeling
this approach based on some existing system which accepts Kerberos
logins but then *doesn't* allow use of delegated credentials to log into
other Kerberized systems from there?  Surely SSH works great with
delegated credentials, as does any website that uses mod_auth_kerb or
mod_auth_gss, or IIS..

I sure hope that the vast majority of deployments where pgAdmin is set
up with Kerberos will be using Kerberos for logging into PG with
delegated credentials, and further, that we will be *strongly*
encouraging that as otherwise you might as well use LDAP auth for all of
it and accept that any compromise of the web server or of PG will result
in complete compromise of any user's account who accesses the system.

I suspect that may not be the case, or at least most people will be working in mixed environments, e.g. Kerberos on their local network with non-Kerberised RDS servers for example. This is certainly something I've seen in the field many times.
 

I don't understand all this push-back.

There are benefits for some users with phase one alone, so I don't see (and still don't) a need to hold it back. It also potentially allows us to get feedback on things that don't work as expected earlier, to minimise any re-work that might be required. Don't forget that pgAdmin releases monthly (except around EOY for obvious reasons), and incrementally releases and adds features, unlike PostgreSQL.
 

The intent is to do the 'phase 2', right?  And it hopefully will happen
in relatively short order, no?  At least, I'd think it would make sense,
while people have developer environments set up and working with
Kerberos to go ahead and get that part done.  All I'm saying is that the
'phase 1' part really shouldn't be independently released, or if it is,
it should be *heavily* caveated that it is strongly discouraged for
people to run it in an environment where pgadmin and PG are in the same
Kerberized environment because it's not possible to set that up, with
just phase 1 done, in a manner which would avoid the pgadmin and PG
servers seeing the user's password.

Phase 2 is scheduled to be done immediately.  

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com

Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Magnus Hagander-2
On Tue, Jan 12, 2021 at 10:08 AM Dave Page <[hidden email]> wrote:

>
> Hi
>
> On Mon, Jan 11, 2021 at 5:42 PM Stephen Frost <[hidden email]> wrote:
>>
>>
>> Accessing systems outside of the Kerberized environment is obviously a
>> different situation as you *can't* use the Kerberos credentials- and,
>> hopefully, everyone is using password managers and has a distinct and
>> different password for every service they do use outside of the
>> Kerberized environment.  When you're talking about a set of systems
>> which live inside of the Kerberized environment, however, it's simply
>> not sensible to ask the user to provide their *domain-level* credentials
>> which an attacker could use to log in as that user to the entire domain
>> and have complete access over their account and that's exactly what is
>> likely to end up being the case here because the only way to set this up
>> would be Kerberos for pgAdmin and LDAP for PG- at least until delegated
>> credentials are implemented.
>
>
> Which is no worse than the current situation - in fact it's arguably better because there's one less system that isn't Kerberised.
>
> Don't forget, you (as the system administrator) also have the choice of whether or not to use Kerberos. If you're not happy to have the pgAdmin authentication be kerberised whilst the database server access is not, then don't enable Kerberos until phase 2 is complete.
>
>>
>>
>> > You basically seem to be saying that once a user logs into something using
>> > Kerberos, *everything* else they login to from there must also be done
>> > using Kerberos - which clearly will not be the case in the vast majority of
>> > deployments.
>>
>> Everything else they login to from there in the same Kerberized
>> environment absolutely should be done using Kerberos delegated
>> credentials.  That's the point of Kerberos delegation.  Are you modeling
>> this approach based on some existing system which accepts Kerberos
>> logins but then *doesn't* allow use of delegated credentials to log into
>> other Kerberized systems from there?  Surely SSH works great with
>> delegated credentials, as does any website that uses mod_auth_kerb or
>> mod_auth_gss, or IIS..
>>
>> I sure hope that the vast majority of deployments where pgAdmin is set
>> up with Kerberos will be using Kerberos for logging into PG with
>> delegated credentials, and further, that we will be *strongly*
>> encouraging that as otherwise you might as well use LDAP auth for all of
>> it and accept that any compromise of the web server or of PG will result
>> in complete compromise of any user's account who accesses the system.
>
>
> I suspect that may not be the case, or at least most people will be working in mixed environments, e.g. Kerberos on their local network with non-Kerberised RDS servers for example. This is certainly something I've seen in the field many times.

+1. I can see a lot of cases where people would like to benefit from
the *convenience* of Kerberos login into their pgadmin, and then
continue to use a db connection that does not use Kerberos. There's
many orgs that for example have a policy that says they *must* use
passwords in to the db regardless of Kerbeos. We can argue whether
that's a smart policy or not, but it's very real, and those people
would still be able to benefit from a Kerberos login into pgadmin.

Getting those people to do kerberos into pgadmin and then password
intot he database would be a strong improvement over ldap to pgadmin
and password into the database. Sure, if the ldap password and the db
password is the same the difference isn't that big, but more often
than not the db password is independent.

RDS is a good example of this, but there are definitely plenty of
non-cloud environments who would also benefit fro that.


>> I don't understand all this push-back.
>
>
> There are benefits for some users with phase one alone, so I don't see (and still don't) a need to hold it back. It also potentially allows us to get feedback on things that don't work as expected earlier, to minimise any re-work that might be required. Don't forget that pgAdmin releases monthly (except around EOY for obvious reasons), and incrementally releases and adds features, unlike PostgreSQL.
>
>>
>>
>> The intent is to do the 'phase 2', right?  And it hopefully will happen
>> in relatively short order, no?  At least, I'd think it would make sense,
>> while people have developer environments set up and working with
>> Kerberos to go ahead and get that part done.  All I'm saying is that the
>> 'phase 1' part really shouldn't be independently released, or if it is,
>> it should be *heavily* caveated that it is strongly discouraged for
>> people to run it in an environment where pgadmin and PG are in the same
>> Kerberized environment because it's not possible to set that up, with
>> just phase 1 done, in a manner which would avoid the pgadmin and PG
>> servers seeing the user's password.
>
>
> Phase 2 is scheduled to be done immediately.

\o/


--
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/


Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Khushboo Vashi
In reply to this post by Aditya Toshniwal
Hi,

Please find the attached updated patch.

Thanks,
Khushboo

On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal <[hidden email]> wrote:
Hi Khushboo,

I've just done the code review. Apart from below, the patch looks good to me:

1) Move the auth source constants -ldap, kerberos out of app object. They don't belong there. You can create the constants somewhere else and import them.

+app.PGADMIN_LDAP_AUTH_SOURCE = 'ldap'

+app.PGADMIN_KERBEROS_AUTH_SOURCE = 'kerberos'


Done 

2) Are we going to make kerberos default for wsgi ?

--- a/web/pgAdmin4.wsgi

+++ b/web/pgAdmin4.wsgi

@@ -24,6 +24,10 @@ builtins.SERVER_MODE = True

 

 import config

 

+

+config.AUTHENTICATION_SOURCES = ['kerberos']

+config.KERBEROS_AUTO_CREATE_USER = True

+


Removed, it was only for testing. 

3) Remove the commented code.

+            # if self.form.data['email'] and self.form.data['password'] and \

+            #         source.get_source_name() ==\

+            #         current_app.PGADMIN_KERBEROS_AUTH_SOURCE:

+            #     continue


Removed the comment, it is actually the part of the code. 

4) KERBEROSAuthentication could be KerberosAuthentication

class KERBEROSAuthentication(BaseAuthentication):


Done. 

5) You can use the constants (ldap, kerberos) you had defined when creating a user.

+                    'auth_source': 'kerberos'


Done. 

6) The below URLs belong to the authenticate module. Currently they are in the browser module. I would also suggest rephrasing the URL from /kerberos_login to /login/kerberos. Same for logout.
Done the rephrasing as well as moved to the authentication module.
 
Also, even though the method GET works, we should use the POST method for login and DELETE for logout.
Kerberos_login just redirects the page to the actual login, so no need for the POST method.
I followed the same method for the Logout user we have used for the normal user.
 

+@blueprint.route("/kerberos_login",

+                 endpoint="kerberos_login", methods=["GET"])


+@blueprint.route("/kerberos_logout",

+                 endpoint="kerberos_logout", methods=["GET"])



 
On Tue, Dec 22, 2020 at 6:07 PM Akshay Joshi <[hidden email]> wrote:
Hi Aditya

Can you please do the code review?

On Tue, Dec 22, 2020 at 3:44 PM Khushboo Vashi <[hidden email]> wrote:
Hi,

Please find the attached patch to support Kerberos Authentication in pgAdmin RM 5457.

The patch introduces a new pluggable option for Kerberos authentication, using SPNEGO to forward kerberos tickets through a browser which will bypass the login page entirely if the Kerberos Authentication succeeds.

The complete setup of the Kerberos Server + pgAdmin Server + Client is documented in a separate file and attached.

This patch also includes the small fix related to logging #5829

Thanks,
Khushboo


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



--
Thanks,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | edbpostgres.com
"Don't Complain about Heat, Plant a TREE"

RM_5457_v1.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Akshay Joshi
Hi Khushboo

Seems you have attached the wrong patch. Please send the updated patch.

On Wed, Jan 13, 2021 at 2:35 PM Khushboo Vashi <[hidden email]> wrote:
Hi,

Please find the attached updated patch.

Thanks,
Khushboo

On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal <[hidden email]> wrote:
Hi Khushboo,

I've just done the code review. Apart from below, the patch looks good to me:

1) Move the auth source constants -ldap, kerberos out of app object. They don't belong there. You can create the constants somewhere else and import them.

+app.PGADMIN_LDAP_AUTH_SOURCE = 'ldap'

+app.PGADMIN_KERBEROS_AUTH_SOURCE = 'kerberos'


Done 

2) Are we going to make kerberos default for wsgi ?

--- a/web/pgAdmin4.wsgi

+++ b/web/pgAdmin4.wsgi

@@ -24,6 +24,10 @@ builtins.SERVER_MODE = True

 

 import config

 

+

+config.AUTHENTICATION_SOURCES = ['kerberos']

+config.KERBEROS_AUTO_CREATE_USER = True

+


Removed, it was only for testing. 

3) Remove the commented code.

+            # if self.form.data['email'] and self.form.data['password'] and \

+            #         source.get_source_name() ==\

+            #         current_app.PGADMIN_KERBEROS_AUTH_SOURCE:

+            #     continue


Removed the comment, it is actually the part of the code. 

4) KERBEROSAuthentication could be KerberosAuthentication

class KERBEROSAuthentication(BaseAuthentication):


Done. 

5) You can use the constants (ldap, kerberos) you had defined when creating a user.

+                    'auth_source': 'kerberos'


Done. 

6) The below URLs belong to the authenticate module. Currently they are in the browser module. I would also suggest rephrasing the URL from /kerberos_login to /login/kerberos. Same for logout.
Done the rephrasing as well as moved to the authentication module.
 
Also, even though the method GET works, we should use the POST method for login and DELETE for logout.
Kerberos_login just redirects the page to the actual login, so no need for the POST method.
I followed the same method for the Logout user we have used for the normal user.
 

+@blueprint.route("/kerberos_login",

+                 endpoint="kerberos_login", methods=["GET"])


+@blueprint.route("/kerberos_logout",

+                 endpoint="kerberos_logout", methods=["GET"])



 
On Tue, Dec 22, 2020 at 6:07 PM Akshay Joshi <[hidden email]> wrote:
Hi Aditya

Can you please do the code review?

On Tue, Dec 22, 2020 at 3:44 PM Khushboo Vashi <[hidden email]> wrote:
Hi,

Please find the attached patch to support Kerberos Authentication in pgAdmin RM 5457.

The patch introduces a new pluggable option for Kerberos authentication, using SPNEGO to forward kerberos tickets through a browser which will bypass the login page entirely if the Kerberos Authentication succeeds.

The complete setup of the Kerberos Server + pgAdmin Server + Client is documented in a separate file and attached.

This patch also includes the small fix related to logging #5829

Thanks,
Khushboo


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



--
Thanks,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | edbpostgres.com
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Khushboo Vashi
Hi,

Please find the attached updated patch.

Thanks,
Khushboo

On Thu, Jan 14, 2021 at 12:00 PM Akshay Joshi <[hidden email]> wrote:
Hi Khushboo

Seems you have attached the wrong patch. Please send the updated patch.

On Wed, Jan 13, 2021 at 2:35 PM Khushboo Vashi <[hidden email]> wrote:
Hi,

Please find the attached updated patch.

Thanks,
Khushboo

On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal <[hidden email]> wrote:
Hi Khushboo,

I've just done the code review. Apart from below, the patch looks good to me:

1) Move the auth source constants -ldap, kerberos out of app object. They don't belong there. You can create the constants somewhere else and import them.

+app.PGADMIN_LDAP_AUTH_SOURCE = 'ldap'

+app.PGADMIN_KERBEROS_AUTH_SOURCE = 'kerberos'


Done 

2) Are we going to make kerberos default for wsgi ?

--- a/web/pgAdmin4.wsgi

+++ b/web/pgAdmin4.wsgi

@@ -24,6 +24,10 @@ builtins.SERVER_MODE = True

 

 import config

 

+

+config.AUTHENTICATION_SOURCES = ['kerberos']

+config.KERBEROS_AUTO_CREATE_USER = True

+


Removed, it was only for testing. 

3) Remove the commented code.

+            # if self.form.data['email'] and self.form.data['password'] and \

+            #         source.get_source_name() ==\

+            #         current_app.PGADMIN_KERBEROS_AUTH_SOURCE:

+            #     continue


Removed the comment, it is actually the part of the code. 

4) KERBEROSAuthentication could be KerberosAuthentication

class KERBEROSAuthentication(BaseAuthentication):


Done. 

5) You can use the constants (ldap, kerberos) you had defined when creating a user.

+                    'auth_source': 'kerberos'


Done. 

6) The below URLs belong to the authenticate module. Currently they are in the browser module. I would also suggest rephrasing the URL from /kerberos_login to /login/kerberos. Same for logout.
Done the rephrasing as well as moved to the authentication module.
 
Also, even though the method GET works, we should use the POST method for login and DELETE for logout.
Kerberos_login just redirects the page to the actual login, so no need for the POST method.
I followed the same method for the Logout user we have used for the normal user.
 

+@blueprint.route("/kerberos_login",

+                 endpoint="kerberos_login", methods=["GET"])


+@blueprint.route("/kerberos_logout",

+                 endpoint="kerberos_logout", methods=["GET"])



 
On Tue, Dec 22, 2020 at 6:07 PM Akshay Joshi <[hidden email]> wrote:
Hi Aditya

Can you please do the code review?

On Tue, Dec 22, 2020 at 3:44 PM Khushboo Vashi <[hidden email]> wrote:
Hi,

Please find the attached patch to support Kerberos Authentication in pgAdmin RM 5457.

The patch introduces a new pluggable option for Kerberos authentication, using SPNEGO to forward kerberos tickets through a browser which will bypass the login page entirely if the Kerberos Authentication succeeds.

The complete setup of the Kerberos Server + pgAdmin Server + Client is documented in a separate file and attached.

This patch also includes the small fix related to logging #5829

Thanks,
Khushboo


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



--
Thanks,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | edbpostgres.com
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246


RM_5457_v2.patch (51K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [pgAdmin4][Patch] - RM 5457 - Kerberos Authentication - Phase 1

Khushboo Vashi
Hi, 

Please ignore my previous patch, attached the updated one.

Thanks,
Khushboo

On Thu, Jan 14, 2021 at 12:17 PM Khushboo Vashi <[hidden email]> wrote:
Hi,

Please find the attached updated patch.

Thanks,
Khushboo

On Thu, Jan 14, 2021 at 12:00 PM Akshay Joshi <[hidden email]> wrote:
Hi Khushboo

Seems you have attached the wrong patch. Please send the updated patch.

On Wed, Jan 13, 2021 at 2:35 PM Khushboo Vashi <[hidden email]> wrote:
Hi,

Please find the attached updated patch.

Thanks,
Khushboo

On Fri, Jan 1, 2021 at 1:07 PM Aditya Toshniwal <[hidden email]> wrote:
Hi Khushboo,

I've just done the code review. Apart from below, the patch looks good to me:

1) Move the auth source constants -ldap, kerberos out of app object. They don't belong there. You can create the constants somewhere else and import them.

+app.PGADMIN_LDAP_AUTH_SOURCE = 'ldap'

+app.PGADMIN_KERBEROS_AUTH_SOURCE = 'kerberos'


Done 

2) Are we going to make kerberos default for wsgi ?

--- a/web/pgAdmin4.wsgi

+++ b/web/pgAdmin4.wsgi

@@ -24,6 +24,10 @@ builtins.SERVER_MODE = True

 

 import config

 

+

+config.AUTHENTICATION_SOURCES = ['kerberos']

+config.KERBEROS_AUTO_CREATE_USER = True

+


Removed, it was only for testing. 

3) Remove the commented code.

+            # if self.form.data['email'] and self.form.data['password'] and \

+            #         source.get_source_name() ==\

+            #         current_app.PGADMIN_KERBEROS_AUTH_SOURCE:

+            #     continue


Removed the comment, it is actually the part of the code. 

4) KERBEROSAuthentication could be KerberosAuthentication

class KERBEROSAuthentication(BaseAuthentication):


Done. 

5) You can use the constants (ldap, kerberos) you had defined when creating a user.

+                    'auth_source': 'kerberos'


Done. 

6) The below URLs belong to the authenticate module. Currently they are in the browser module. I would also suggest rephrasing the URL from /kerberos_login to /login/kerberos. Same for logout.
Done the rephrasing as well as moved to the authentication module.
 
Also, even though the method GET works, we should use the POST method for login and DELETE for logout.
Kerberos_login just redirects the page to the actual login, so no need for the POST method.
I followed the same method for the Logout user we have used for the normal user.
 

+@blueprint.route("/kerberos_login",

+                 endpoint="kerberos_login", methods=["GET"])


+@blueprint.route("/kerberos_logout",

+                 endpoint="kerberos_logout", methods=["GET"])



 
On Tue, Dec 22, 2020 at 6:07 PM Akshay Joshi <[hidden email]> wrote:
Hi Aditya

Can you please do the code review?

On Tue, Dec 22, 2020 at 3:44 PM Khushboo Vashi <[hidden email]> wrote:
Hi,

Please find the attached patch to support Kerberos Authentication in pgAdmin RM 5457.

The patch introduces a new pluggable option for Kerberos authentication, using SPNEGO to forward kerberos tickets through a browser which will bypass the login page entirely if the Kerberos Authentication succeeds.

The complete setup of the Kerberos Server + pgAdmin Server + Client is documented in a separate file and attached.

This patch also includes the small fix related to logging #5829

Thanks,
Khushboo


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246



--
Thanks,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | edbpostgres.com
"Don't Complain about Heat, Plant a TREE"


--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Principal Software Architect
EDB Postgres
Mobile: +91 976-788-8246


RM_5457_v3.patch (51K) Download Attachment
12