Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backends/sqla: allow pgsql conns via unix sockets #1721

Conversation

dev-zero
Copy link
Contributor

@dev-zero dev-zero commented Jul 6, 2018

SQLA/psycopg2 allows connection via unix sockets instead of TCP/IP.
This allows for secure password-less authentication via PostgreSQL's
socket peer authentication, which is the default on many distros.

Example pg_hba.conf:

local   all             all                                     peer

would allow a user test access to the PostgreSQL cluster if a user
test exists in PostgreSQL without any password (implicitly assuming
that it is sufficient that the user was already authenticated by the
OS).
Using pg_ident.conf one can also map local users to PostgreSQL users
with a different name:

Example pg_hba.conf:

local   aiida           aiida                                   peer map=aiida

Example pg_ident.conf:

aiida           test                 aiida

Would allow the user aiida access to the database aiida and the map
allows the system user test to impersonate the database user aiida.

psycopg2 automatically tries the local socket connection if no port is
specified, but for that must the connection string not contain the
colon char otherwise required for the host:port separation.

@dev-zero dev-zero force-pushed the feature/allow-pgsql-socket-connection branch from c7043d3 to 6308fbf Compare July 9, 2018 07:50
@codecov-io
Copy link

codecov-io commented Jul 9, 2018

Codecov Report

Merging #1721 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1721   +/-   ##
========================================
  Coverage    57.16%   57.16%           
========================================
  Files          275      275           
  Lines        33912    33912           
========================================
  Hits         19386    19386           
  Misses       14526    14526
Impacted Files Coverage Δ
aiida/backends/sqlalchemy/utils.py 82.5% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2132342...d16008e. Read the comment docs.

@DropD
Copy link
Contributor

DropD commented Jul 10, 2018

@szoupanos Have you checked this out?

@szoupanos
Copy link
Contributor

Thanks a lot!

A few questions/comments:
a) What is the benefit of using this? Simplicity or also speed? From a small search on the internet I see that there are not guaranteed speed benefits from the use of unix sockets vs tcp sockets
b) I would be against creating a new non-universal procedure of installing AiiDA if this implies that the user has to change the configuration files of postgresql (something that he is not obliged to do today, if I am not mistaken).
c) Your approach will work correctly with the default PostgreSQL configuration
local all all peer
if there is a postgresql user with the same name as the local *nix user and it also has the right to create databases etc. Right? Is it possible that the user may not have the right to create databases etc?

So, if there are no speed benefits, and what I write at (c) is correct (and the default behaviour & the *nix user can create databases) in many systems, it makes sense to do this change. Otherwise, it seems to me that it increases the installation complexity without any big benefits.

Do I miss something?

@dev-zero
Copy link
Contributor Author

@szoupanos

a) there is no speed gain, only advantages in simplicity and security: postgresql doesn't have to listen on a tcp port which could accidentally be exposed and one does not have to manage yet another (usually weak) password (which could be used by another user on the same system to gain access to the database)
b) the default installation procedure can be left unchanged and continues to work. Actually, the only thing the patch here changes is that if the port is left empty in verdi setup, it will pass along a correct connection string to SQLA/psycopg2, while currently an exception is being thrown.
c) correct. When setting up a PostgreSQL user, the admin can decide whether that user has the right to create databases. On my machine, the following was sufficient to create a usable database for AiiDA:

sudo su - postgres
createuser tiziano
createdb -O tiziano aiida_tiziano

SQLA/psycopg2 allows connection via unix sockets instead of TCP/IP.
This allows for secure password-less authentication via PostgreSQL's
socket peer authentication, which is the default on many distros.

Example `pg_hba.conf`:

    local   all             all                                     peer

would allow a user `test` access to the PostgreSQL cluster if a user
`test` exists in PostgreSQL without any password (implicitly assuming
that it is sufficient that the user was already authenticated by the
OS).
Using `pg_ident.conf` one can also map local users to PostgreSQL users
with a different name:

Example `pg_hba.conf`:

    local   aiida           aiida                                   peer map=aiida

Example `pg_ident.conf`:

    aiida           test                 aiida

Would allow the user `aiida` access to the database `aiida` and the map
allows the system user `test` to impersonate the database user `aiida`.

psycopg2 automatically tries the local socket connection if no port is
specified, but for that must the connection string not contain the
colon char otherwise required for the host:port separation.
@dev-zero dev-zero force-pushed the feature/allow-pgsql-socket-connection branch from 6308fbf to d16008e Compare July 10, 2018 13:23
Copy link
Member

@giovannipizzi giovannipizzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any disadvantage with this patch! I'm approving it.
@dev-zero maybe you can move the content of the PR to an "advanced" section in the docs, explaining how to setup AiiDA with this approach? (and if needed simplifying the current docs)

@giovannipizzi giovannipizzi merged commit 80e0ca2 into aiidateam:develop Jul 10, 2018
@dev-zero dev-zero deleted the feature/allow-pgsql-socket-connection branch August 31, 2018 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants