Skip to content

Commit

Permalink
Adding filename to master key generation Issue #419
Browse files Browse the repository at this point in the history 
Remove remote connection part from the admin.c

Added -m flag
  • Loading branch information
@palak-chaturvedi
palak-chaturvedi committed Apr 16, 2024
1 parent 5dd618b commit 6eeff18
Show file tree
Hide file tree
Showing 5 changed files with 165 additions and 110 deletions.
136 changes: 85 additions & 51 deletions src/admin.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
#include <logging.h>
#include <security.h>
#include <utils.h>
#include <shmem.h>
#include <configuration.h>

/* system */
#include <ctype.h>
Expand All @@ -52,9 +54,9 @@
#define ACTION_REMOVE_USER 4
#define ACTION_LIST_USERS 5

static int master_key(char* password, bool generate_pwd, int pwd_length);
static int add_user(char* users_path, char* username, char* password, bool generate_pwd, int pwd_length);
static int update_user(char* users_path, char* username, char* password, bool generate_pwd, int pwd_length);
static int master_key(char* password, bool generate_pwd, int pwd_length, char* filename);
static int add_user(char* users_path, char* username, char* password, bool generate_pwd, int pwd_length, char* filename);
static int update_user(char* users_path, char* username, char* password, bool generate_pwd, int pwd_length, char* filename);
static int remove_user(char* users_path, char* username);
static int list_users(char* users_path);

Expand Down Expand Up @@ -164,6 +166,7 @@ usage(void)
printf(" pgagroal-admin [ -f FILE ] [ COMMAND ] \n");
printf("\n");
printf("Options:\n");
printf(" -m, --master-key-file MASTER_KEY_FILE Set the path to the master.key file\n");

This comment has been minimized.

Sign in to view

Copy link
@fluca1978

fluca1978 Apr 16, 2024

Collaborator

Description could be something like "Sets the full path to the master key file", and on a new line "Defaults to $home/%s" with PGAGROAL_DEFAULT_MASTER_KEY_FILE_NAME = master.key`.
printf(" -f, --file FILE Set the path to a user file\n");
printf(" Defaults to %s\n", PGAGROAL_DEFAULT_USERS_FILE);
printf(" -U, --user USER Set the user name\n");
Expand Down Expand Up @@ -197,11 +200,13 @@ main(int argc, char** argv)
int option_index = 0;
size_t command_count = sizeof(command_table) / sizeof(struct pgagroal_command);
struct pgagroal_parsed_command parsed = {.cmd = NULL, .args = {0}};
char* master_key_path = NULL;

while (1)
{
static struct option long_options[] =
{
{"master-key-file", required_argument, 0, 'm'},

This comment has been minimized.

Sign in to view

Copy link
@fluca1978

fluca1978 Apr 16, 2024

Collaborator

I think the argument should be optional, if not specified, got with NULL, i.e., $HOME.
{"user", required_argument, 0, 'U'},
{"password", required_argument, 0, 'P'},
{"file", required_argument, 0, 'f'},
Expand All @@ -211,7 +216,7 @@ main(int argc, char** argv)
{"help", no_argument, 0, '?'}
};

c = getopt_long(argc, argv, "gV?f:U:P:l:",
c = getopt_long(argc, argv, "gV?f:m:U:P:l:",
long_options, &option_index);

if (c == -1)
Expand All @@ -221,6 +226,9 @@ main(int argc, char** argv)

switch (c)
{
case 'm':
master_key_path = optarg;
break;
case 'U':
username = optarg;
break;
Expand Down Expand Up @@ -253,6 +261,12 @@ main(int argc, char** argv)
errx(1, "Using the root account is not allowed");
}

if (argc <= 1)
{
usage();
exit(1);
}

if (!parse_command(argc, argv, optind, &parsed, command_table, command_count))
{
usage();
Expand All @@ -273,21 +287,21 @@ main(int argc, char** argv)

if (parsed.cmd->action == ACTION_MASTER_KEY)
{
if (master_key(password, generate_pwd, pwd_length))
if (master_key(password, generate_pwd, pwd_length, master_key_path))
{
errx(1, "Cannot generate master key");
}
}
else if (parsed.cmd->action == ACTION_ADD_USER)
{
if (add_user(file_path, username, password, generate_pwd, pwd_length))
if (add_user(file_path, username, password, generate_pwd, pwd_length, master_key_path))
{
errx(1, "Error for <user add>");
}
}
else if (parsed.cmd->action == ACTION_UPDATE_USER)
{
if (update_user(file_path, username, password, generate_pwd, pwd_length))
if (update_user(file_path, username, password, generate_pwd, pwd_length, master_key_path))
{
errx(1, "Error for <user edit>");
}
Expand Down Expand Up @@ -318,77 +332,90 @@ main(int argc, char** argv)
}

static int
master_key(char* password, bool generate_pwd, int pwd_length)
master_key(char* password, bool generate_pwd, int pwd_length, char* filename)
{
FILE* file = NULL;
char buf[MISC_LENGTH];
char* encoded = NULL;
struct stat st = {0};
bool do_free = true;

if (pgagroal_get_home_directory() == NULL)
if (filename == NULL)
{
char* username = pgagroal_get_user_name();
if (pgagroal_get_home_directory() == NULL)
{
char* username = pgagroal_get_user_name();

if (username != NULL)
{
warnx("No home directory for user \'%s\'", username);
}
else
{
warnx("No home directory for user running pgagroal");
}

goto error;
}

if (username != NULL)
memset(&buf, 0, sizeof(buf));

This comment has been minimized.

Sign in to view

Copy link
@fluca1978

fluca1978 Apr 16, 2024

Collaborator

uncrustify
snprintf(&buf[0], sizeof(buf), "%s/.pgagroal", pgagroal_get_home_directory());

if (stat(&buf[0], &st) == -1)
{
warnx("No home directory for user \'%s\'", username);
mkdir(&buf[0], S_IRWXU);
}
else
{
warnx("No home directory for user running pgagroal");
if (S_ISDIR(st.st_mode) && st.st_mode & S_IRWXU && !(st.st_mode & S_IRWXG) && !(st.st_mode & S_IRWXO))
{
/* Ok */
}
else
{
warnx("Wrong permissions for directory <%s> (must be 0700)", &buf[0]);
goto error;
}
}

goto error;
}
memset(&buf, 0, sizeof(buf));
snprintf(&buf[0], sizeof(buf), "%s/.pgagroal/master.key", pgagroal_get_home_directory());

memset(&buf, 0, sizeof(buf));
snprintf(&buf[0], sizeof(buf), "%s/.pgagroal", pgagroal_get_home_directory());
if (pgagroal_exists(&buf[0]))
{
warnx("The file %s already exists, cannot continue", &buf[0]);

This comment has been minimized.

Sign in to view

Copy link
@fluca1978

fluca1978 Apr 16, 2024

Collaborator

Should it be "The master key file %s already exists..."?
goto error;
}

if (stat(&buf[0], &st) == -1)
{
mkdir(&buf[0], S_IRWXU);
}
else
{
if (S_ISDIR(st.st_mode) && st.st_mode & S_IRWXU && !(st.st_mode & S_IRWXG) && !(st.st_mode & S_IRWXO))
if (stat(&buf[0], &st) == -1)
{
/* Ok */
}
else
{
warnx("Wrong permissions for directory <%s> (must be 0700)", &buf[0]);
goto error;
if (S_ISREG(st.st_mode) && st.st_mode & (S_IRUSR | S_IWUSR) && !(st.st_mode & S_IRWXG) && !(st.st_mode & S_IRWXO))
{
/* Ok */
}
else
{
warnx("Wrong permissions for file <%s> (must be 0600)", &buf[0]);

This comment has been minimized.

Sign in to view

Copy link
@fluca1978

fluca1978 Apr 16, 2024

Collaborator

Since you are working on this, why not making it more coherent?
Use angle brackets around the file name also in the previous warning message (the file already exists); use the "master key" description, so "Wrong permissions for master key file <%s>..."
goto error;
}
}
}

memset(&buf, 0, sizeof(buf));
snprintf(&buf[0], sizeof(buf), "%s/.pgagroal/master.key", pgagroal_get_home_directory());

if (pgagroal_exists(&buf[0]))
{
warnx("The file %s already exists, cannot continue", &buf[0]);
goto error;
file = fopen(&buf[0], "w+");
}

if (stat(&buf[0], &st) == -1)
{
/* Ok */
}
else
{
if (S_ISREG(st.st_mode) && st.st_mode & (S_IRUSR | S_IWUSR) && !(st.st_mode & S_IRWXG) && !(st.st_mode & S_IRWXO))
{
/* Ok */
}
else
if (pgagroal_exists(filename))
{
warnx("Wrong permissions for file <%s> (must be 0600)", &buf[0]);
warnx("The file %s already exists, cannot continue", filename);

This comment has been minimized.

Sign in to view

Copy link
@fluca1978

fluca1978 Apr 16, 2024

Collaborator

As above, "The master key file <%s>..."
goto error;
}
file = fopen(filename, "w+");
}

file = fopen(&buf[0], "w+");
if (file == NULL)
{
warnx("Could not write to master key file <%s>", &buf[0]);
Expand Down Expand Up @@ -439,7 +466,14 @@ master_key(char* password, bool generate_pwd, int pwd_length)
fclose(file);

chmod(&buf[0], S_IRUSR | S_IWUSR);
printf("Master Key stored into %s\n", &buf[0]);
if (filename == NULL)
{
printf("Master Key stored into %s\n", &buf[0]);
}
else
{
printf("Master Key stored into %s\n", filename);
}
return 0;

error:
Expand All @@ -460,7 +494,7 @@ master_key(char* password, bool generate_pwd, int pwd_length)
}

static int
add_user(char* users_path, char* username, char* password, bool generate_pwd, int pwd_length)
add_user(char* users_path, char* username, char* password, bool generate_pwd, int pwd_length, char* filename)
{
FILE* users_file = NULL;
char line[MISC_LENGTH];
Expand All @@ -476,7 +510,7 @@ add_user(char* users_path, char* username, char* password, bool generate_pwd, in
char* verify = NULL;
bool do_free = true;

if (pgagroal_get_master_key(&master_key))
if (pgagroal_get_master_key(&master_key, filename))
{
warnx("Invalid master key");
goto error;
Expand Down Expand Up @@ -635,7 +669,7 @@ add_user(char* users_path, char* username, char* password, bool generate_pwd, in
}

static int
update_user(char* users_path, char* username, char* password, bool generate_pwd, int pwd_length)
update_user(char* users_path, char* username, char* password, bool generate_pwd, int pwd_length, char* filename)
{
FILE* users_file = NULL;
FILE* users_file_tmp = NULL;
Expand All @@ -656,7 +690,7 @@ update_user(char* users_path, char* username, char* password, bool generate_pwd,

memset(&tmpfilename, 0, sizeof(tmpfilename));

if (pgagroal_get_master_key(&master_key))
if (pgagroal_get_master_key(&master_key, filename))
{
warnx("Invalid master key");
goto error;
Expand Down
43 changes: 22 additions & 21 deletions src/include/pgagroal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,14 @@ extern "C" {
#define unlikely(x) __builtin_expect (!!(x), 0)

#define MAX(a, b) \
({ __typeof__ (a) _a = (a); \
__typeof__ (b) _b = (b); \
_a > _b ? _a : _b; })
({ __typeof__ (a) _a = (a); \
__typeof__ (b) _b = (b); \
_a > _b ? _a : _b; })

#define MIN(a, b) \
({ __typeof__ (a) _a = (a); \
__typeof__ (b) _b = (b); \
_a < _b ? _a : _b; })
({ __typeof__ (a) _a = (a); \
__typeof__ (b) _b = (b); \
_a < _b ? _a : _b; })

/*
* Common piece of code to perform a sleeping.
Expand All @@ -199,13 +199,13 @@ extern "C" {
*
*/
#define SLEEP(zzz) \
do \
{ \
struct timespec ts_private; \
ts_private.tv_sec = 0; \
ts_private.tv_nsec = zzz; \
nanosleep(&ts_private, NULL); \
} while (0);
do \
{ \
struct timespec ts_private; \
ts_private.tv_sec = 0; \
ts_private.tv_nsec = zzz; \
nanosleep(&ts_private, NULL); \
} while (0);

/*
* Commonly used block of code to sleep
Expand All @@ -222,14 +222,14 @@ do \
SLEEP_AND_GOTO(100000L, retry)
*/
#define SLEEP_AND_GOTO(zzz, goto_to) \
do \
{ \
struct timespec ts_private; \
ts_private.tv_sec = 0; \
ts_private.tv_nsec = zzz; \
nanosleep(&ts_private, NULL); \
goto goto_to; \
} while (0);
do \
{ \
struct timespec ts_private; \
ts_private.tv_sec = 0; \
ts_private.tv_nsec = zzz; \
nanosleep(&ts_private, NULL); \
goto goto_to; \
} while (0);

/**
* The shared memory segment
Expand Down Expand Up @@ -514,6 +514,7 @@ struct main_configuration
bool track_prepared_statements; /**< Track prepared statements (transaction pooling) */

char unix_socket_dir[MISC_LENGTH]; /**< The directory for the Unix Domain Socket */
char master_key_file_location[MISC_LENGTH]; /**< The directory for the MasterKey */

atomic_schar su_connection; /**< The superuser connection */

Expand Down
2 changes: 1 addition & 1 deletion src/include/security.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ pgagroal_remote_management_scram_sha256(char* username, char* password, int serv
* @return 0 upon success, otherwise 1
*/
int
pgagroal_get_master_key(char** masterkey);
pgagroal_get_master_key(char** masterkey, char* filename);

/**
* Encrypt a string
Expand Down

0 comments on commit 6eeff18

Please sign in to comment.