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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set default GSSEncMode to allow instead of prefer #1909

Conversation

nstapelbroek
Copy link

Hello there 馃憢

First off, thank you for pgjdbc. I'm a bit new to JVM and this repo makes it a lot nicer to learn how things work.

I wanted to propose a change in the default connection properties with the intent to prevent fatal errors for some default Postgres servers. As it's a very small change I figured a PR could help things more along than an issue.

The problem

I recently bumped my pgjdbc to 42.2.16 and discovered numerous fatal errors in our Postgres server logs.
The root cause of these errors traced back to the introduction of GSSAPI encrypted connection support in 42.2.15.
While I think the feature itself is probably a good addition to those using single-sign-on, the decision to stimulate it
in every connection is probably a bit too much. Hence this PR will set the default gssEncMode to allow instead of prefer.

Arguments to change the default of gssEncMode:

  • The feature was introduced in a patch version of the driver and adds new functionality that changes the default behaviour of connections.
  • As far as I understand, 50% of the currently supported Postgres versions can use this feature, which leaves the other half to produce fatal errors when enabled. (note that these fatal errors don't actually break your app due to fallback 馃憤 )
  • GSSAPI is not enabled by default on the Postgres side
  • The implementation is causing some problems on Azure, although I'm not 100% sure this PR will fix that

Reproduction

  1. Start/update a project with pgjdbc 42.2.15 or up and Postgres 10, 9.6 or 9.5
  2. Run your project and make sure you connect with default connection properties e.g. jdbc:postgresql://127.0.0.1:5432/mydb
  3. Acknowledge Postgres server logs:
FATAL: unsupported frontend protocol 1234.5680: server supports 2.0 to 3.0` when your application connects

Notes

  • My local build fails, I'll check a bit on what I'm doing wrong here

Let me know what you think 馃槃 . I'd be happy to hear your thoughts.

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes to Existing Features:

  • Does this break existing behaviour? If so please explain.
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully run tests with your changes locally?

as it requires configuration on the server side to work it's probably not a sane default for connections

see pgjdbc#1909
@nstapelbroek
Copy link
Author

Just pushed an amended commit because I caught myself assuming that this has something to do with the Azure fallback issue. Pardon me.

@codecov-commenter
Copy link

Codecov Report

Merging #1909 into master will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##             master    #1909   +/-   ##
=========================================
  Coverage     69.20%   69.21%           
  Complexity     4213     4213           
=========================================
  Files           197      197           
  Lines         18005    18004    -1     
  Branches       2919     2919           
=========================================
+ Hits          12460    12461    +1     
+ Misses         4201     4199    -2     
  Partials       1344     1344           

@davecramer
Copy link
Member

Thanks very much for your detailed PR. I address this in the next 42.2.17 release here : #1908

@davecramer
Copy link
Member

closed by #1908

@davecramer davecramer closed this Oct 6, 2020
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.

None yet

3 participants