will / crystal-pg

a postgres driver for crystal
BSD 3-Clause "New" or "Revised" License
462 stars 77 forks source link

Cleartext auth is not supported #192

Open marynowac opened 4 years ago

marynowac commented 4 years ago

I came across a problem with the database connection. A standard piece of code:

DB.open "postgres://myuser:mypasswd@remoteserver/mydb" do |db|
  # code
end

throws an exception: "(DB::ConnectionRefused), caused by: Cleartext auth is not supported (Exception)"

I can connect that db using a pgAdmin4 or simple php script:

$db = new PDO("pgsql:dbname=mydb host=remoteserver", "myuser", "mypasswd");

and I have no influence on the database configuration.

What should I add to a connection string to avoid that error?

straight-shoota commented 4 years ago

To summarize from Gitter:

Crystal PG doesn't support cleartext auth. It was removed in cafe2a2c90a22595abe1312d330567cd85e683b2 resolving #46.

However, that entirely depends on the circumstances. For example, when the connection is encrypted and validated through SSL, it's completely safe to transmit plain text credentials. Or when the connection is in a private network, there is no harm either.

So, while generally agreeing on the principle to avoid dangerous mechanisms, there might be valid use cases that legitimize support for cleartext authentication. AFAIK most PG clients support cleartext, so it's not that forbidding it would be a standard practice.

I'd suggest to bring cleartext authentication back. Maybe it could be enabled only with opt-in parameter cleartext=yes. This would make it possible to use crystal-pg for use cases like @marynowac where the server configuration is not accessible.

will commented 4 years ago

The problem is that simply supporting it allows for a dowgrade attack. Even if it's over SSL, unless you're doing verify-full, you can't be sure that there isn't a MITM, who is lying and saying that postgres server you're talking to only supports clear text passwords. Allowing it for one person endangers everyone. The other clients also should not be doing this, but have to continue for legacy reasons.

I think the only safe way to allow people to do this, is move the error out to some method, and let people monkey patch the driver themsleves to add support. At least that way hopefully they are aware how dangerous it is.

straight-shoota commented 4 years ago

@will What speaks against enabling this using runtime configuration? I don't think the developer of a crystal-pg-based app should decide whether cleartext auth is supported, but the actual user who declares where to connect to.

will commented 4 years ago

If it's a setting somewhere that is very explicit and everytime it's invoked it's clear that it's crazy dangerous, like Whatever.enable_cleartext_passwords_that_merely_turning_this_setting_on_can_compromise_your_password_even_if_you_dont_actually_use_a_cleartext_password = true

will commented 4 years ago

If it's information in some readme or documentation, no one will read it. It has to be incredibly explicit what you're opting in to by doing this.

straight-shoota commented 4 years ago

I'm saying that it shouldn't be determined at compile time, but be configurable at runtime. So you can have a binary that is not inherently unsafe, but allows using cleartext authentication, for example if you provide cleartext_auth=yes param in the connection URI.

jhass commented 4 years ago

Wild idea... can we tag a specific password to be allowed for cleartext auth and refuse to use passwords without the tag for it? something like making the password {allowCleartext}myActualNotSoSecretPassword

straight-shoota commented 4 years ago

@jhass What benefit would that provide over a URL parameter? It seems to only complicate things.

jhass commented 4 years ago

It pretty much forces it to a user choice. I can easily imagine an application only exposing a host user and password parameter, but not one to give the full URL. Then the application might always or never add the cleartext parameter when constructing the URL.

andy-twosticks commented 2 years ago

Am I right in thinking that no-one has answered the OPs original question?

[...] I have no influence on the database configuration. What should I add to a connection string to avoid that error?

I'm in a similar situation and I can't find any documentation on how to configure the URL string at all. Neither can I find an answer to that question.

If the answer is, "with that database configuration, you cannot connect to the database using crystal-pg" -- then, okay. But can someone confirm that?

straight-shoota commented 2 years ago

The ability to use cleartext auth has been added in https://github.com/will/crystal-pg/pull/220 So this issue can really be closed. There were multiple iterations of PRs and it wasn't clearly referenced to this issues.

andy-twosticks commented 2 years ago

@straight-shoota You can certainly close the issue without answering the OPs question, which I would like to know the answer to also. You can close any issue at any time for any reason. But that's ... not terribly helpful?

How do I turn on cleartext auth? Also, given that this is a dreadful idea, how do I arrange to connect to the database if I don't want to do that?

straight-shoota commented 2 years ago

I would assume the linked PR description should answer that question: You have to add cleartext to the enabled auth methods in code and then it's valid to use in a connection string. More extensive documentation is probably still missing.

will commented 2 years ago

Yes, it is now possible to optionally turn on plain text auth. It's off by default.

@andy-twosticks the way to do it is in the issue @straight-shoota linked. You need to manually set the auth_methods setting to include cleartext, such as auth_methods=cleartext or DB.open("postgres://crystal_scram:pass@127.0.0.1?auth_methods=cleartext,md5")

Thanks for bumping this issue also, A few months ago I wrote a tool that lets you steal the password from any driver that supports plaintext by default (including the main libpq), but kept it private until I got word from the postgres security list that this is in fact a known issue. But then when I got an answer that it'd be okay to publish, I was busy and forgot to do so until this bump. edit: and note, any person who uses this crystal driver with cleartext in their auth_methods setting is vulnerable to this attack, which is why it's not enabled by default.

andy-twosticks commented 2 years ago

@straight-shoota @will I don't see anything in that link that explains what to do. Am I supposed to go looking through all the code of the pull request? It's not clear.

Again: what other options do I have to make a connection without turning this on? I'm sure it's obvious to you. It's not to me, or to the OP.

will commented 2 years ago

Add ?auth_methods=cleartext to the end of your connection url. It'a also in the readme https://github.com/will/crystal-pg#authentication-methods