From c2e524dbc7dfabea85e4a52df385d8041eac444f Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Tue, 11 Jun 2024 09:24:08 -0400 Subject: [PATCH] swtpm: Use fchmod to set mode bits provided by user The mode bits that the user provided were only applied with open() and were subject to masking with the value of current umask. When umask was set to 0027 the test case test_commandline was failing because the mode bits on the create TPM state file were not the expected ones (masked by umask). Therefore, set the mode bits using fchmod if the user provided them, otherwise do not set them. This way the mode bits will be set to the values the user requested. Currently the directory storage backend was setting the mode bits to the default value (0640) *after* opening the TPM state file. Now, if the user did not provide any mode bits then the mode bits will be set so that the file can be written to as owner. This ensures that at least mode bits 0600 are set by default. However, if the user provided mode bit flags then these will be used without modification. Signed-off-by: Stefan Berger --- src/swtpm/common.c | 17 ++++++++--- src/swtpm/swtpm_nvstore_dir.c | 44 +++++++++++++++++++++------ src/swtpm/swtpm_nvstore_linear_file.c | 12 +++++++- src/swtpm/tpmstate.c | 8 +++-- src/swtpm/tpmstate.h | 5 +-- 5 files changed, 66 insertions(+), 20 deletions(-) diff --git a/src/swtpm/common.c b/src/swtpm/common.c index b15f900..03961f3 100644 --- a/src/swtpm/common.c +++ b/src/swtpm/common.c @@ -633,13 +633,14 @@ handle_pid_options(char *options) * @options: the 'pid' options to parse * @tpmstatedir: Point to pointer for tpmstatedir * @mode: the mode of the TPM's state files + * @mode_is_default: true if user did not provide mode bits but using default * @tpmbackend_uri: Point to pointer for backend URI * * Returns 0 on success, -1 on failure. */ static int parse_tpmstate_options(char *options, char **tpmstatedir, mode_t *mode, - char **tpmbackend_uri) + bool *mode_is_default, char **tpmbackend_uri) { OptionValues *ovs = NULL; char *error = NULL; @@ -654,9 +655,14 @@ parse_tpmstate_options(char *options, char **tpmstatedir, mode_t *mode, } directory = option_get_string(ovs, "dir", NULL); - *mode = option_get_mode_t(ovs, "mode", 0640); backend_uri = option_get_string(ovs, "backend-uri", NULL); + /* Did user provide mode bits? User can only provide <= 0777 */ + *mode = option_get_mode_t(ovs, "mode", 01000); + *mode_is_default = (*mode == 01000); + if (*mode_is_default) + *mode = 0640; + if (directory) { *tpmstatedir = strdup(directory); if (!*tpmstatedir) { @@ -703,12 +709,13 @@ handle_tpmstate_options(char *options) char *temp_uri = NULL; int ret = 0; mode_t mode; + bool mode_is_default = true; if (!options) return 0; if (parse_tpmstate_options(options, &tpmstatedir, &mode, - &tpmbackend_uri) < 0) { + &mode_is_default, &tpmbackend_uri) < 0) { ret = -1; goto error; } @@ -724,7 +731,7 @@ handle_tpmstate_options(char *options) } if (tpmstate_set_backend_uri(temp_uri) < 0 || - tpmstate_set_mode(mode) < 0) { + tpmstate_set_mode(mode, mode_is_default) < 0) { ret = -1; goto error; } @@ -735,7 +742,7 @@ handle_tpmstate_options(char *options) } if ((strncmp(tpmbackend_uri, "dir://", 6) == 0 || strncmp(tpmbackend_uri, "file://", 7)) && - tpmstate_set_mode(mode) < 0) { + tpmstate_set_mode(mode, mode_is_default) < 0) { ret = -1; goto error; } diff --git a/src/swtpm/swtpm_nvstore_dir.c b/src/swtpm/swtpm_nvstore_dir.c index ae1a1d6..513d01f 100644 --- a/src/swtpm/swtpm_nvstore_dir.c +++ b/src/swtpm/swtpm_nvstore_dir.c @@ -257,6 +257,9 @@ SWTPM_NVRAM_LoadData_Dir(unsigned char **data, char filepath[FILENAME_MAX]; /* rooted file path from name */ struct stat statbuf; const char *tpm_state_path = NULL; + bool mode_is_default = false; + bool do_chmod; + mode_t mode; tpm_state_path = SWTPM_NVRAM_Uri_to_Dir(uri); @@ -286,15 +289,6 @@ SWTPM_NVRAM_LoadData_Dir(unsigned char **data, } } - if (rc == 0) { - if (fchmod(fd, tpmstate_get_mode()) < 0) { - logprintf(STDERR_FILENO, - "SWTPM_NVRAM_LoadData: Could not fchmod %s : %s\n", - filepath, strerror(errno)); - rc = TPM_FAIL; - } - } - /* determine the file length */ if (rc == 0) { irc = fstat(fd, &statbuf); @@ -329,6 +323,26 @@ SWTPM_NVRAM_LoadData_Dir(unsigned char **data, rc = TPM_FAIL; } } + + if (rc == 0) { + mode = tpmstate_get_mode(&mode_is_default); + /* + * Make sure the file can be written to when state needs to be written. + * Do not touch user-provided flags. + */ + do_chmod = !mode_is_default; + if (mode_is_default && (statbuf.st_mode & 0200) == 0) { + mode |= 0200; + do_chmod = true; + } + if (do_chmod && fchmod(fd, mode) < 0) { + logprintf(STDERR_FILENO, + "SWTPM_NVRAM_LoadData: Could not fchmod %s : %s\n", + filepath, strerror(errno)); + rc = TPM_FAIL; + } + } + /* close the file */ if (fd >= 0) { TPM_DEBUG(" SWTPM_NVRAM_LoadData: Closing file %s\n", filepath); @@ -362,6 +376,7 @@ SWTPM_NVRAM_StoreData_Dir(unsigned char *filedata, char tmpfile[FILENAME_MAX]; /* rooted temporary file path */ char filepath[FILENAME_MAX]; /* rooted file path from name */ const char *tpm_state_path = NULL; + bool mode_is_default = true; #if 0 static bool do_dir_fsync = true; /* turn off fsync on dir if it fails, @@ -390,7 +405,7 @@ SWTPM_NVRAM_StoreData_Dir(unsigned char *filedata, /* open the file */ TPM_DEBUG(" SWTPM_NVRAM_StoreData: Opening file %s\n", tmpfile); fd = open(tmpfile, O_WRONLY|O_CREAT|O_TRUNC|O_NOFOLLOW, - tpmstate_get_mode()); /* closed @1 */ + tpmstate_get_mode(&mode_is_default)); /* closed @1 */ if (fd < 0) { logprintf(STDERR_FILENO, "SWTPM_NVRAM_StoreData: Error (fatal) opening %s for " @@ -399,6 +414,15 @@ SWTPM_NVRAM_StoreData_Dir(unsigned char *filedata, } } + if (rc == 0 && !mode_is_default) { + if (fchmod(fd, tpmstate_get_mode(&mode_is_default)) < 0) { + logprintf(STDERR_FILENO, + "SWTPM_NVRAM_StoreData: Error (fatal) changing mode bits " + "on %s failed: %s\n", tmpfile, strerror(errno)); + rc = TPM_FAIL; + } + } + /* write the data to the file */ if (rc == 0) { TPM_DEBUG(" SWTPM_NVRAM_StoreData: Writing %u bytes of data\n", diff --git a/src/swtpm/swtpm_nvstore_linear_file.c b/src/swtpm/swtpm_nvstore_linear_file.c index 24dc706..6a6584e 100644 --- a/src/swtpm/swtpm_nvstore_linear_file.c +++ b/src/swtpm/swtpm_nvstore_linear_file.c @@ -157,6 +157,7 @@ SWTPM_NVRAM_LinearFile_Open(const char* uri, { TPM_RESULT rc = 0; const char *path = SWTPM_NVRAM_LinearFile_UriToPath(uri); + bool mode_is_default = false; if (mmap_state.mapped) { logprintf(STDERR_FILENO, @@ -164,13 +165,22 @@ SWTPM_NVRAM_LinearFile_Open(const char* uri, return TPM_FAIL; } - mmap_state.fd = open(path, O_RDWR|O_CREAT, tpmstate_get_mode()); + mmap_state.fd = open(path, O_RDWR|O_CREAT, + tpmstate_get_mode(&mode_is_default)); if (mmap_state.fd < 0) { logprintf(STDERR_FILENO, "SWTPM_NVRAM_LinearFile_Open: Could not open file: %s\n", strerror(errno)); return TPM_FAIL; } + /* Set non-default (user-provided) mode bits not masked by umask */ + if (!mode_is_default && + fchmod(mmap_state.fd, tpmstate_get_mode(&mode_is_default)) < 0) { + logprintf(STDERR_FILENO, + "SWTPM_NVRAM_LinearFile_Open: Could not change mode bits: %s\n", + strerror(errno)); + return TPM_FAIL; + } rc = SWTPM_NVRAM_LinearFile_Mmap(); if (rc == 0) { diff --git a/src/swtpm/tpmstate.c b/src/swtpm/tpmstate.c index 2eefdbc..852b515 100644 --- a/src/swtpm/tpmstate.c +++ b/src/swtpm/tpmstate.c @@ -50,6 +50,8 @@ static char *g_backend_uri; static mode_t g_tpmstate_mode = 0640; +/* false if provided user via command line mode=..., true if using default */ +static bool g_tpmstate_mode_is_default = true; static TPMLIB_TPMVersion g_tpmstate_version = TPMLIB_TPM_VERSION_1_2; void tpmstate_global_free(void) @@ -86,15 +88,17 @@ const char *tpmstate_get_backend_uri(void) return NULL; } -int tpmstate_set_mode(mode_t mode) +int tpmstate_set_mode(mode_t mode, bool mode_is_default) { g_tpmstate_mode = mode; + g_tpmstate_mode_is_default = mode_is_default; return 0; } -mode_t tpmstate_get_mode(void) +mode_t tpmstate_get_mode(bool *mode_is_default) { + *mode_is_default = g_tpmstate_mode_is_default; return g_tpmstate_mode; } diff --git a/src/swtpm/tpmstate.h b/src/swtpm/tpmstate.h index 9e37d8e..3d36135 100644 --- a/src/swtpm/tpmstate.h +++ b/src/swtpm/tpmstate.h @@ -38,14 +38,15 @@ #ifndef _SWTPM_TPMSTATE_H_ #define _SWTPM_TPMSTATE_H_ +#include #include #include int tpmstate_set_backend_uri(char *backend_uri); const char *tpmstate_get_backend_uri(void); -int tpmstate_set_mode(mode_t mode); -mode_t tpmstate_get_mode(void); +int tpmstate_set_mode(mode_t mode, bool mode_is_default); +mode_t tpmstate_get_mode(bool *mode_is_default); void tpmstate_global_free(void); void tpmstate_set_version(TPMLIB_TPMVersion version);