From 05e8c7ef6a86e17a0ac421b9c80a2e57f56b4b3d Mon Sep 17 00:00:00 2001 From: rfairley Date: Tue, 6 Nov 2018 15:25:15 -0500 Subject: [PATCH] lib/repo: Search a list of paths in gpgkeypath for gpg keys This allows specifying gpgpath as list of paths that can point to a file or a directory. If a directory path is given, paths to all regular files in the directory are added to the remote as gpg ascii keys. If the path is not a directory, the file is directly added (whether regular file, empty - errors will be reported later when verifying gpg keys e.g. when pulling). Adding the gpgkeypath property looks like: ostree --repo=repo remote add --set=gpgpath="/path/key1.asc,/path/keys.d" R1 https://example.com/some/remote/ostree/repo Closes #773 Closes: #1773 Approved by: cgwalters --- man/ostree.xml | 37 +++++----- src/libostree/ostree-gpg-verifier.c | 73 +++++++++++++++++++- src/libostree/ostree-gpg-verifier.h | 13 ++++ src/libostree/ostree-repo.c | 16 +++-- src/libotutil/ot-keyfile-utils.c | 101 ++++++++++++++++++++++++++++ src/libotutil/ot-keyfile-utils.h | 17 +++++ tests/test-remote-gpg-import.sh | 73 ++++++++++++++++++++ 7 files changed, 307 insertions(+), 23 deletions(-) diff --git a/man/ostree.xml b/man/ostree.xml index d1c156659e..f865982e7f 100644 --- a/man/ostree.xml +++ b/man/ostree.xml @@ -432,6 +432,7 @@ Boston, MA 02111-1307, USA. Examples + For specific examples, please see the man page regarding the specific ostree command. For example: @@ -445,28 +446,32 @@ Boston, MA 02111-1307, USA. OSTree supports signing commits with GPG. Operations on the system - repository by default use keyring files in + repository by default use keyring files in /usr/share/ostree/trusted.gpg.d. Any public key in a keyring file in that directory will be trusted by the client. No private keys should be present in this directory. - In addition to the system repository, OSTree supports two - other paths. First, there is a - gpgkeypath option for remotes, which must - point to the filename of an ASCII-armored key. - - Second, there is support for a per-remote - remotename.trustedkeys.gpg - file stored in the toplevel of the repository (alongside - objects/ and such). This is - particularly useful when downloading content that may not - be fully trusted (e.g. you want to inspect it but not - deploy it as an OS), or use it for containers. This file - is written via ostree remote add - --gpg-import. - + In addition to the system repository, OSTree supports two + other paths. First, there is a + gpgkeypath option for remotes, which must point + to the filename of an ASCII-armored GPG key, or a directory containing + ASCII-armored GPG keys to import. Multiple file and directory paths + to import from can be specified, as a comma-separated list of paths. This option + may be specified by using --set in ostree remote add. + + + Second, there is support for a per-remote + remotename.trustedkeys.gpg + file stored in the toplevel of the repository (alongside + objects/ and such). This is + particularly useful when downloading content that may not + be fully trusted (e.g. you want to inspect it but not + deploy it as an OS), or use it for containers. This file + is written via ostree remote add + --gpg-import. + diff --git a/src/libostree/ostree-gpg-verifier.c b/src/libostree/ostree-gpg-verifier.c index 1b70f8aa4e..a279348eb3 100644 --- a/src/libostree/ostree-gpg-verifier.c +++ b/src/libostree/ostree-gpg-verifier.c @@ -304,12 +304,82 @@ _ostree_gpg_verifier_add_key_ascii_file (OstreeGpgVerifier *self, g_ptr_array_add (self->key_ascii_files, g_strdup (path)); } +gboolean +_ostree_gpg_verifier_add_keyfile_path (OstreeGpgVerifier *self, + const char *path, + GCancellable *cancellable, + GError **error) +{ + g_autoptr(GError) temp_error = NULL; + if (!_ostree_gpg_verifier_add_keyfile_dir_at (self, AT_FDCWD, path, + cancellable, &temp_error)) + { + g_assert (temp_error); + + /* If failed due to not being a directory, add the file as an ascii key. */ + if (g_error_matches (temp_error, G_IO_ERROR, G_IO_ERROR_NOT_DIRECTORY)) + { + g_clear_error (&temp_error); + + _ostree_gpg_verifier_add_key_ascii_file (self, path); + } + else + { + g_propagate_error (error, g_steal_pointer (&temp_error)); + + return FALSE; + } + } + return TRUE; +} + +/* Add files that exist one level below the directory at @path as ascii + * key files. If @path cannot be opened as a directory, + * an error is returned. + */ +gboolean +_ostree_gpg_verifier_add_keyfile_dir_at (OstreeGpgVerifier *self, + int dfd, + const char *path, + GCancellable *cancellable, + GError **error) +{ + g_auto(GLnxDirFdIterator) dfd_iter = { 0, }; + + if (!glnx_dirfd_iterator_init_at (dfd, path, FALSE, + &dfd_iter, error)) + return FALSE; + + g_debug ("Adding GPG keyfile dir %s to verifier", path); + + while (TRUE) + { + struct dirent *dent; + + if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&dfd_iter, &dent, + cancellable, error)) + return FALSE; + if (dent == NULL) + break; + + if (dent->d_type != DT_REG) + continue; + + /* TODO: Potentially open the files here and have the GPG verifier iterate + over the fds. See https://github.com/ostreedev/ostree/pull/1773#discussion_r235421900. */ + g_autofree char *iter_path = g_build_filename (path, dent->d_name, NULL); + + _ostree_gpg_verifier_add_key_ascii_file (self, iter_path); + } + + return TRUE; +} + gboolean _ostree_gpg_verifier_add_keyring_dir (OstreeGpgVerifier *self, GFile *path, GCancellable *cancellable, GError **error) - { return _ostree_gpg_verifier_add_keyring_dir_at (self, AT_FDCWD, gs_file_get_path_cached (path), @@ -322,7 +392,6 @@ _ostree_gpg_verifier_add_keyring_dir_at (OstreeGpgVerifier *self, const char *path, GCancellable *cancellable, GError **error) - { g_auto(GLnxDirFdIterator) dfd_iter = { 0, }; if (!glnx_dirfd_iterator_init_at (dfd, path, FALSE, diff --git a/src/libostree/ostree-gpg-verifier.h b/src/libostree/ostree-gpg-verifier.h index 4fe62d98e7..634d33b299 100644 --- a/src/libostree/ostree-gpg-verifier.h +++ b/src/libostree/ostree-gpg-verifier.h @@ -75,4 +75,17 @@ void _ostree_gpg_verifier_add_keyring_file (OstreeGpgVerifier *self, void _ostree_gpg_verifier_add_key_ascii_file (OstreeGpgVerifier *self, const char *path); +gboolean +_ostree_gpg_verifier_add_keyfile_path (OstreeGpgVerifier *self, + const char *path, + GCancellable *cancellable, + GError **error); + +gboolean +_ostree_gpg_verifier_add_keyfile_dir_at (OstreeGpgVerifier *self, + int dfd, + const char *path, + GCancellable *cancellable, + GError **error); + G_END_DECLS diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index fa3c2a940f..f5231db989 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -5081,7 +5081,6 @@ _ostree_repo_gpg_verify_data_internal (OstreeRepo *self, } else if (remote_name != NULL) { - g_autofree char *gpgkeypath = NULL; /* Add the remote's keyring file if it exists. */ g_autoptr(OstreeRemote) remote = NULL; @@ -5100,12 +5099,19 @@ _ostree_repo_gpg_verify_data_internal (OstreeRepo *self, add_global_keyring_dir = FALSE; } - if (!ot_keyfile_get_value_with_default (remote->options, remote->group, "gpgkeypath", NULL, - &gpgkeypath, error)) + g_auto(GStrv) gpgkeypath_list = NULL; + + if (!ot_keyfile_get_string_as_list (remote->options, remote->group, "gpgkeypath", + ";,", &gpgkeypath_list, error)) return NULL; - if (gpgkeypath) - _ostree_gpg_verifier_add_key_ascii_file (verifier, gpgkeypath); + if (gpgkeypath_list) + { + for (char **iter = gpgkeypath_list; *iter != NULL; ++iter) + if (!_ostree_gpg_verifier_add_keyfile_path (verifier, *iter, + cancellable, error)) + return NULL; + } } if (add_global_keyring_dir) diff --git a/src/libotutil/ot-keyfile-utils.c b/src/libotutil/ot-keyfile-utils.c index 3b29377f2f..a0ab75cc0c 100644 --- a/src/libotutil/ot-keyfile-utils.c +++ b/src/libotutil/ot-keyfile-utils.c @@ -101,6 +101,107 @@ ot_keyfile_get_value_with_default (GKeyFile *keyfile, return ret; } +/* Read the value of key as a string. If the value string contains + * one of the separators and none of the others, read the + * string as a NULL-terminated array out_value. If the value string contains + * none of the separators, read the string as a single entry into a + * NULL-terminated array out_value. If the value string contains multiple of + * the separators, an error is given. + * Returns TRUE on success, FALSE on error. */ +gboolean +ot_keyfile_get_string_as_list (GKeyFile *keyfile, + const char *section, + const char *key, + const char *separators, + char ***out_value, + GError **error) +{ + guint sep_count = 0; + gchar sep = '\0'; + g_autofree char *value_str = NULL; + g_autofree char **value_list = NULL; + + g_return_val_if_fail (keyfile != NULL, FALSE); + g_return_val_if_fail (section != NULL, FALSE); + g_return_val_if_fail (key != NULL, FALSE); + g_return_val_if_fail (separators != NULL, FALSE); + + if (!ot_keyfile_get_value_with_default (keyfile, section, key, NULL, + &value_str, error)) + return FALSE; + + if (value_str) + { + for (size_t i = 0; i < strlen (separators) && sep_count <= 1; i++) + { + if (strchr (value_str, separators[i])) + { + sep_count++; + sep = separators[i]; + } + } + + if (sep_count == 0) + { + value_list = g_new (gchar *, 2); + value_list[0] = g_steal_pointer (&value_str); + value_list[1] = NULL; + } + else if (sep_count == 1) + { + if (!ot_keyfile_get_string_list_with_default (keyfile, section, key, + sep, NULL, &value_list, error)) + return FALSE; + } + else + { + return glnx_throw (error, "key value list contains more than one separator"); + } + } + + ot_transfer_out_value (out_value, &value_list); + return TRUE; +} + +gboolean +ot_keyfile_get_string_list_with_default (GKeyFile *keyfile, + const char *section, + const char *key, + char separator, + char **default_value, + char ***out_value, + GError **error) +{ + g_autoptr(GError) temp_error = NULL; + + g_return_val_if_fail (keyfile != NULL, FALSE); + g_return_val_if_fail (section != NULL, FALSE); + g_return_val_if_fail (key != NULL, FALSE); + + g_key_file_set_list_separator (keyfile, separator); + + g_autofree char **ret_value = g_key_file_get_string_list (keyfile, section, + key, NULL, &temp_error); + + if (temp_error) + { + if (g_error_matches (temp_error, G_KEY_FILE_ERROR, + G_KEY_FILE_ERROR_KEY_NOT_FOUND)) + { + g_clear_error (&temp_error); + ret_value = default_value; + } + else + { + g_propagate_error (error, g_steal_pointer (&temp_error)); + return FALSE; + } + } + + ot_transfer_out_value (out_value, &ret_value); + return TRUE; +} + gboolean ot_keyfile_copy_group (GKeyFile *source_keyfile, GKeyFile *target_keyfile, diff --git a/src/libotutil/ot-keyfile-utils.h b/src/libotutil/ot-keyfile-utils.h index e136ee47a6..2dcb899cf7 100644 --- a/src/libotutil/ot-keyfile-utils.h +++ b/src/libotutil/ot-keyfile-utils.h @@ -44,6 +44,23 @@ ot_keyfile_get_value_with_default (GKeyFile *keyfile, char **out_value, GError **error); +gboolean +ot_keyfile_get_string_as_list (GKeyFile *keyfile, + const char *section, + const char *key, + const char *separators, + char ***out_value_list, + GError **error); + +gboolean +ot_keyfile_get_string_list_with_default (GKeyFile *keyfile, + const char *section, + const char *key, + char separator, + char **default_value, + char ***out_value, + GError **error); + gboolean ot_keyfile_copy_group (GKeyFile *source_keyfile, GKeyFile *target_keyfile, diff --git a/tests/test-remote-gpg-import.sh b/tests/test-remote-gpg-import.sh index f816429c4d..1bb42d82e8 100755 --- a/tests/test-remote-gpg-import.sh +++ b/tests/test-remote-gpg-import.sh @@ -154,6 +154,79 @@ ${OSTREE} prune --refs-only ${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/key3.asc R4 $(cat httpd-address)/ostree/gnomerepo ${OSTREE} pull R4:main >/dev/null +# Test gpgkeypath success with multiple keys to try +${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/key1.asc,${test_tmpdir}/gpghome/key2.asc,${test_tmpdir}/gpghome/key3.asc R7 $(cat httpd-address)/ostree/gnomerepo +${OSTREE} pull R7:main >/dev/null + +# Test gpgkeypath failure with multiple keys but none in keyring +${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/key1.asc,${test_tmpdir}/gpghome/key2.asc R8 $(cat httpd-address)/ostree/gnomerepo +if ${OSTREE} pull R8:main 2>err.txt; then + assert_not_reached "Unexpectedly succeeded at pulling with different key" +fi +assert_file_has_content err.txt "GPG signatures found, but none are in trusted keyring" + +# Test gpgkeypath success with directory containing a valid key +${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/ R9 $(cat httpd-address)/ostree/gnomerepo +${OSTREE} pull R9:main >/dev/null + +# Test gpgkeypath failure with nonexistent directory +${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/INVALIDKEYDIRPATH/ R10 $(cat httpd-address)/ostree/gnomerepo +if ${OSTREE} pull R10:main 2>err.txt; then + assert_not_reached "Unexpectedly succeeded at pulling with nonexistent key directory" +fi +assert_file_has_content err.txt "INVALIDKEYDIRPATH.*No such file or directory" + +# Test gpgkeypath failure with a directory containing a valid key, and a nonexistent key +${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/,${test_tmpdir}/gpghome/INVALIDKEYPATH.asc R11 $(cat httpd-address)/ostree/gnomerepo +if ${OSTREE} pull R11:main 2>err.txt; then + assert_not_reached "Unexpectedly succeeded at pulling with nonexistent key" +fi +assert_file_has_content err.txt "INVALIDKEYPATH.*No such file or directory" + +# Test gpgkeypath success with a directory containing a valid key, and a key not in keyring +${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/,${test_tmpdir}/gpghome/key1.asc R12 $(cat httpd-address)/ostree/gnomerepo +${OSTREE} pull R12:main >/dev/null + +# Test gpgkeypath failure with a nonexistent directory, and a valid key +${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/INVALIDKEYDIRPATH/,${test_tmpdir}/gpghome/key3.asc R13 $(cat httpd-address)/ostree/gnomerepo +if ${OSTREE} pull R13:main 2>err.txt; then + assert_not_reached "Unexpectedly succeeded at pulling with nonexistent key directory" +fi +assert_file_has_content err.txt "INVALIDKEYDIRPATH.*No such file or directory" + +# Test gpgkeypath failure with a nonexistent directory and a nonexistent key +${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/INVALIDKEYDIRPATH/,${test_tmpdir}/gpghome/INVALIDKEYPATH.asc R14 $(cat httpd-address)/ostree/gnomerepo +if ${OSTREE} pull R14:main 2>err.txt; then + assert_not_reached "Unexpectedly succeeded at pulling with nonexistent key" +fi +assert_file_has_content err.txt "INVALIDKEYDIRPATH.*No such file or directory" + +# Test gpgkeypath success for no trailing slash in directory path +${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome R15 $(cat httpd-address)/ostree/gnomerepo +${OSTREE} pull R15:main >/dev/null + +# Test gpgkeypath failure with prefixed separator giving an empty path, and a nonexistent key +${OSTREE} remote add --set=gpgkeypath=,${test_tmpdir}/gpghome/INVALIDKEYPATH.asc R16 $(cat httpd-address)/ostree/gnomerepo +if ${OSTREE} pull R16:main 2>err.txt; then + assert_not_reached "Unexpectedly succeeded at pulling with nonexistent key" +fi +assert_file_has_content err.txt "().*No such file or directory" + +# Test gpgkeypath success with suffixed separator +${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/key3.asc, R17 $(cat httpd-address)/ostree/gnomerepo +${OSTREE} pull R17:main >/dev/null + +# Test gpgkeypath success with multiple keys specified, with semicolons +${OSTREE} remote add --set=gpgkeypath="${test_tmpdir}/gpghome/key1.asc;${test_tmpdir}/gpghome/key2.asc;${test_tmpdir}/gpghome/key3.asc" R18 $(cat httpd-address)/ostree/gnomerepo +${OSTREE} pull R18:main >/dev/null + +# Test gpgkeypath failure multiple keys specified, with mix of commas and semicolons +${OSTREE} remote add --set=gpgkeypath="${test_tmpdir}/gpghome/key1.asc,${test_tmpdir}/gpghome/key2.asc;${test_tmpdir}/gpghome/key3.asc" R19 $(cat httpd-address)/ostree/gnomerepo +if ${OSTREE} pull R19:main 2>err.txt; then + assert_not_reached "Unexpectedly succeeded at pulling with invalid gpgkeypath value" +fi +assert_file_has_content err.txt ".*key value list contains more than one separator" + rm repo/refs/remotes/* -rf ${OSTREE} prune --refs-only