Adding filename to master key generation Issue #419

[palak-chaturvedi]

Updated with respect to the new configuration.c changes.

• Can add -c conf-file to the master-key generation pgagroal-admin -c conf-file -g master-key
• The pgagroal_get_master_key() is also updated to access the master_key_file_location from the conf file.

@jesperpedersen @fluca1978

[jesperpedersen]

Please, use the same pull request, and force push changes instead of opening new ones...

[palak-chaturvedi]

Okk I will continue in this There were some merge conflicts in it. I was not able to pull all the new commits from the master branch. So I thought I would make it much cleaner in this one.

[fluca1978]

@palak-chaturvedi please be patient, let's go back one step: add the -m flag to both pagroal- and pgagroal-admin, so that if specified, the executables can find out the correct location of the master key file and other related stuff. Then we will decide if and how to handle it within the main configuration file.

[palak-chaturvedi]

If we add the -m flag then we will have to change lots of functions including reloading the configuration files and all the functions in configuration.c . Is this better option than conf file changing?

[jesperpedersen]

It is ok for the pgagroal.conf file, but pgagroal-admin shouldn't depend on that file

[palak-chaturvedi]

Ok so If the conf file is not given then all the functions other than those that require master-key should not throw error.

I am working on the -m flag it would change all the configuration.c functions because master key is required in those codes.

[fluca1978]

I'm wondering if this is rally worth, since it seems to be fairly complex (as of design).

However, mu opinion is that pgagroal.conf should have a setting for the master key file location, and pgagroal-admin should have a -m flag that reflects the same setting. In this way, the configuration of main should be straightforward, while the pgagroal-admin can use the same location specified on the command line. However, this could cause some confusion, since pgagroal-admin does not ask for a master key location, and will write on the default path, so the user can end up with pgagroal-admin writing in a location and pgagroal looking into another without any error or warning. Is it worth?

[jesperpedersen]

The master_key_path is in pgagroal.conf, so it will be used for pgagroal. pgagroal-cli also uses pgagroal.conf.

The only binary that you have to change are pgagroal-admin with the -m switch, and pgagroal-vault for the pgagroal-vault.conf configuration

[palak-chaturvedi]

heyy @fluca1978 I made the changes can you please check once again?

[fluca1978]

Looks fine. A few comments:

1) if I try to use a command that do not require a master key file with a wrong master key file, everything works fine. Maybe we should check for the -m usage early in pgagroal-admin:

% pgagroal-admin user ls --master-key-file /this/does/not/exists
luca

2) if pgagroal does not find the master key file, it warns with the wrong error message:

% pgagroal
pgagroal: Invalid master key file (file </etc/pgagroal/pgagroal_users.conf>)

3) I would remove master_key_file_location from the configuration and add the -m flag to the pgagroal tool itself.

[fluca1978]

@palak-chaturvedi in commit 411a0fa you fixed the first problem, good! Still issue two is there.

% pgagroal                                   
pgagroal: Invalid master key file (file </etc/pgagroal/pgagroal_users.conf>)

and this is related to issue three: I would remove the master key file from the configuration and add the -m option to main.c. Otherwise, if you believe it is better to keep such file in the configuration file, can you please argument your idea?