Refactor command execution to not use tempfile

Previously, we wrote the passphrase contents to a temporary file on
/dev/shm and then wiped it afterwards. This is odd, why don't we use a
pipe for this purpose, like it's intended to be used? Replace all of
that previous code by piped IPC.
This commit is contained in:
Johannes Bauer 2019-10-25 13:02:35 +02:00
parent 3478fa4555
commit ab670a431a
5 changed files with 85 additions and 90 deletions

View File

@ -65,7 +65,7 @@ static bool unlock_luks_volume(const struct volume_entry_t *volume, const struct
bool success = true; bool success = true;
char luks_passphrase[LUKS_PASSPHRASE_TEXT_SIZE_BYTES]; char luks_passphrase[LUKS_PASSPHRASE_TEXT_SIZE_BYTES];
if (ascii_encode(luks_passphrase, sizeof(luks_passphrase), unlock_msg->luks_passphrase_raw, sizeof(unlock_msg->luks_passphrase_raw))) { if (ascii_encode(luks_passphrase, sizeof(luks_passphrase), unlock_msg->luks_passphrase_raw, sizeof(unlock_msg->luks_passphrase_raw))) {
success = open_luks_device_pw(volume->volume_uuid, volume->devmapper_name, luks_passphrase, strlen(luks_passphrase)); success = open_luks_device(volume->volume_uuid, volume->devmapper_name, luks_passphrase, strlen(luks_passphrase));
} else { } else {
log_msg(LLVL_FATAL, "Failed to transcribe raw LUKS passphrase to text form."); log_msg(LLVL_FATAL, "Failed to transcribe raw LUKS passphrase to text form.");
success = false; success = false;

64
exec.c
View File

@ -1,6 +1,6 @@
/* /*
luksrku - Tool to remotely unlock LUKS disks using TLS. luksrku - Tool to remotely unlock LUKS disks using TLS.
Copyright (C) 2016-2016 Johannes Bauer Copyright (C) 2016-2019 Johannes Bauer
This file is part of luksrku. This file is part of luksrku.
@ -27,6 +27,7 @@
#include <sys/types.h> #include <sys/types.h>
#include <sys/wait.h> #include <sys/wait.h>
#include <string.h> #include <string.h>
#include <stdint.h>
#include <errno.h> #include <errno.h>
#include "exec.h" #include "exec.h"
@ -78,45 +79,80 @@ static char **argv_dup(const char **argv) {
return result; return result;
} }
struct runresult_t exec_command(const char **argv, bool show_output) { struct exec_result_t exec_command(const struct exec_cmd_t *command) {
struct runresult_t runresult; char **argvcopy = argv_dup(command->argv);
char **argvcopy = argv_dup(argv); if (!argvcopy) {
return (struct exec_result_t) { .success = false };
}
memset(&runresult, 0, sizeof(runresult)); int pipefd[2];
if (pipe(pipefd) == -1) {
log_libc(LLVL_ERROR, "Creation of pipe(2) failed trying to execute %s", argvcopy[0]);
argv_free(argvcopy);
return (struct exec_result_t) { .success = false };
}
const int pipe_read_end = pipefd[0];
const int pipe_write_end = pipefd[1];
pid_t pid = fork(); pid_t pid = fork();
if (pid == -1) { if (pid == -1) {
perror("fork"); perror("fork");
runresult.success = false;
argv_free(argvcopy); argv_free(argvcopy);
return runresult; return (struct exec_result_t) { .success = false };
} }
if (pid == 0) { if (pid == 0) {
/* Child */ /* Child */
if (!show_output) {
close(pipe_write_end);
if (dup2(pipe_read_end, STDIN_FILENO) == -1) {
log_libc(LLVL_ERROR, "Could not dup2(2) stdin while trying to execute %s", argvcopy[0]);
exit(EXIT_FAILURE);
}
if (!command->show_output) {
/* Shut up the child if user did not request debug output */ /* Shut up the child if user did not request debug output */
close(1); close(STDOUT_FILENO);
close(2); close(STDERR_FILENO);
} }
execvp(argvcopy[0], argvcopy); execvp(argvcopy[0], argvcopy);
log_libc(LLVL_ERROR, "Execution of %s in forked child process failed execvp(3)", argvcopy[0]); log_libc(LLVL_ERROR, "Execution of %s in forked child process failed execvp(3)", argvcopy[0]);
/* Exec failed, terminate child with EXIT_FAILUR (parent will catch /* Exec failed, terminate child with EXIT_FAILURE (parent will catch
* this as the return code) */ * this as the return code) */
exit(EXIT_FAILURE); exit(EXIT_FAILURE);
} }
/* Parent process */
struct exec_result_t runresult = {
.success = true,
};
close(pipe_read_end);
if (command->stdin_data && command->stdin_length) {
unsigned int offset = 0;
unsigned int remaining_bytes = command->stdin_length;
const uint8_t *byte_buffer = (const uint8_t*)command->stdin_data;
while (remaining_bytes) {
ssize_t written = write(pipe_write_end, byte_buffer + offset, remaining_bytes);
if (written <= 0) {
log_libc(LLVL_ERROR, "writing to pipe returned %d", written);
runresult.success = false;
}
offset += written;
remaining_bytes -= written;
}
}
close(pipe_write_end);
int status; int status;
if (waitpid(pid, &status, 0) == (pid_t)-1) { if (waitpid(pid, &status, 0) == (pid_t)-1) {
log_libc(LLVL_ERROR, "exec_command %s failed executing waitpid(2)", argvcopy[0]); log_libc(LLVL_ERROR, "exec_command %s failed executing waitpid(2)", argvcopy[0]);
runresult.success = false; runresult.success = false;
runresult.returncode = -1;
} else { } else {
runresult.success = true;
runresult.returncode = WEXITSTATUS(status); runresult.returncode = WEXITSTATUS(status);
} }
argv_free(argvcopy); argv_free(argvcopy);
log_msg(LLVL_DEBUG, "Subprocess (PID %d): %s exited with returncode %d", pid, argv[0], runresult.returncode); log_msg(LLVL_DEBUG, "Subprocess (PID %d): %s exited with returncode %d", pid, command->argv[0], runresult.returncode);
return runresult; return runresult;
} }

13
exec.h
View File

@ -1,6 +1,6 @@
/* /*
luksrku - Tool to remotely unlock LUKS disks using TLS. luksrku - Tool to remotely unlock LUKS disks using TLS.
Copyright (C) 2016-2016 Johannes Bauer Copyright (C) 2016-2019 Johannes Bauer
This file is part of luksrku. This file is part of luksrku.
@ -26,14 +26,21 @@
#include <stdbool.h> #include <stdbool.h>
struct runresult_t { struct exec_cmd_t {
const char **argv;
bool show_output;
const void *stdin_data;
unsigned int stdin_length;
};
struct exec_result_t {
bool success; bool success;
int returncode; int returncode;
}; };
/*************** AUTO GENERATED SECTION FOLLOWS ***************/ /*************** AUTO GENERATED SECTION FOLLOWS ***************/
void argv_dump(const char **argv); void argv_dump(const char **argv);
struct runresult_t exec_command(const char **argv, bool show_output); struct exec_result_t exec_command(const struct exec_cmd_t *cmd);
/*************** AUTO GENERATED SECTION ENDS ***************/ /*************** AUTO GENERATED SECTION ENDS ***************/
#endif #endif

73
luks.c
View File

@ -37,85 +37,38 @@
#include "uuid.h" #include "uuid.h"
bool is_luks_device_opened(const char *mapping_name) { bool is_luks_device_opened(const char *mapping_name) {
const char *command[] = { struct exec_cmd_t cmd = {
.argv = (const char *[]){
"dmsetup", "dmsetup",
"status", "status",
mapping_name, mapping_name,
NULL, NULL,
},
.show_output = should_log(LLVL_TRACE),
}; };
struct runresult_t runresult = exec_command(command, should_log(LLVL_TRACE)); struct exec_result_t runresult = exec_command(&cmd);
return runresult.success && (runresult.returncode == 0); return runresult.success && (runresult.returncode == 0);
} }
bool open_luks_device(const uint8_t *encrypted_device_uuid, const char *mapping_name, const char *passphrase_file) { bool open_luks_device(const uint8_t *encrypted_device_uuid, const char *mapping_name, const char *passphrase, unsigned int passphrase_length) {
char encrypted_device[64]; char encrypted_device[64];
strcpy(encrypted_device, "UUID="); strcpy(encrypted_device, "UUID=");
sprintf_uuid(encrypted_device + 5, encrypted_device_uuid); sprintf_uuid(encrypted_device + 5, encrypted_device_uuid);
log_msg(LLVL_INFO, "Trying to unlock LUKS mapping %s based on %s", mapping_name, encrypted_device); log_msg(LLVL_INFO, "Trying to unlock LUKS mapping %s based on %s", mapping_name, encrypted_device);
const char *command[] = { struct exec_cmd_t cmd = {
.argv = (const char *[]){
"cryptsetup", "cryptsetup",
"luksOpen", "luksOpen",
"-T", "1", "-T", "1",
"-d", passphrase_file,
encrypted_device, encrypted_device,
mapping_name, mapping_name,
NULL, NULL,
},
.stdin_data = passphrase,
.stdin_length = passphrase_length,
.show_output = should_log(LLVL_DEBUG),
}; };
struct runresult_t runresult = exec_command(command, should_log(LLVL_DEBUG)); struct exec_result_t runresult = exec_command(&cmd);
return runresult.success && (runresult.returncode == 0); return runresult.success && (runresult.returncode == 0);
} }
static bool wipe_passphrase_file(const char *filename, int length) {
uint8_t wipe_buf[length];
memset(wipe_buf, 0, length);
int fd = open(filename, O_WRONLY);
if (fd == -1) {
log_libc(LLVL_ERROR, "Wiping of passphrase file %s failed in open(2)", filename);
return false;
}
if (write(fd, wipe_buf, length) != length) {
log_libc(LLVL_ERROR, "Wiping of passphrase file %s failed in write(2)", filename);
close(fd);
return false;
}
close(fd);
unlink(filename);
return true;
}
static const char *write_passphrase_file(const void *passphrase, int passphrase_length) {
//const char *filename = "/dev/shm/luksrku_passphrase.bin"; /* TODO make this variable */
const char *filename = "/tmp/luksrku_passphrase.bin";
int fd = open(filename, O_CREAT | O_WRONLY | O_TRUNC, 0600);
if (fd == -1) {
log_libc(LLVL_ERROR, "Creation of passphrase file %s failed", filename);
return NULL;
}
if (write(fd, passphrase, passphrase_length) != passphrase_length) {
log_libc(LLVL_ERROR, "Writing to passphrase file %s failed", filename);
wipe_passphrase_file(filename, passphrase_length);
close(fd);
return NULL;
}
close(fd);
return filename;
}
bool open_luks_device_pw(const uint8_t *encrypted_device_uuid, const char *mapping_name, const char *passphrase, unsigned int passphrase_length) {
const char *pw_filename = write_passphrase_file(passphrase, passphrase_length);
if (!pw_filename) {
return false;
}
bool success = open_luks_device(encrypted_device_uuid, mapping_name, pw_filename);
if (!wipe_passphrase_file(pw_filename, passphrase_length)) {
log_libc(LLVL_ERROR, "Wiping of passphrase file failed -- treating this unlock as failed (luksOpen %s)", success ? "succeeded" : "also failed");
success = false;
}
return success;
}

3
luks.h
View File

@ -29,8 +29,7 @@
/*************** AUTO GENERATED SECTION FOLLOWS ***************/ /*************** AUTO GENERATED SECTION FOLLOWS ***************/
bool is_luks_device_opened(const char *mapping_name); bool is_luks_device_opened(const char *mapping_name);
bool open_luks_device(const uint8_t *encrypted_device_uuid, const char *mapping_name, const char *passphrase_file); bool open_luks_device(const uint8_t *encrypted_device_uuid, const char *mapping_name, const char *passphrase, unsigned int passphrase_length);
bool open_luks_device_pw(const uint8_t *encrypted_device_uuid, const char *mapping_name, const char *passphrase, unsigned int passphrase_length);
/*************** AUTO GENERATED SECTION ENDS ***************/ /*************** AUTO GENERATED SECTION ENDS ***************/
#endif #endif