agroal / pgagroal

High-performance connection pool for PostgreSQL
https://agroal.github.io/pgagroal/
BSD 3-Clause "New" or "Revised" License
667 stars 59 forks source link

Issue #419 Allow pgagroal-admin to specify where to store the master key password file #430

Closed palak-chaturvedi closed 3 months ago

palak-chaturvedi commented 3 months ago

Added the filename attribute to master-key generation. To tackle the multiple uses of file in various location, stored the location of the file in a text file.

fluca1978 commented 3 months ago

First of all, it seems you need to rebase your code. Second, I don't like the idea of having master_key.txt to hold the path of the master key file location. Moreover, the master key file is used for other operations, so we need to be able to pass to other commands. One possible solution could be to use a command line flag pgagroal-admin -m /path/to/blah/foo/master.key master-key so that it is also possible then pgagroal-admin -m /path/to/blah/foo/master.key used add. This would be only a partial solution, in any case, since the master key file is used also in other source files, so the flag has to be then added to (at least I think), main.c. Another idea, could be to allow for a tunable in pgagroal.conf that indicates a possible alternative directory for the master key file, so that the user is not asked to type the location but the application knows where to find the master key outside of ~/.pgagroal.

palak-chaturvedi commented 3 months ago

The master.key filed is called in only two files. I changed the "master_key" function as well as the "pgagroal_get_master_key" function in security.c. I can add arguments but I thought if the user does not write the key in arguments but it has been created in a location other than default then if the location is not stored somewhere the function would throw error. I also thought txt file is not the best option (adding it to conf file is an option but the conf file cannot be edited while creating the master_key to store the location. The location would be predefined by the user). Instead of it arguments options is much better I think. What are your views on this? @fluca1978

fluca1978 commented 3 months ago

Sorry, I don't like the idea of having a text file storing a "pointer" to the master key file. I think we should either add a command line flag (e.g., -m) to all the executables that can use the master key file or add a tunable in the configuration file pgagroal.conf. Clearly, the latter has to be in place before pgagroal-admin master-key is run, in other words, it is not pgagroal-admin supposed to change the configuration by itself.