From b3d12b6a8f0ad7d56a584c1962e92f69b289a2a2 Mon Sep 17 00:00:00 2001 From: Sergio Correia Date: Mon, 27 Sep 2021 15:19:42 -0300 Subject: [PATCH 3/3] keys: make sure keys are created with 0440 mode There is no need for them to be readable by other than the owner/group. Helpers (tangd-keygen and tangd-rotate-keys) also updated. --- src/keys.c | 7 ++++++ src/tangd-keygen | 3 +++ src/tangd-rotate-keys | 1 + tests/adv | 11 +++++++++ tests/helpers | 5 ++++ tests/rec | 3 +++ tests/test-keys.c.in | 54 +++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 84 insertions(+) diff --git a/src/keys.c b/src/keys.c index 55d0cff..a433f9c 100644 --- a/src/keys.c +++ b/src/keys.c @@ -17,6 +17,7 @@ * along with this program. If not, see . */ +#include #include #include #include @@ -337,6 +338,12 @@ create_new_keys(const char* jwkdir) fprintf(stderr, "Error saving JWK to file (%s)\n", path); return 0; } + + /* Set 0440 permission for the new key. */ + if (chmod(path, S_IRUSR | S_IRGRP) == -1) { + fprintf(stderr, "Unable to set permissions for JWK file (%s)\n", path); + return 0; + } } return 1; } diff --git a/src/tangd-keygen b/src/tangd-keygen index f37121f..ed51124 100755 --- a/src/tangd-keygen +++ b/src/tangd-keygen @@ -34,7 +34,10 @@ THP_DEFAULT_HASH=S256 # SHA-256. jwe=$(jose jwk gen -i '{"alg":"ES512"}') [ -z "$sig" ] && sig=$(echo "$jwe" | jose jwk thp -i- -a "${THP_DEFAULT_HASH}") echo "$jwe" > "$1/$sig.jwk" +chmod 0440 "$1/$sig.jwk" + jwe=$(jose jwk gen -i '{"alg":"ECMR"}') [ -z "$exc" ] && exc=$(echo "$jwe" | jose jwk thp -i- -a "${THP_DEFAULT_HASH}") echo "$jwe" > "$1/$exc.jwk" +chmod 0440 "$1/$exc.jwk" diff --git a/src/tangd-rotate-keys b/src/tangd-rotate-keys index a095a91..8649652 100755 --- a/src/tangd-rotate-keys +++ b/src/tangd-rotate-keys @@ -78,6 +78,7 @@ cd "${JWKDIR}" || error "Unable to change to keys directory '${JWKDIR}'" thp="$(printf '%s' "${jwe}" | jose jwk thp --input=- \ -a "${DEFAULT_THP_HASH}")" echo "${jwe}" > "${thp}.jwk" + chmod 0440 "${thp}.jwk" log "Created new key ${thp}.jwk" "${VERBOSE}" done cd - >/dev/null diff --git a/tests/adv b/tests/adv index 4c8bc97..1fde37e 100755 --- a/tests/adv +++ b/tests/adv @@ -27,6 +27,10 @@ export TMP=`mktemp -d` mkdir -p $TMP/db tangd-keygen $TMP/db sig exc +# Make sure keys generated by tangd-keygen have proper permissions. +valid_key_perm "${TMP}/db/sig.jwk" +valid_key_perm "${TMP}/db/exc.jwk" + jose jwk gen -i '{"alg": "ES512"}' -o $TMP/db/.sig.jwk jose jwk gen -i '{"alg": "ES512"}' -o $TMP/db/.oth.jwk @@ -96,6 +100,10 @@ for i in 1 2 3 4 5 6 7 8 9; do # Make sure the requested keys exist and are valid. validate_sig "${TMP}/db/other-sig-${i}.jwk" validate_exc "${TMP}/db/other-exc-${i}.jwk" + + # Make sure keys generated by tangd-keygen have proper permissions. + valid_key_perm "${TMP}/db/other-sig-${i}.jwk" + valid_key_perm "${TMP}/db/other-exc-${i}.jwk" done # Verify the advertisement is correct. @@ -121,6 +129,9 @@ thp= for jwk in "${TMP}"/db/*.jwk; do validate_sig "${jwk}" && thp="$(jose jwk thp -a "${THP_DEFAULT_HASH}" \ -i "${jwk}")" + + # Make sure keys generated by tangd-rotate-keys have proper permissions. + valid_key_perm "${jwk}" done [ -z "${thp}" ] && die "There should be valid keys after rotation" test "$(tang-show-keys $PORT)" = "${thp}" diff --git a/tests/helpers b/tests/helpers index 7ce54d7..8b789fb 100755 --- a/tests/helpers +++ b/tests/helpers @@ -75,3 +75,8 @@ die() { echo "${1}" >&2 exit 1 } + +valid_key_perm() { + _perm="$(stat -c %a "${1}")" + [ "${_perm}" = "440" ] +} diff --git a/tests/rec b/tests/rec index af0d075..7fba6a9 100755 --- a/tests/rec +++ b/tests/rec @@ -28,6 +28,9 @@ mkdir -p $TMP/db # Generate the server keys tangd-keygen $TMP/db sig exc +# Make sure keys generated by tangd-keygen have proper permissions. +valid_key_perm "${TMP}/db/sig.jwk" +valid_key_perm "${TMP}/db/exc.jwk" # Generate the client keys exc_kid=`jose jwk thp -i $TMP/db/exc.jwk` diff --git a/tests/test-keys.c.in b/tests/test-keys.c.in index 1f811f3..fca26c4 100644 --- a/tests/test-keys.c.in +++ b/tests/test-keys.c.in @@ -32,6 +32,56 @@ struct test_result_int { int expected; }; +static void +verify_keys_permissions(const char* targetdir) +{ + struct stat st; + struct dirent* d; + DIR* dir = opendir(targetdir); + ASSERT(dir); + char filepath[PATH_MAX]; + const char* pattern = ".jwk"; + while ((d = readdir(dir)) != NULL) { + if (strcmp(d->d_name, ".") == 0 || strcmp(d->d_name, "..") == 0) { + continue; + } + + char* dot = strrchr(d->d_name, '.'); + if (!dot) { + continue; + } + + if (strcmp(dot, pattern) == 0) { + /* Found a file with .jwk extension. */ + if (snprintf(filepath, PATH_MAX, "%s/%s", targetdir, d->d_name) < 0) { + fprintf(stderr, "Unable to prepare variable with file full path (%s); skipping\n", d->d_name); + continue; + } + filepath[sizeof(filepath) - 1] = '\0'; + ASSERT(stat(filepath, &st) == 0); + + ASSERT_WITH_MSG(st.st_mode & (S_IRUSR | S_IRGRP), "key = %s, missing perm (0%o)", filepath, (S_IRUSR | S_IRGRP)); + int unexpected_perms[] = { + S_ISUID, /* 04000 set-user-ID */ + S_ISGID, /* 02000 set-group-ID */ + S_IWUSR, /* 00200 write by owner */ + S_IXUSR, /* 00100 execute/search by owner */ + S_IWGRP, /* 00020 write by group */ + S_IXGRP, /* 00010 execute/search by group */ + S_IROTH, /* 00004 read by others */ + S_IWOTH, /* 00002 write by others */ + S_IXOTH, /* 00001 execute/search by others */ + 0 + }; + for (int i = 0; unexpected_perms[i] != 0; i++) { + ASSERT_WITH_MSG((st.st_mode & unexpected_perms[i]) == 0, "key = %s, i = %d, unexpected perm (0%o)", filepath, i, unexpected_perms[i]); + } + + } + } + closedir(dir); +} + static void test_create_new_keys(void) { @@ -40,6 +90,10 @@ test_create_new_keys(void) __attribute__((cleanup(cleanup_tang_keys_info))) struct tang_keys_info* tki = read_keys(newdir); ASSERT(tki); ASSERT(tki->m_keys_count == 2); + + /* Make sure keys have proper permissions. */ + verify_keys_permissions(newdir); + remove_tempdir(newdir); } -- 2.31.1