diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index bbd9f0e6..c3c1a54e 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -7,12 +7,14 @@ on: - flatpak-1.0.x - flatpak-1.2.x - flatpak-1.4.x + - flatpak-1.6.x pull_request: branches: - master - flatpak-1.0.x - flatpak-1.2.x - flatpak-1.4.x + - flatpak-1.6.x jobs: check: diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c index 054db04b..5062356a 100644 --- a/common/flatpak-dir.c +++ b/common/flatpak-dir.c @@ -15153,7 +15153,7 @@ flatpak_dir_delete_mirror_refs (FlatpakDir *self, * OS updates which Flatpak shouldn't touch. */ remotes = ostree_repo_remote_list (repo, NULL); - for (i = 0; remotes[i] != NULL; i++) + for (i = 0; remotes != NULL && remotes[i] != NULL; i++) { g_autofree char *remote_collection_id = NULL; diff --git a/common/flatpak-oci-registry.c b/common/flatpak-oci-registry.c index 420efff0..2505771e 100644 --- a/common/flatpak-oci-registry.c +++ b/common/flatpak-oci-registry.c @@ -901,6 +901,7 @@ object_get_string_member_with_default (JsonNode *json, static char * get_token_for_www_auth (FlatpakOciRegistry *self, + const char *repository, const char *www_authenticate, const char *auth, GCancellable *cancellable, @@ -911,6 +912,7 @@ get_token_for_www_auth (FlatpakOciRegistry *self, g_autoptr(GHashTable) params = NULL; g_autoptr(GHashTable) args = NULL; const char *realm, *service, *scope, *token; + g_autofree char *default_scope = NULL; g_autoptr(SoupURI) auth_uri = NULL; g_autoptr(GBytes) body = NULL; g_autoptr(JsonNode) json = NULL; @@ -941,16 +943,21 @@ get_token_for_www_auth (FlatpakOciRegistry *self, service = g_hash_table_lookup (params, "service"); if (service) g_hash_table_insert (args, "service", (char *)service); + scope = g_hash_table_lookup (params, "scope"); - if (scope) - g_hash_table_insert (args, "scope", (char *)scope); + if (scope == NULL) + scope = default_scope = g_strdup_printf("repository:%s:pull", repository); + g_hash_table_insert (args, "scope", (char *)scope); soup_uri_set_query_from_form (auth_uri, args); auth_msg = soup_message_new_from_uri ("GET", auth_uri); - g_autofree char *basic_auth = g_strdup_printf ("Basic %s", auth); - soup_message_headers_replace (auth_msg->request_headers, "Authorization", basic_auth); + if (auth) + { + g_autofree char *basic_auth = g_strdup_printf ("Basic %s", auth); + soup_message_headers_replace (auth_msg->request_headers, "Authorization", basic_auth); + } auth_stream = soup_session_send (self->soup_session, auth_msg, NULL, error); if (auth_stream == NULL) @@ -1030,7 +1037,7 @@ flatpak_oci_registry_get_token (FlatpakOciRegistry *self, return NULL; } - token = get_token_for_www_auth (self, www_authenticate, basic_auth, cancellable, error); + token = get_token_for_www_auth (self, repository, www_authenticate, basic_auth, cancellable, error); if (token == NULL) return NULL; diff --git a/common/flatpak-run.c b/common/flatpak-run.c index e9fa2961..51c002ff 100644 --- a/common/flatpak-run.c +++ b/common/flatpak-run.c @@ -2830,17 +2830,13 @@ flatpak_run_setup_base_argv (FlatpakBwrap *bwrap, { g_autofree char *run_dir = NULL; g_autofree char *passwd_contents = NULL; - g_autofree char *group_contents = NULL; + g_autoptr(GString) group_contents = NULL; const char *pkcs11_conf_contents = NULL; struct group *g; gulong pers; gid_t gid = getgid (); g_autoptr(GFile) etc = NULL; - g = getgrgid (gid); - if (g == NULL) - return flatpak_fail_error (error, FLATPAK_ERROR_SETUP_FAILED, _("Invalid group: %d"), gid); - run_dir = g_strdup_printf ("/run/user/%d", getuid ()); passwd_contents = g_strdup_printf ("%s:x:%d:%d:%s:%s:%s\n" @@ -2851,10 +2847,14 @@ flatpak_run_setup_base_argv (FlatpakBwrap *bwrap, g_get_home_dir (), DEFAULT_SHELL); - group_contents = g_strdup_printf ("%s:x:%d:%s\n" - "nfsnobody:x:65534:\n", - g->gr_name, - gid, g_get_user_name ()); + group_contents = g_string_new (""); + g = getgrgid (gid); + /* if NULL, the primary group is not known outside the container, so + * it might as well stay unknown inside the container... */ + if (g != NULL) + g_string_append_printf (group_contents, "%s:x:%d:%s\n", + g->gr_name, gid, g_get_user_name ()); + g_string_append (group_contents, "nfsnobody:x:65534:\n"); pkcs11_conf_contents = "# Disable user pkcs11 config, because the host modules don't work in the runtime\n" @@ -2897,7 +2897,7 @@ flatpak_run_setup_base_argv (FlatpakBwrap *bwrap, if (!flatpak_bwrap_add_args_data (bwrap, "passwd", passwd_contents, -1, "/etc/passwd", error)) return FALSE; - if (!flatpak_bwrap_add_args_data (bwrap, "group", group_contents, -1, "/etc/group", error)) + if (!flatpak_bwrap_add_args_data (bwrap, "group", group_contents->str, -1, "/etc/group", error)) return FALSE; if (!flatpak_bwrap_add_args_data (bwrap, "pkcs11.conf", pkcs11_conf_contents, -1, "/etc/pkcs11/pkcs11.conf", error)) diff --git a/common/flatpak-utils-http.c b/common/flatpak-utils-http.c index 2ff30366..5b16bf3b 100644 --- a/common/flatpak-utils-http.c +++ b/common/flatpak-utils-http.c @@ -28,6 +28,9 @@ #include #include +/* copied from libostree */ +#define DEFAULT_N_NETWORK_RETRIES 5 + typedef struct { char *uri; @@ -432,11 +435,33 @@ load_uri_callback (GObject *source_object, code = FLATPAK_OCI_ERROR_NOT_CHANGED; break; + case 403: case 404: case 410: code = G_IO_ERROR_NOT_FOUND; break; + case 408: + code = G_IO_ERROR_TIMED_OUT; + break; + + case SOUP_STATUS_CANCELLED: + code = G_IO_ERROR_CANCELLED; + break; + + case SOUP_STATUS_CANT_RESOLVE: + case SOUP_STATUS_CANT_CONNECT: + code = G_IO_ERROR_HOST_NOT_FOUND; + break; + + case SOUP_STATUS_IO_ERROR: +#if !GLIB_CHECK_VERSION(2, 44, 0) + code = G_IO_ERROR_BROKEN_PIPE; +#else + code = G_IO_ERROR_CONNECTION_CLOSED; +#endif + break; + default: code = G_IO_ERROR_FAILED; } @@ -510,15 +535,59 @@ flatpak_create_soup_session (const char *user_agent) return soup_session; } -GBytes * -flatpak_load_http_uri (SoupSession *soup_session, - const char *uri, - FlatpakHTTPFlags flags, - const char *token, - FlatpakLoadUriProgress progress, - gpointer user_data, - GCancellable *cancellable, - GError **error) +/* Check whether a particular operation should be retried. This is entirely + * based on how it failed (if at all) last time, and whether the operation has + * some retries left. The retry count is set when the operation is first + * created, and must be decremented by the caller. (@n_retries_remaining == 0) + * will always return %FALSE from this function. + * + * This code is copied from libostree's _ostree_fetcher_should_retry_request() + */ +static gboolean +flatpak_http_should_retry_request (const GError *error, + guint n_retries_remaining) +{ + if (error == NULL) + g_debug ("%s: error: unset, n_retries_remaining: %u", + G_STRFUNC, n_retries_remaining); + else + g_debug ("%s: error: %u:%u %s, n_retries_remaining: %u", + G_STRFUNC, error->domain, error->code, error->message, + n_retries_remaining); + + if (error == NULL || n_retries_remaining == 0) + return FALSE; + + /* Return TRUE for transient errors. */ + if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_TIMED_OUT) || + g_error_matches (error, G_IO_ERROR, G_IO_ERROR_HOST_NOT_FOUND) || + g_error_matches (error, G_IO_ERROR, G_IO_ERROR_HOST_UNREACHABLE) || + g_error_matches (error, G_IO_ERROR, G_IO_ERROR_PARTIAL_INPUT) || +#if !GLIB_CHECK_VERSION(2, 44, 0) + g_error_matches (error, G_IO_ERROR, G_IO_ERROR_BROKEN_PIPE) || +#else + g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CONNECTION_CLOSED) || +#endif + g_error_matches (error, G_RESOLVER_ERROR, G_RESOLVER_ERROR_NOT_FOUND) || + g_error_matches (error, G_RESOLVER_ERROR, G_RESOLVER_ERROR_TEMPORARY_FAILURE)) + { + g_debug ("Should retry request (remaining: %u retries), due to transient error: %s", + n_retries_remaining, error->message); + return TRUE; + } + + return FALSE; +} + +static GBytes * +flatpak_load_http_uri_once (SoupSession *soup_session, + const char *uri, + FlatpakHTTPFlags flags, + const char *token, + FlatpakLoadUriProgress progress, + gpointer user_data, + GCancellable *cancellable, + GError **error) { GBytes *bytes = NULL; g_autoptr(GMainContext) context = NULL; @@ -576,16 +645,56 @@ flatpak_load_http_uri (SoupSession *soup_session, return bytes; } -gboolean -flatpak_download_http_uri (SoupSession *soup_session, - const char *uri, - FlatpakHTTPFlags flags, - GOutputStream *out, - const char *token, - FlatpakLoadUriProgress progress, - gpointer user_data, - GCancellable *cancellable, - GError **error) +GBytes * +flatpak_load_http_uri (SoupSession *soup_session, + const char *uri, + FlatpakHTTPFlags flags, + const char *token, + FlatpakLoadUriProgress progress, + gpointer user_data, + GCancellable *cancellable, + GError **error) +{ + g_autoptr(GError) local_error = NULL; + guint n_retries_remaining = DEFAULT_N_NETWORK_RETRIES; + + do + { + g_autoptr(GBytes) bytes = NULL; + + if (n_retries_remaining < DEFAULT_N_NETWORK_RETRIES) + { + g_clear_error (&local_error); + + if (progress) + progress (0, user_data); /* Reset the progress */ + } + + bytes = flatpak_load_http_uri_once (soup_session, uri, flags, + token, progress, user_data, + cancellable, &local_error); + + if (local_error == NULL) + return g_steal_pointer (&bytes); + } + while (flatpak_http_should_retry_request (local_error, n_retries_remaining--)); + + g_assert (local_error != NULL); + g_propagate_error (error, g_steal_pointer (&local_error)); + return NULL; +} + +static gboolean +flatpak_download_http_uri_once (SoupSession *soup_session, + const char *uri, + FlatpakHTTPFlags flags, + GOutputStream *out, + const char *token, + FlatpakLoadUriProgress progress, + gpointer user_data, + guint64 *out_bytes_written, + GCancellable *cancellable, + GError **error) { g_autoptr(SoupRequestHTTP) request = NULL; g_autoptr(GMainLoop) loop = NULL; @@ -627,6 +736,9 @@ flatpak_download_http_uri (SoupSession *soup_session, g_main_loop_run (loop); + if (out_bytes_written) + *out_bytes_written = data.downloaded_bytes; + if (data.error) { g_propagate_error (error, data.error); @@ -638,6 +750,54 @@ flatpak_download_http_uri (SoupSession *soup_session, return TRUE; } +gboolean +flatpak_download_http_uri (SoupSession *soup_session, + const char *uri, + FlatpakHTTPFlags flags, + GOutputStream *out, + const char *token, + FlatpakLoadUriProgress progress, + gpointer user_data, + GCancellable *cancellable, + GError **error) +{ + g_autoptr(GError) local_error = NULL; + guint n_retries_remaining = DEFAULT_N_NETWORK_RETRIES; + + do + { + guint64 bytes_written = 0; + + if (n_retries_remaining < DEFAULT_N_NETWORK_RETRIES) + { + g_clear_error (&local_error); + + if (progress) + progress (0, user_data); /* Reset the progress */ + } + + if (flatpak_download_http_uri_once (soup_session, uri, flags, + out, token, + progress, user_data, + &bytes_written, + cancellable, &local_error)) + { + g_assert (local_error == NULL); + return TRUE; + } + + /* If the output stream has already been written to we can't retry. + * TODO: use a range request to resume the download */ + if (bytes_written > 0) + break; + } + while (flatpak_http_should_retry_request (local_error, n_retries_remaining--)); + + g_assert (local_error != NULL); + g_propagate_error (error, g_steal_pointer (&local_error)); + return FALSE; +} + static gboolean sync_and_rename_tmpfile (GLnxTmpfile *tmpfile, const char *dest_name, @@ -670,16 +830,16 @@ sync_and_rename_tmpfile (GLnxTmpfile *tmpfile, return TRUE; } -gboolean -flatpak_cache_http_uri (SoupSession *soup_session, - const char *uri, - FlatpakHTTPFlags flags, - int dest_dfd, - const char *dest_subpath, - FlatpakLoadUriProgress progress, - gpointer user_data, - GCancellable *cancellable, - GError **error) +static gboolean +flatpak_cache_http_uri_once (SoupSession *soup_session, + const char *uri, + FlatpakHTTPFlags flags, + int dest_dfd, + const char *dest_subpath, + FlatpakLoadUriProgress progress, + gpointer user_data, + GCancellable *cancellable, + GError **error) { g_autoptr(SoupRequestHTTP) request = NULL; g_autoptr(GMainLoop) loop = NULL; @@ -834,3 +994,43 @@ flatpak_cache_http_uri (SoupSession *soup_session, return TRUE; } + +gboolean +flatpak_cache_http_uri (SoupSession *soup_session, + const char *uri, + FlatpakHTTPFlags flags, + int dest_dfd, + const char *dest_subpath, + FlatpakLoadUriProgress progress, + gpointer user_data, + GCancellable *cancellable, + GError **error) +{ + g_autoptr(GError) local_error = NULL; + guint n_retries_remaining = DEFAULT_N_NETWORK_RETRIES; + + do + { + if (n_retries_remaining < DEFAULT_N_NETWORK_RETRIES) + { + g_clear_error (&local_error); + + if (progress) + progress (0, user_data); /* Reset the progress */ + } + + if (flatpak_cache_http_uri_once (soup_session, uri, flags, + dest_dfd, dest_subpath, + progress, user_data, + cancellable, &local_error)) + { + g_assert (local_error == NULL); + return TRUE; + } + } + while (flatpak_http_should_retry_request (local_error, n_retries_remaining--)); + + g_assert (local_error != NULL); + g_propagate_error (error, g_steal_pointer (&local_error)); + return FALSE; +} diff --git a/oci-authenticator/flatpak-oci-authenticator.c b/oci-authenticator/flatpak-oci-authenticator.c index 0957a1cc..d98326ac 100644 --- a/oci-authenticator/flatpak-oci-authenticator.c +++ b/oci-authenticator/flatpak-oci-authenticator.c @@ -428,10 +428,12 @@ handle_request_ref_tokens (FlatpakAuthenticator *authenticator, g_autoptr(GError) error = NULL; g_autoptr(AutoFlatpakAuthenticatorRequest) request = NULL; const char *auth = NULL; + gboolean have_auth; const char *oci_registry_uri = NULL; gsize n_refs, i; gboolean no_interaction = FALSE; g_autoptr(FlatpakOciRegistry) registry = NULL; + g_autofree char *first_token = NULL; GVariantBuilder tokens; GVariantBuilder results; g_autofree char *sender = g_strdup (g_dbus_method_invocation_get_sender (invocation)); @@ -439,6 +441,7 @@ handle_request_ref_tokens (FlatpakAuthenticator *authenticator, g_debug ("handling Authenticator.RequestRefTokens"); g_variant_lookup (arg_authenticator_options, "auth", "&s", &auth); + have_auth = auth != NULL; if (!g_variant_lookup (arg_options, "xa.oci-registry-uri", "&s", &oci_registry_uri)) { @@ -476,18 +479,33 @@ handle_request_ref_tokens (FlatpakAuthenticator *authenticator, return error_request (request, sender, error->message); - if (auth == NULL) + /* Look up credentials in config files */ + if (!have_auth) { g_debug ("Looking for %s in auth info", oci_registry_uri); auth = lookup_auth_from_config (oci_registry_uri); + have_auth = auth != NULL; } + /* Try to see if we can get a token without presenting credentials */ n_refs = g_variant_n_children (arg_refs); - if (auth == NULL && n_refs > 0 && + if (!have_auth && n_refs > 0) + { + g_autoptr(GVariant) ref_data = g_variant_get_child_value (arg_refs, 0); + + first_token = get_token_for_ref (registry, ref_data, NULL, &error); + if (first_token != NULL) + have_auth = TRUE; + else + g_clear_error (&error); + } + + /* Prompt the user for credentials */ + n_refs = g_variant_n_children (arg_refs); + if (!have_auth && n_refs > 0 && !no_interaction) { g_autoptr(GVariant) ref_data = g_variant_get_child_value (arg_refs, 0); - g_autofree char *token = NULL; while (auth == NULL) { @@ -498,13 +516,21 @@ handle_request_ref_tokens (FlatpakAuthenticator *authenticator, if (test_auth == NULL) return cancel_request (request, sender); - token = get_token_for_ref (registry, ref_data, test_auth, &error); - if (token != NULL) - auth = g_steal_pointer (&test_auth); + first_token = get_token_for_ref (registry, ref_data, test_auth, &error); + if (first_token != NULL) + { + auth = g_steal_pointer (&test_auth); + have_auth = TRUE; + } + else + { + g_debug ("Failed to get token: %s", error->message); + g_clear_error (&error); + } } } - if (auth == NULL) + if (!have_auth) return error_request (request, sender, "No authentication information available"); g_variant_builder_init (&tokens, G_VARIANT_TYPE ("a{sas}")); @@ -515,9 +541,16 @@ handle_request_ref_tokens (FlatpakAuthenticator *authenticator, char *for_refs_strv[2] = { NULL, NULL}; g_autofree char *token = NULL; - token = get_token_for_ref (registry, ref_data, auth, &error); - if (token == NULL) - return error_request (request, sender, error->message); + if (i == 0 && first_token != NULL) + { + token = g_steal_pointer (&first_token); + } + else + { + token = get_token_for_ref (registry, ref_data, auth, &error); + if (token == NULL) + return error_request (request, sender, error->message); + } g_variant_get_child (ref_data, 0, "&s", &for_refs_strv[0]); g_variant_builder_add (&tokens, "{s^as}", token, for_refs_strv); diff --git a/tests/libtest.sh b/tests/libtest.sh index e64be49f..795c69e1 100644 --- a/tests/libtest.sh +++ b/tests/libtest.sh @@ -166,6 +166,16 @@ assert_file_has_content () { fi } +assert_log_has_gpg_signature_error () { + if ! grep -q -e "GPG signatures found, but none are in trusted keyring" "$1"; then + if ! grep -q -e "Can't check signature: public key not found" "$1"; then + sed -e 's/^/# /' < "$1" >&2 + echo 1>&2 "File '$1' doesn't have gpg signature error" + exit 1 + fi + fi +} + assert_symlink_has_content () { if ! readlink "$1" | grep -q -e "$2"; then readlink "$1" |sed -e 's/^/# /' >&2 diff --git a/tests/test-repo.sh b/tests/test-repo.sh index fd4b9337..c9192440 100644 --- a/tests/test-repo.sh +++ b/tests/test-repo.sh @@ -221,25 +221,24 @@ ${FLATPAK} ${U} uninstall -y org.test.Platform org.test.Hello if ${FLATPAK} ${U} install -y test-missing-gpg-repo org.test.Platform 2> install-error-log; then assert_not_reached "Should not be able to install with missing gpg key" fi -assert_file_has_content install-error-log "GPG signatures found, but none are in trusted keyring" - +assert_log_has_gpg_signature_error install-error-log if ${FLATPAK} ${U} install test-missing-gpg-repo org.test.Hello 2> install-error-log; then assert_not_reached "Should not be able to install with missing gpg key" fi -assert_file_has_content install-error-log "GPG signatures found, but none are in trusted keyring" +assert_log_has_gpg_signature_error install-error-log echo "ok fail with missing gpg key" if ${FLATPAK} ${U} install test-wrong-gpg-repo org.test.Platform 2> install-error-log; then assert_not_reached "Should not be able to install with wrong gpg key" fi -assert_file_has_content install-error-log "GPG signatures found, but none are in trusted keyring" +assert_log_has_gpg_signature_error install-error-log if ${FLATPAK} ${U} install test-wrong-gpg-repo org.test.Hello 2> install-error-log; then assert_not_reached "Should not be able to install with wrong gpg key" fi -assert_file_has_content install-error-log "GPG signatures found, but none are in trusted keyring" +assert_log_has_gpg_signature_error install-error-log echo "ok fail with wrong gpg key"