Update SCRAM using Postgres 15.1 #797
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit copies the original files from PostgreSQL source code. It uses minor version 15.1 (latest stable minor version). As is, it overrides the changes made by previous commits. The required changes are reintroduced in the next commit. This SCRAM update requires additional files due to upstream changes. Although some header files were mixed *.c files (such as md5_int.h), all *.h files were added to include/common and the references were updated. The reason is that Makefile does not include the current directory for header files. No changes were made to these files yet. It is done in the next commit. THIS COMMIT DOES NOT COMPILE.
After copying (over) the required files, a few changes are required to compile the code. The challenge was to apply minimal changes to make it work. system.h and postgres_compat.h does the trick plus changing some references to header files that is now in the include/common directory. Makefile included the new required files.
JelteF
reviewed
Dec 24, 2022
| scram_SaltedPassword(password, salt, saltlen, iterations, salted_password); | ||
| scram_ServerKey(salted_password, computed_key); | ||
| scram_SaltedPassword(password, salt, saltlen, iterations, salted_password, &errstr); | ||
| scram_ServerKey(salted_password, computed_key, &errstr); |
There was a problem hiding this comment.
Shouldn't we be doing something with errstr?
There was a problem hiding this comment.
Yes. I forgot to add a TODO. A few error/warning messages was added to these scram_XXX functions in Postgres and we should do the same in PgBouncer as well.
|
@eulerto Do you still want to work on this? |
|
It is on my list for some time. I will post an updated PR soon. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since SCRAM support as copied from Postgres 11.5, SCRAM infrastructure changed quite a bit. This update uses Postgres 15.1 as base. New files need to be added due to code rearrangement.
I split this work in 3 steps:
etc/import-common.sh. It adds additional files that needs to be copied for this update.Although Postgres mixes some header files (such as
sha2_int.h) with *.c files, I moved all header files to include/common and updated the references to them. The reason is thatMakefiledoes not include the current directory.After copying (over) the required files, a few changes are required to compile the code. The challenge was to apply minimal changes to make it work.
system.handpostgres_compat.hdoes the trick plus changing some header file references that is now in theinclude/commondirectory. Makefile was updated to include the new required files.TODO:
scram_XXXfunctions should useerrstrfor error handling