Обсуждение: [pgAdmin4][Patch]: Allow user to provide custom SSL certificates andprovide .pgpass file

Hi Dave,

On Fri, Aug 25, 2017 at 2:14 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Thu, Aug 24, 2017 at 9:02 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi,

PFA patch to allow user to provide custom path for SSL certificates and also allow user to pass .pgpass file when making server connection.

RM#2649

RM#2650

SSL certificates options reference:

https://www.postgresql.org/docs/10/static/libpq-connect.html

https://www.postgresql.org/docs/9.1/static/ssl-tcp.html

.pgpass file reference:

https://www.postgresql.org/docs/10/static/libpq-pgpass.html

Nice! A few comments:

- Should we enable the certificate options for "Prefer" mode? What did we do in pgAdmin 3?

​In pgAdmin3, all SSL options are enabled for each mode.​

Yes, we should also consider 'prefer' mode.

- We need a doc update :-). Please include updates to all the screenshots on the relevant page, to ensure they all show the right tabs.

​Sure​, will sent it in next patch.

- I think the file browser needs an option to view hidden files/folders. Typically a pgpass file is at ~/.pgpass on *nix systems, and certificates are likely to be stored in ~/.ssh. 

​Let me check the file manager code, I will update on this.​

 

Thanks! 

--

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi

On Wed, Aug 30, 2017 at 6:49 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi Dave,

PFA updated patch with new screenshots and docs accordingly.

RM

​#​

2649

​ & RM#​

2650

On Tue, Aug 29, 2017 at 7:51 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Fri, Aug 25, 2017 at 2:45 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi Dave,

Please find updated patch, 

- For displaying hidden files I have added preference option in Storage section.

How painful would it be to include it on the file dialogue as well?

​Done​

 

 

- Updated Docs & Screenshots.

- User can use 'prefer' option to enable SSL options.

Cool.

A couple of other things I realised in playing with this:

1) The SSL tab should come before Advanced I think.

​Done​

 

2) The docs now mention the default SSL files. In server mode, using defaults is probably a bad idea I suspect (because they would be shared). Should we force the values to /dev/null (and whatever is appropriate on Windows) if running in server mode? Users can always override that with something from their storage area.

Thoughts?

​In my opinion we should not ​force users to provide certificates, we can let them decide how they want to configure it.

It's not about forcing them to provide them, it's about preventing them from using defaults which may be owned by the user that the app runs as on a server, but that should not be (unless explicitly allowed by the sysadmin) accessible to every pgAdmin user. 

Thoughts from others? Ashesh?

--

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi Dave,

Attaching updated patch, Please review.

On Mon, Sep 4, 2017 at 7:31 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Mon, Sep 4, 2017 at 2:55 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi Dave,

Can we at least commit the patch?

In future, If user complaints regarding SSL default path behaviour in server mode then adding default null file wouldn't be a big change if required.

I'm not concerned about complaints on the behaviour, I'm concerned about complaints that it's a security risk if we have multiple users inadvertently able to read a certificate and key owned by the webserver account.

​Done. 

Added logic to handle default certificates in Web mode. ​

 

Ashesh/Akshay - please read the thread and provide your feedback. Others chime in if you have anything as well please.

Thanks.

 

On Wed, Aug 30, 2017 at 2:23 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Wed, Aug 30, 2017 at 6:49 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi Dave,

PFA updated patch with new screenshots and docs accordingly.

RM

​#​

2649

​ & RM#​

2650

On Tue, Aug 29, 2017 at 7:51 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Fri, Aug 25, 2017 at 2:45 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi Dave,

Please find updated patch, 

- For displaying hidden files I have added preference option in Storage section.

How painful would it be to include it on the file dialogue as well?

​Done​

 

 

- Updated Docs & Screenshots.

- User can use 'prefer' option to enable SSL options.

Cool.

A couple of other things I realised in playing with this:

1) The SSL tab should come before Advanced I think.

​Done​

 

2) The docs now mention the default SSL files. In server mode, using defaults is probably a bad idea I suspect (because they would be shared). Should we force the values to /dev/null (and whatever is appropriate on Windows) if running in server mode? Users can always override that with something from their storage area.

Thoughts?

​In my opinion we should not ​force users to provide certificates, we can let them decide how they want to configure it.

It's not about forcing them to provide them, it's about preventing them from using defaults which may be owned by the user that the app runs as on a server, but that should not be (unless explicitly allowed by the sysadmin) accessible to every pgAdmin user. 

Thoughts from others? Ashesh?

--

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi

I've attached an updated patch, as I changed the docs a little. However:

- "Hidden?" on the File dialogue should be "Show hidden files and folders?" for clarity.

- Please remove the confirmation messagebox when the user checks the Hidden checkbox. I don't think it's necessary, as nothing will be lost.

- If I open the file dialogue, check the Hidden box, close the dialogue again and then open it again, the previous value for Hidden isn't restored. It should be remembered between dialogue invocations, like the path is.

Any chance you can fix the above by tomorrow AM?

Thanks.

On Mon, Sep 25, 2017 at 6:34 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi Dave,

Attaching updated patch, Please review.

On Mon, Sep 4, 2017 at 7:31 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Mon, Sep 4, 2017 at 2:55 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi Dave,

Can we at least commit the patch?

In future, If user complaints regarding SSL default path behaviour in server mode then adding default null file wouldn't be a big change if required.

I'm not concerned about complaints on the behaviour, I'm concerned about complaints that it's a security risk if we have multiple users inadvertently able to read a certificate and key owned by the webserver account.

​Done. 

Added logic to handle default certificates in Web mode. ​

 

Ashesh/Akshay - please read the thread and provide your feedback. Others chime in if you have anything as well please.

Thanks.

 

On Wed, Aug 30, 2017 at 2:23 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Wed, Aug 30, 2017 at 6:49 AM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi Dave,

PFA updated patch with new screenshots and docs accordingly.

RM

​#​

2649

​ & RM#​

2650

On Tue, Aug 29, 2017 at 7:51 PM, Dave Page <dpage@pgadmin.org> wrote:

Hi

On Fri, Aug 25, 2017 at 2:45 PM, Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> wrote:

Hi Dave,

Please find updated patch, 

- For displaying hidden files I have added preference option in Storage section.

How painful would it be to include it on the file dialogue as well?

​Done​

 

 

- Updated Docs & Screenshots.

- User can use 'prefer' option to enable SSL options.

Cool.

A couple of other things I realised in playing with this:

1) The SSL tab should come before Advanced I think.

​Done​

 

2) The docs now mention the default SSL files. In server mode, using defaults is probably a bad idea I suspect (because they would be shared). Should we force the values to /dev/null (and whatever is appropriate on Windows) if running in server mode? Users can always override that with something from their storage area.

Thoughts?

​In my opinion we should not ​force users to provide certificates, we can let them decide how they want to configure it.

It's not about forcing them to provide them, it's about preventing them from using defaults which may be owned by the user that the app runs as on a server, but that should not be (unless explicitly allowed by the sysadmin) accessible to every pgAdmin user. 

Thoughts from others? Ashesh?

--

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--

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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company