From ceeaa07202f7dd83f6835e84c7c787f3b8c36847 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 27 Feb 2020 12:58:30 +0000 Subject: [PATCH 01/10] run: Cope with the primary gid not being in the nsswitch database If it's an opaque integer on the host system, it might as well be an opaque integer in the container too. Fixes: #3416 Signed-off-by: Simon McVittie (cherry picked from commit a36e0183b1bb8a8cbb51ba989b53dacb855bec88) --- common/flatpak-run.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) 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)) From ad27a7e5086b1987c160db2e952c4a49762e9cde Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Mon, 16 Mar 2020 10:47:41 +0100 Subject: [PATCH 02/10] repair: Don't crash if no remotes are configured If no remotes are configured, ostree_repo_remote_list returns NULL so don't dereference it. Fixes: https://github.com/flatpak/flatpak/issues/3436 (cherry picked from commit e2ee3306b705da77e98665d287b7c0150f7a18cf) --- common/flatpak-dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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; From 346d42d90da2f5a3fbe3cd15385962767c8e8704 Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Fri, 13 Mar 2020 13:38:56 -0400 Subject: [PATCH 03/10] flatpak-oci-authenticator: try getting a token without credentials Some registries require getting a token even to download an image anonymously. So, if no auth has been configured, before prompting the user for username/password, try without a BasicAuth header. Signed-off-by: Owen W. Taylor (cherry picked from commit fe3f17a89a559c38d9e98869190f02d1c4b3f089) --- common/flatpak-oci-registry.c | 7 +++-- oci-authenticator/flatpak-oci-authenticator.c | 28 ++++++++++++++++--- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/common/flatpak-oci-registry.c b/common/flatpak-oci-registry.c index 420efff0..94d4eb05 100644 --- a/common/flatpak-oci-registry.c +++ b/common/flatpak-oci-registry.c @@ -949,8 +949,11 @@ get_token_for_www_auth (FlatpakOciRegistry *self, 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) diff --git a/oci-authenticator/flatpak-oci-authenticator.c b/oci-authenticator/flatpak-oci-authenticator.c index 0957a1cc..da44914d 100644 --- a/oci-authenticator/flatpak-oci-authenticator.c +++ b/oci-authenticator/flatpak-oci-authenticator.c @@ -428,6 +428,7 @@ 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; @@ -439,6 +440,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,14 +478,29 @@ 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); + g_autofree char *token = NULL; + + token = get_token_for_ref (registry, ref_data, NULL, &error); + if (token != NULL) + have_auth = TRUE; + } + + /* 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); @@ -500,11 +517,14 @@ handle_request_ref_tokens (FlatpakAuthenticator *authenticator, token = get_token_for_ref (registry, ref_data, test_auth, &error); if (token != NULL) - auth = g_steal_pointer (&test_auth); + { + auth = g_steal_pointer (&test_auth); + have_auth = TRUE; + } } } - 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}")); From 07ed998ea5c8410d584a1fe92546249b1da151b4 Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Fri, 13 Mar 2020 13:33:54 -0400 Subject: [PATCH 04/10] flatpak-oci-registry.c: supply a default scope when getting a token If no scope parameter is supplied in the WWW-Authenticate header, docker and libpod will make up their own of the form repository::pull when requesting a bearer token. Match that. Signed-off-by: Owen W. Taylor (cherry picked from commit f7616a8b3c2c0db8f1c97be87faaadf01c7011ff) --- common/flatpak-oci-registry.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/common/flatpak-oci-registry.c b/common/flatpak-oci-registry.c index 94d4eb05..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,9 +943,11 @@ 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); @@ -1033,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; From f4f387a5098be60bfe661037683d0903031704ac Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Mon, 16 Mar 2020 16:22:04 -0400 Subject: [PATCH 05/10] oci-authenticator: reuse token results when we already have them When we already have a token for the first repository after probing for no-auth authenticator or testing user-entered credentials, just use that, don't request it again in the loop over repositories. This gives a significant optimization of the prompted-credentials case for registry.redhat.io, which takes 4-5 seconds to generate a token, hopefully avoiding the user thinking something has gone wrong. Signed-off-by: Owen W. Taylor (cherry picked from commit 08636d47299fb4ed47075c2e5f77da3d0440df9c) --- oci-authenticator/flatpak-oci-authenticator.c | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/oci-authenticator/flatpak-oci-authenticator.c b/oci-authenticator/flatpak-oci-authenticator.c index da44914d..d351fc57 100644 --- a/oci-authenticator/flatpak-oci-authenticator.c +++ b/oci-authenticator/flatpak-oci-authenticator.c @@ -433,6 +433,7 @@ handle_request_ref_tokens (FlatpakAuthenticator *authenticator, 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)); @@ -491,10 +492,9 @@ handle_request_ref_tokens (FlatpakAuthenticator *authenticator, if (!have_auth && n_refs > 0) { g_autoptr(GVariant) ref_data = g_variant_get_child_value (arg_refs, 0); - g_autofree char *token = NULL; - token = get_token_for_ref (registry, ref_data, NULL, &error); - if (token != NULL) + first_token = get_token_for_ref (registry, ref_data, NULL, &error); + if (first_token != NULL) have_auth = TRUE; } @@ -504,7 +504,6 @@ handle_request_ref_tokens (FlatpakAuthenticator *authenticator, !no_interaction) { g_autoptr(GVariant) ref_data = g_variant_get_child_value (arg_refs, 0); - g_autofree char *token = NULL; while (auth == NULL) { @@ -515,8 +514,8 @@ 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) + 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; @@ -535,9 +534,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); From 53142f3d6a65cccba57b1898db07ae33333c1ffa Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Mon, 16 Mar 2020 15:58:16 -0400 Subject: [PATCH 06/10] oci-authenticator: fix failures to clear GError Fix problems overwriting a GError when we retry multiple times. One of these was introduced with the recent change e3f17a89a flatpak-oci-authenticator: try getting a token without credentials but the other was existing. Signed-off-by: Owen W. Taylor (cherry picked from commit 425f628263476208925c271bfa0dcc01ee3c8b5a) --- oci-authenticator/flatpak-oci-authenticator.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/oci-authenticator/flatpak-oci-authenticator.c b/oci-authenticator/flatpak-oci-authenticator.c index d351fc57..d98326ac 100644 --- a/oci-authenticator/flatpak-oci-authenticator.c +++ b/oci-authenticator/flatpak-oci-authenticator.c @@ -496,6 +496,8 @@ handle_request_ref_tokens (FlatpakAuthenticator *authenticator, 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 */ @@ -520,6 +522,11 @@ handle_request_ref_tokens (FlatpakAuthenticator *authenticator, auth = g_steal_pointer (&test_auth); have_auth = TRUE; } + else + { + g_debug ("Failed to get token: %s", error->message); + g_clear_error (&error); + } } } From ad803b986248eb3ae460d3b77f3ff59c9ff869ee Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Thu, 19 Mar 2020 16:33:49 -0700 Subject: [PATCH 07/10] flatpak-utils-http.c: Use more specific GIOError codes Instead of defaulting to G_IO_ERROR_FAILED, use more specific codes when we can. These were copied from libostree. (cherry picked from commit 0b25455af11ee8588f8e31da3d44849f8934160e) --- common/flatpak-utils-http.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/common/flatpak-utils-http.c b/common/flatpak-utils-http.c index 2ff30366..f965d798 100644 --- a/common/flatpak-utils-http.c +++ b/common/flatpak-utils-http.c @@ -432,11 +432,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; } From 8185143423f1675af8dc599bf9a88ce224828936 Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Thu, 19 Mar 2020 18:11:02 -0700 Subject: [PATCH 08/10] flatpak-utils-http.c: Add retry logic for transient failures Currently if flatpak is installing an extra data app such as Spotify and the server with the .deb file fails to complete the request, the installation fails with a message like "Connection terminated unexpectedly". This commit makes flatpak instead try 5 times to download a given URI if the error returned seems like a transient one (so not, for example, 404 not found). This is analogous to what was done in libostree in commit 938055392fd455027a69398c441b992ae521aa87, and we use some code from there. (cherry picked from commit 5560132ba6ab654316117ce7b40a754b12cb38e9) --- common/flatpak-utils-http.c | 236 +++++++++++++++++++++++++++++++----- 1 file changed, 207 insertions(+), 29 deletions(-) diff --git a/common/flatpak-utils-http.c b/common/flatpak-utils-http.c index f965d798..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; @@ -532,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; @@ -598,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; @@ -649,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); @@ -660,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, @@ -692,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; @@ -856,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; +} From 3ab23776b3538577b3aad526b56769cfcea65c81 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Mon, 16 Mar 2020 15:01:54 +0100 Subject: [PATCH 09/10] tests: Fix gpg signature failure checks It seems recent ostree reports a different error string for signature check failures. (cherry picked from commit ab5f2dd7e8885268aeb9369615af8647755c060d) --- tests/libtest.sh | 10 ++++++++++ tests/test-repo.sh | 9 ++++----- 2 files changed, 14 insertions(+), 5 deletions(-) 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" From 4e4b9f936a8fb30e8bb95543eedeb1d2d715621d Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Mon, 30 Mar 2020 13:34:27 +0200 Subject: [PATCH 10/10] CI: Run on flatpak-1.6.x branch --- .github/workflows/check.yml | 2 ++ 1 file changed, 2 insertions(+) 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: