Skip to content

[#461] fix ssl connection to postgres in server_passthrough#488

Closed
ashu3103 wants to merge 1 commit intopgagroal:masterfrom
ashu3103:fix-ssl
Closed

[#461] fix ssl connection to postgres in server_passthrough#488
ashu3103 wants to merge 1 commit intopgagroal:masterfrom
ashu3103:fix-ssl

Conversation

@ashu3103
Copy link
Collaborator

@ashu3103 ashu3103 commented Dec 3, 2024

Refer #462

@ashu3103 ashu3103 self-assigned this Dec 7, 2024
@jesperpedersen jesperpedersen added the enhancement Improvement to an existing feature label Jan 10, 2025
@jesperpedersen
Copy link
Collaborator

Ideally I would like to see the information inside

doc/tutorial/06_tls.md
doc/manual/advanced/08-tls.md

if you can find some way to "combine" it. Or we need it after the TLS chapter - therefore moving the vault chapter

@jesperpedersen
Copy link
Collaborator

@Userfrom1995 Can you take this one ?

@Userfrom1995
Copy link
Collaborator

@Userfrom1995 Can you take this one ?
Sure, I can take this one.

@jesperpedersen
Copy link
Collaborator

@Userfrom1995 Thanks, work with @ashu3103 to get it done

@ashu3103
Copy link
Collaborator Author

ashu3103 commented May 12, 2025

@Userfrom1995 hey,

In this PR one of our primary aim was to enable or more precisely fix the scram-sha-256 capabilities (can extend it to scram-sha-256-plus later on). The main issue I was facing is that while parsing AuthenticationSASL message from the server side, it includes the scram-sha-256-plus in the list which is not supported by pgagroal currently which was giving channel bonding errors at the client side. I will draft a more formal description of the issue and how I tried encountering it later (still finding my documented work 😓).

Till then you can go through security.c file and the SASL Authentication and the corresponding message exchanges. Also you can follow the tutorial/06_tls.md tutorial in the changed files (remember to replace md5 with scram-sha-256 in the pg_hba file) you will see the issue.

Edit: You can add this work and probably scram-sha-256-plus support as a part of your Security Enhancement milestones too.

Thanks

@jesperpedersen
Copy link
Collaborator

@Userfrom1995 Don't worry about the SCRAM-SHA-XYZ-PLUS and associated UTF-8 work right now (https://datatracker.ietf.org/doc/html/rfc7677 - #52, #51)

@Userfrom1995
Copy link
Collaborator

Hey @ashu3103 ,

Thanks for the heads-up and the detailed context — really helpful! I’ll go through security.c , try to reproduce the error, and see how best to address it.

Regarding the idea of adding support for scram-sha-256-plus, I agree — it’s a great addition and fits well within my security enhancement milestones. I’ll make sure to include it in the plan.

Appreciate your input!

@jesperpedersen
Copy link
Collaborator

@ashu3103 Can you rebase on master ?
@Userfrom1995 Can you give this a quick scan based on master ?

@jesperpedersen
Copy link
Collaborator

@ashu3103 cough

@ashu3103
Copy link
Collaborator Author

@ashu3103 cough

Hi @jesperpedersen, actually I am away from my workstation for a while. I'll ping you once I'll be back again

@jesperpedersen
Copy link
Collaborator

@fluca1978 Can you take a quick check ?

@fluca1978
Copy link
Collaborator

I've locally rebased it against master and run some tests, it works fine on my side.
@jesperpedersen would you like to merge this or do you think we need more tests?

@jesperpedersen
Copy link
Collaborator

@fluca1978 We can merge

@fluca1978
Copy link
Collaborator

Closing this since I merged.

Thanks @ashu3103 for your work.
Thanks @Userfrom1995 for your work too.

@fluca1978 fluca1978 closed this Jan 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvement to an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants