From 63ec26a4bfdc6376119bba4378d9f2463858f376 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2021 22:13:46 +0100 Subject: [PATCH 01/11] fs-util/rm-rf: improve remove+free destructors to take and return NULL Let#s make these helpers useful even without _cleanup_ logic, to destory arbitary fields: make them OK wiht a NULL pointer as input, and always return one as output. --- src/basic/fs-util.h | 13 +++++++++++-- src/basic/rm-rf.h | 14 ++++++++++++-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h index 13b68729422..8bd8da17046 100644 --- a/src/basic/fs-util.h +++ b/src/basic/fs-util.h @@ -100,16 +100,25 @@ int chase_symlinks_and_opendir(const char *path, const char *root, unsigned chas int chase_symlinks_and_stat(const char *path, const char *root, unsigned chase_flags, char **ret_path, struct stat *ret_stat, int *ret_fd); /* Useful for usage with _cleanup_(), removes a directory and frees the pointer */ -static inline void rmdir_and_free(char *p) { +static inline char *rmdir_and_free(char *p) { PROTECT_ERRNO; + + if (!p) + return NULL; + (void) rmdir(p); free(p); + return NULL; } DEFINE_TRIVIAL_CLEANUP_FUNC(char*, rmdir_and_free); -static inline void unlink_and_free(char *p) { +static inline char* unlink_and_free(char *p) { + if (!p) + return NULL; + (void) unlink_noerrno(p); free(p); + return NULL; } DEFINE_TRIVIAL_CLEANUP_FUNC(char*, unlink_and_free); diff --git a/src/basic/rm-rf.h b/src/basic/rm-rf.h index ec56232b5dc..6483b30d7e0 100644 --- a/src/basic/rm-rf.h +++ b/src/basic/rm-rf.h @@ -18,17 +18,27 @@ int rm_rf_children(int fd, RemoveFlags flags, struct stat *root_dev); int rm_rf(const char *path, RemoveFlags flags); /* Useful for usage with _cleanup_(), destroys a directory and frees the pointer */ -static inline void rm_rf_physical_and_free(char *p) { +static inline char *rm_rf_physical_and_free(char *p) { PROTECT_ERRNO; + + if (!p) + return NULL; + (void) rm_rf(p, REMOVE_ROOT|REMOVE_PHYSICAL); free(p); + return NULL; } DEFINE_TRIVIAL_CLEANUP_FUNC(char*, rm_rf_physical_and_free); /* Similar as above, but also has magic btrfs subvolume powers */ -static inline void rm_rf_subvolume_and_free(char *p) { +static inline char *rm_rf_subvolume_and_free(char *p) { PROTECT_ERRNO; + + if (!p) + return NULL; + (void) rm_rf(p, REMOVE_ROOT|REMOVE_PHYSICAL|REMOVE_SUBVOLUME); free(p); + return NULL; } DEFINE_TRIVIAL_CLEANUP_FUNC(char*, rm_rf_subvolume_and_free); From 7d41de2e948ce59af08b7193856bb9d4fcd931a8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2021 16:47:41 +0100 Subject: [PATCH 02/11] import: comment indent fix --- src/import/pull-job.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/import/pull-job.h b/src/import/pull-job.h index 719196caec9..70ef62699ab 100644 --- a/src/import/pull-job.h +++ b/src/import/pull-job.h @@ -17,7 +17,7 @@ typedef void (*PullJobProgress)(PullJob *job); typedef enum PullJobState { PULL_JOB_INIT, PULL_JOB_ANALYZING, /* Still reading into ->payload, to figure out what we have */ - PULL_JOB_RUNNING, /* Writing to destination */ + PULL_JOB_RUNNING, /* Writing to destination */ PULL_JOB_DONE, PULL_JOB_FAILED, _PULL_JOB_STATE_MAX, From c6cb8daf723978b98c4d6f0b627ad3fdc4954c53 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2021 16:42:45 +0100 Subject: [PATCH 03/11] import: make scope of variable smaller --- src/import/pull-common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/import/pull-common.c b/src/import/pull-common.c index 33be609aec5..243cf7cb858 100644 --- a/src/import/pull-common.c +++ b/src/import/pull-common.c @@ -255,7 +255,6 @@ int pull_make_verification_jobs( _cleanup_(pull_job_unrefp) PullJob *checksum_job = NULL, *signature_job = NULL; int r; - const char *chksums = NULL; assert(ret_checksum_job); assert(ret_signature_job); @@ -266,6 +265,7 @@ int pull_make_verification_jobs( if (verify != IMPORT_VERIFY_NO) { _cleanup_free_ char *checksum_url = NULL, *fn = NULL; + const char *chksums = NULL; /* Queue jobs for the checksum file for the image. */ r = import_url_last_component(url, &fn); From c20307fd347da5f2d6cfe7fad3ae64450ffec818 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2021 16:42:58 +0100 Subject: [PATCH 04/11] import: use TAKE_PTR() where available --- src/import/pull-common.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/import/pull-common.c b/src/import/pull-common.c index 243cf7cb858..4631431707c 100644 --- a/src/import/pull-common.c +++ b/src/import/pull-common.c @@ -302,10 +302,8 @@ int pull_make_verification_jobs( signature_job->uncompressed_max = signature_job->compressed_max = 1ULL * 1024ULL * 1024ULL; } - *ret_checksum_job = checksum_job; - *ret_signature_job = signature_job; - - checksum_job = signature_job = NULL; + *ret_checksum_job = TAKE_PTR(checksum_job); + *ret_signature_job = TAKE_PTR(signature_job); return 0; } From f14717a7e2d9331010a091baeae6cf9e99f4bb5c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2021 16:45:29 +0100 Subject: [PATCH 05/11] import: rework how verification works Previously the PullJob object took internal care of rerequested the SHA256SUMS file, if requesting .sha256 didn't work. This was a weird a non-abstraction only used when actually getting the checksum files. Let's move this out of the PullJob, so that it is generic again, and does roughly the same stuff for all resources it is used for: let's define a generic .on_not_found() handler that can be set on a PullJob object, and is called whenever with see HTTP 404, and may be used to provide a new URL to try if the first didn't work. This is also preparation for later work to support PKCS#7 signatures instead of gpg signatures, where a similar logic is needed, and we thus should have a generic infrastructure place. This gets rid of the VerificationStyle field in the PullJob object: instead of storing this non-generic field we just derive the same information from the URL itself, which is safe, since we generated it ourselves earlier. --- src/import/pull-common.c | 63 ++++++++++++++++++++++++++++++++++++++-- src/import/pull-common.h | 11 +++++++ src/import/pull-job.c | 43 +++++++++++++++------------ src/import/pull-job.h | 10 ++----- src/import/pull-raw.c | 22 ++++++++++---- src/import/pull-tar.c | 22 ++++++++++---- 6 files changed, 133 insertions(+), 38 deletions(-) diff --git a/src/import/pull-common.c b/src/import/pull-common.c index 4631431707c..faa988b5f2a 100644 --- a/src/import/pull-common.c +++ b/src/import/pull-common.c @@ -374,6 +374,7 @@ int pull_verify(PullJob *main_job, char sig_file_path[] = "/tmp/sigXXXXXX", gpg_home[] = "/tmp/gpghomeXXXXXX"; _cleanup_(sigkill_waitp) pid_t pid = 0; bool gpg_home_created = false; + VerificationStyle style; int r; assert(main_job); @@ -406,7 +407,13 @@ int pull_verify(PullJob *main_job, if (!signature_job) return 0; - if (checksum_job->style == VERIFICATION_PER_FILE) + r = verification_style_from_url(checksum_job->url, &style); + if (r < 0) + return log_error_errno(r, "Failed to determine verification style from URL '%s': %m", checksum_job->url); + + if (style == VERIFICATION_PER_FILE) /* if this is per-file verification, then the signature is in the + * checksum job, let's thus ignore the signature job from now on, + * and use the checksum job as signature job. */ signature_job = checksum_job; assert(signature_job->state == PULL_JOB_DONE); @@ -480,7 +487,7 @@ int pull_verify(PullJob *main_job, cmd[k++] = "--keyring=" VENDOR_KEYRING_PATH; cmd[k++] = "--verify"; - if (checksum_job->style == VERIFICATION_PER_DIRECTORY) { + if (style == VERIFICATION_PER_DIRECTORY) { cmd[k++] = sig_file_path; cmd[k++] = "-"; cmd[k++] = NULL; @@ -522,3 +529,55 @@ finish: return r; } + +int verification_style_from_url(const char *url, VerificationStyle *ret) { + _cleanup_free_ char *last = NULL; + int r; + + assert(url); + assert(ret); + + /* Determines which kind of verification style is appropriate for this url */ + + r = import_url_last_component(url, &last); + if (r < 0) + return r; + + if (streq(last, "SHA256SUMS")) { + *ret = VERIFICATION_PER_DIRECTORY; + return 0; + } + + if (endswith(last, ".sha256")) { + *ret = VERIFICATION_PER_FILE; + return 0; + } + + return -EINVAL; +} + +int pull_job_restart_with_sha256sum(PullJob *j, char **ret) { + VerificationStyle style; + int r; + + assert(j); + + /* Generic implementation of a PullJobNotFound handler, that restarts the job requesting SHA256SUMS */ + + r = verification_style_from_url(j->url, &style); + if (r < 0) + return log_error_errno(r, "Failed to determine verification style of URL '%s': %m", j->url); + + if (style == VERIFICATION_PER_DIRECTORY) /* Nothing to do anymore */ + return 0; + + assert(style == VERIFICATION_PER_FILE); /* This must have been .sha256 style URL before */ + + log_debug("Got 404 for %s, now trying to get SHA256SUMS instead.", j->url); + + r = import_url_change_last_component(j->url, "SHA256SUMS", ret); + if (r < 0) + return log_error_errno(r, "Failed to replace SHA256SUMS suffix: %m"); + + return 1; +} diff --git a/src/import/pull-common.h b/src/import/pull-common.h index 025bcee2bd7..a83e9b7e145 100644 --- a/src/import/pull-common.h +++ b/src/import/pull-common.h @@ -16,3 +16,14 @@ int pull_make_auxiliary_job(PullJob **ret, const char *url, int (*strip_suffixes int pull_make_verification_jobs(PullJob **ret_checksum_job, PullJob **ret_signature_job, ImportVerify verify, const char *url, CurlGlue *glue, PullJobFinished on_finished, void *userdata); int pull_verify(PullJob *main_job, PullJob *roothash_job, PullJob *settings_job, PullJob *checksum_job, PullJob *signature_job); + +typedef enum VerificationStyle { + VERIFICATION_PER_FILE, /* SuSE-style ".sha256" files with inline gpg signature */ + VERIFICATION_PER_DIRECTORY, /* Ubuntu-style SHA256SUM files with detached SHA256SUM.gpg signatures */ + _VERIFICATION_STYLE_MAX, + _VERIFICATION_STYLE_INVALID = -1, +} VerificationStyle; + +int verification_style_from_url(const char *url, VerificationStyle *style); + +int pull_job_restart_with_sha256sum(PullJob *job, char **ret); diff --git a/src/import/pull-job.c b/src/import/pull-job.c index eea00380a49..2f6bca83365 100644 --- a/src/import/pull-job.c +++ b/src/import/pull-job.c @@ -61,16 +61,16 @@ static void pull_job_finish(PullJob *j, int ret) { j->on_finished(j); } -static int pull_job_restart(PullJob *j) { +static int pull_job_restart(PullJob *j, const char *new_url) { int r; - char *chksum_url = NULL; - r = import_url_change_last_component(j->url, "SHA256SUMS", &chksum_url); + assert(j); + assert(new_url); + + r = free_and_strdup(&j->url, new_url); if (r < 0) return r; - free(j->url); - j->url = chksum_url; j->state = PULL_JOB_INIT; j->payload = mfree(j->payload); j->payload_size = 0; @@ -114,23 +114,31 @@ void pull_job_curl_on_finished(CurlGlue *g, CURL *curl, CURLcode result) { r = 0; goto finish; } else if (status >= 300) { - if (status == 404 && j->style == VERIFICATION_PER_FILE) { - /* retry pull job with SHA256SUMS file */ - r = pull_job_restart(j); + if (status == 404 && j->on_not_found) { + _cleanup_free_ char *new_url = NULL; + + /* This resource wasn't found, but the implementor wants to maybe let us know a new URL, query for it. */ + r = j->on_not_found(j, &new_url); if (r < 0) goto finish; - code = curl_easy_getinfo(j->curl, CURLINFO_RESPONSE_CODE, &status); - if (code != CURLE_OK) { - log_error("Failed to retrieve response code: %s", curl_easy_strerror(code)); - r = -EIO; - goto finish; - } + if (r > 0) { /* A new url to use */ + assert(new_url); - if (status == 0) { - j->style = VERIFICATION_PER_DIRECTORY; - return; + r = pull_job_restart(j, new_url); + if (r < 0) + goto finish; + + code = curl_easy_getinfo(j->curl, CURLINFO_RESPONSE_CODE, &status); + if (code != CURLE_OK) { + log_error("Failed to retrieve response code: %s", curl_easy_strerror(code)); + r = -EIO; + goto finish; + } + + if (status == 0) + return; } } @@ -556,7 +564,6 @@ int pull_job_new(PullJob **ret, const char *url, CurlGlue *glue, void *userdata) .start_usec = now(CLOCK_MONOTONIC), .compressed_max = 64LLU * 1024LLU * 1024LLU * 1024LLU, /* 64GB safety limit */ .uncompressed_max = 64LLU * 1024LLU * 1024LLU * 1024LLU, /* 64GB safety limit */ - .style = VERIFICATION_STYLE_UNSET, .url = TAKE_PTR(u), }; diff --git a/src/import/pull-job.h b/src/import/pull-job.h index 70ef62699ab..48cc773beb6 100644 --- a/src/import/pull-job.h +++ b/src/import/pull-job.h @@ -13,6 +13,7 @@ typedef void (*PullJobFinished)(PullJob *job); typedef int (*PullJobOpenDisk)(PullJob *job); typedef int (*PullJobHeader)(PullJob *job, const char *header, size_t sz); typedef void (*PullJobProgress)(PullJob *job); +typedef int (*PullJobNotFound)(PullJob *job, char **ret_new_url); typedef enum PullJobState { PULL_JOB_INIT, @@ -24,12 +25,6 @@ typedef enum PullJobState { _PULL_JOB_STATE_INVALID = -1, } PullJobState; -typedef enum VerificationStyle { - VERIFICATION_STYLE_UNSET, - VERIFICATION_PER_FILE, /* SuSE-style ".sha256" files with inline signature */ - VERIFICATION_PER_DIRECTORY, /* Ubuntu-style SHA256SUM files with detach SHA256SUM.gpg signatures */ -} VerificationStyle; - #define PULL_JOB_IS_COMPLETE(j) (IN_SET((j)->state, PULL_JOB_DONE, PULL_JOB_FAILED)) struct PullJob { @@ -43,6 +38,7 @@ struct PullJob { PullJobOpenDisk on_open_disk; PullJobHeader on_header; PullJobProgress on_progress; + PullJobNotFound on_not_found; CurlGlue *glue; CURL *curl; @@ -79,8 +75,6 @@ struct PullJob { gcry_md_hd_t checksum_context; char *checksum; - - VerificationStyle style; }; int pull_job_new(PullJob **job, const char *url, CurlGlue *glue, void *userdata); diff --git a/src/import/pull-raw.c b/src/import/pull-raw.c index 4a2bab3d073..69b3c307f62 100644 --- a/src/import/pull-raw.c +++ b/src/import/pull-raw.c @@ -473,11 +473,23 @@ static void raw_pull_job_on_finished(PullJob *j) { if (!raw_pull_is_done(i)) return; - if (i->signature_job && i->checksum_job->style == VERIFICATION_PER_DIRECTORY && i->signature_job->error != 0) { - log_error_errno(j->error, "Failed to retrieve signature file, cannot verify. (Try --verify=no?)"); + if (i->signature_job && i->signature_job->error != 0) { + VerificationStyle style; - r = i->signature_job->error; - goto finish; + r = verification_style_from_url(i->checksum_job->url, &style); + if (r < 0) { + log_error_errno(r, "Failed to determine verification style from checksum URL: %m"); + goto finish; + } + + if (style == VERIFICATION_PER_DIRECTORY) { /* A failed signature file download only matters + * in per-directory verification mode, since only + * then the signature is detached, and thus a file + * of its own. */ + log_error_errno(j->error, "Failed to retrieve signature file, cannot verify. (Try --verify=no?)"); + r = i->signature_job->error; + goto finish; + } } if (i->roothash_job) @@ -722,7 +734,7 @@ int raw_pull_start( if (i->checksum_job) { i->checksum_job->on_progress = raw_pull_job_on_progress; - i->checksum_job->style = VERIFICATION_PER_FILE; + i->checksum_job->on_not_found = pull_job_restart_with_sha256sum; r = pull_job_begin(i->checksum_job); if (r < 0) diff --git a/src/import/pull-tar.c b/src/import/pull-tar.c index b646d38539e..728b0fe6855 100644 --- a/src/import/pull-tar.c +++ b/src/import/pull-tar.c @@ -302,11 +302,23 @@ static void tar_pull_job_on_finished(PullJob *j) { if (!tar_pull_is_done(i)) return; - if (i->signature_job && i->checksum_job->style == VERIFICATION_PER_DIRECTORY && i->signature_job->error != 0) { - log_error_errno(j->error, "Failed to retrieve signature file, cannot verify. (Try --verify=no?)"); + if (i->signature_job && i->signature_job->error != 0) { + VerificationStyle style; - r = i->signature_job->error; - goto finish; + r = verification_style_from_url(i->checksum_job->url, &style); + if (r < 0) { + log_error_errno(r, "Failed to determine verification style from checksum URL: %m"); + goto finish; + } + + if (style == VERIFICATION_PER_DIRECTORY) { /* A failed signature file download only matters + * in per-directory verification mode, since only + * then the signature is detached, and thus a file + * of its own. */ + log_error_errno(j->error, "Failed to retrieve signature file, cannot verify. (Try --verify=no?)"); + r = i->signature_job->error; + goto finish; + } } i->tar_job->disk_fd = safe_close(i->tar_job->disk_fd); @@ -540,7 +552,7 @@ int tar_pull_start( if (i->checksum_job) { i->checksum_job->on_progress = tar_pull_job_on_progress; - i->checksum_job->style = VERIFICATION_PER_FILE; + i->checksum_job->on_not_found = pull_job_restart_with_sha256sum; r = pull_job_begin(i->checksum_job); if (r < 0) From 8bc3f0b89f8bdf292a2d76108d45b7d5c29d145f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2021 21:22:27 +0100 Subject: [PATCH 06/11] import: reset PullJob properly Properly reset all fields that have to do with the current GET job when we restart things. Previously we freed/reset only some stuff, leaking some memory even. --- src/import/pull-job.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/import/pull-job.c b/src/import/pull-job.c index 2f6bca83365..b5fb0193ec1 100644 --- a/src/import/pull-job.c +++ b/src/import/pull-job.c @@ -72,11 +72,30 @@ static int pull_job_restart(PullJob *j, const char *new_url) { return r; j->state = PULL_JOB_INIT; + j->error = 0; j->payload = mfree(j->payload); j->payload_size = 0; j->payload_allocated = 0; j->written_compressed = 0; j->written_uncompressed = 0; + j->content_length = UINT64_MAX; + j->etag = mfree(j->etag); + j->etag_exists = false; + j->mtime = 0; + j->checksum = mfree(j->checksum); + + curl_glue_remove_and_free(j->glue, j->curl); + j->curl = NULL; + + curl_slist_free_all(j->request_header); + j->request_header = NULL; + + import_compress_free(&j->compress); + + if (j->checksum_context) { + gcry_md_close(j->checksum_context); + j->checksum_context = NULL; + } r = pull_job_begin(j); if (r < 0) From 273cb07d1b38061f816ccac23fd583e9252430fd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2021 21:23:20 +0100 Subject: [PATCH 07/11] import: small memory management simplification --- src/import/pull-job.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/import/pull-job.c b/src/import/pull-job.c index b5fb0193ec1..f41a7e7a5d0 100644 --- a/src/import/pull-job.c +++ b/src/import/pull-job.c @@ -434,10 +434,9 @@ fail: } static size_t pull_job_header_callback(void *contents, size_t size, size_t nmemb, void *userdata) { + _cleanup_free_ char *length = NULL, *last_modified = NULL, *etag = NULL; PullJob *j = userdata; size_t sz = size * nmemb; - _cleanup_free_ char *length = NULL, *last_modified = NULL; - char *etag; int r; assert(contents); @@ -456,8 +455,7 @@ static size_t pull_job_header_callback(void *contents, size_t size, size_t nmemb goto fail; } if (r > 0) { - free(j->etag); - j->etag = etag; + free_and_replace(j->etag, etag); if (strv_contains(j->old_etags, j->etag)) { log_info("Image already downloaded. Skipping download."); From 8dc0291c0dafa9c49a79fa4c561c645056aeb12b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2021 22:54:42 +0100 Subject: [PATCH 08/11] import: turn on HTTP logging in debug mode --- src/import/curl-util.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/import/curl-util.c b/src/import/curl-util.c index 5e0904379e1..e6db8106355 100644 --- a/src/import/curl-util.c +++ b/src/import/curl-util.c @@ -231,7 +231,8 @@ int curl_glue_make(CURL **ret, const char *url, void *userdata) { if (!c) return -ENOMEM; - /* curl_easy_setopt(c, CURLOPT_VERBOSE, 1L); */ + if (DEBUG_LOGGING) + (void) curl_easy_setopt(c, CURLOPT_VERBOSE, 1L); if (curl_easy_setopt(c, CURLOPT_URL, url) != CURLE_OK) return -EIO; From 6792cbbcf84b730f465decbeaf247c6b1ccf1c18 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2021 22:55:15 +0100 Subject: [PATCH 09/11] import: ignore non-successful HTTP codes for collecing image metadata Previously we'd collect the data from redirects too, which wasn't particularly terrible, since these typically don't carry the data we were interested in, but it's still incorrect to do so. --- src/import/pull-job.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/import/pull-job.c b/src/import/pull-job.c index f41a7e7a5d0..d1e61ba601f 100644 --- a/src/import/pull-job.c +++ b/src/import/pull-job.c @@ -435,8 +435,10 @@ fail: static size_t pull_job_header_callback(void *contents, size_t size, size_t nmemb, void *userdata) { _cleanup_free_ char *length = NULL, *last_modified = NULL, *etag = NULL; - PullJob *j = userdata; size_t sz = size * nmemb; + PullJob *j = userdata; + CURLcode code; + long status; int r; assert(contents); @@ -449,6 +451,18 @@ static size_t pull_job_header_callback(void *contents, size_t size, size_t nmemb assert(j->state == PULL_JOB_ANALYZING); + code = curl_easy_getinfo(j->curl, CURLINFO_RESPONSE_CODE, &status); + if (code != CURLE_OK) { + log_error("Failed to retrieve response code: %s", curl_easy_strerror(code)); + r = -EIO; + goto fail; + } + + if (status < 200 || status >= 300) + /* If this is not HTTP 2xx, let's skip these headers, they are probably for + * some redirect or so, and we are not interested in the headers of those. */ + return sz; + r = curl_header_strdup(contents, sz, "ETag:", &etag); if (r < 0) { log_oom(); From 133b34f69a72dc90d4e336837d699245390c9f50 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2021 23:18:54 +0100 Subject: [PATCH 10/11] import: optionally pull .verity + .roothash.p7s data when downloading We already had support for downlading a .nspawn and a .roothash file, let's make the set complete, and also download .verity + roothash.p7s if it exists, as nspawn consumes that. Since there are now four kinds of additional resources to acquire, let's introduce a PullFlags flags value for this instead of separate 'bool' variables, it's just too many to always pass those around on the function parameter list. --- src/import/pull-common.c | 4 +- src/import/pull-common.h | 14 ++- src/import/pull-raw.c | 252 ++++++++++++++++++++++++++------------- src/import/pull-raw.h | 3 +- src/import/pull-tar.c | 74 +++++------- src/import/pull-tar.h | 3 +- src/import/pull.c | 58 ++++++--- 7 files changed, 262 insertions(+), 146 deletions(-) diff --git a/src/import/pull-common.c b/src/import/pull-common.c index faa988b5f2a..8f295090038 100644 --- a/src/import/pull-common.c +++ b/src/import/pull-common.c @@ -110,7 +110,7 @@ int pull_find_old_etags( return 0; } -int pull_make_local_copy(const char *final, const char *image_root, const char *local, bool force_local) { +int pull_make_local_copy(const char *final, const char *image_root, const char *local, PullFlags flags) { const char *p; int r; @@ -122,7 +122,7 @@ int pull_make_local_copy(const char *final, const char *image_root, const char * p = prefix_roota(image_root, local); - if (force_local) + if (FLAGS_SET(flags, PULL_FORCE)) (void) rm_rf(p, REMOVE_ROOT|REMOVE_PHYSICAL|REMOVE_SUBVOLUME); r = btrfs_subvol_snapshot(final, p, diff --git a/src/import/pull-common.h b/src/import/pull-common.h index a83e9b7e145..f24093e6e11 100644 --- a/src/import/pull-common.h +++ b/src/import/pull-common.h @@ -6,7 +6,19 @@ #include "import-util.h" #include "pull-job.h" -int pull_make_local_copy(const char *final, const char *root, const char *local, bool force_local); +typedef enum PullFlags { + PULL_FORCE = 1 << 0, /* replace existing image */ + PULL_SETTINGS = 1 << 1, /* .nspawn settings file */ + PULL_ROOTHASH = 1 << 2, /* only for raw: .roothash file for verity */ + PULL_ROOTHASH_SIGNATURE = 1 << 3, /* only for raw: .roothash.p7s file for verity */ + PULL_VERITY = 1 << 4, /* only for raw: .verity file for verity */ + + /* The supported flags for the tar and the raw pulling */ + PULL_FLAGS_MASK_TAR = PULL_FORCE|PULL_SETTINGS, + PULL_FLAGS_MASK_RAW = PULL_FORCE|PULL_SETTINGS|PULL_ROOTHASH|PULL_ROOTHASH_SIGNATURE|PULL_VERITY, +} PullFlags; + +int pull_make_local_copy(const char *final, const char *root, const char *local, PullFlags flags); int pull_find_old_etags(const char *url, const char *root, int dt, const char *prefix, const char *suffix, char ***etags); diff --git a/src/import/pull-raw.c b/src/import/pull-raw.c index 69b3c307f62..5b4525d456c 100644 --- a/src/import/pull-raw.c +++ b/src/import/pull-raw.c @@ -42,21 +42,22 @@ struct RawPull { sd_event *event; CurlGlue *glue; + PullFlags flags; + ImportVerify verify; char *image_root; PullJob *raw_job; - PullJob *roothash_job; - PullJob *settings_job; PullJob *checksum_job; PullJob *signature_job; + PullJob *settings_job; + PullJob *roothash_job; + PullJob *roothash_signature_job; + PullJob *verity_job; RawPullFinished on_finished; void *userdata; char *local; - bool force_local; - bool settings; - bool roothash; char *final_path; char *temp_path; @@ -67,7 +68,11 @@ struct RawPull { char *roothash_path; char *roothash_temp_path; - ImportVerify verify; + char *roothash_signature_path; + char *roothash_signature_temp_path; + + char *verity_path; + char *verity_temp_path; }; RawPull* raw_pull_unref(RawPull *i) { @@ -75,34 +80,30 @@ RawPull* raw_pull_unref(RawPull *i) { return NULL; pull_job_unref(i->raw_job); - pull_job_unref(i->settings_job); - pull_job_unref(i->roothash_job); pull_job_unref(i->checksum_job); pull_job_unref(i->signature_job); + pull_job_unref(i->settings_job); + pull_job_unref(i->roothash_job); + pull_job_unref(i->roothash_signature_job); + pull_job_unref(i->verity_job); curl_glue_unref(i->glue); sd_event_unref(i->event); - if (i->temp_path) { - (void) unlink(i->temp_path); - free(i->temp_path); - } - - if (i->roothash_temp_path) { - (void) unlink(i->roothash_temp_path); - free(i->roothash_temp_path); - } - - if (i->settings_temp_path) { - (void) unlink(i->settings_temp_path); - free(i->settings_temp_path); - } + unlink_and_free(i->temp_path); + unlink_and_free(i->settings_temp_path); + unlink_and_free(i->roothash_temp_path); + unlink_and_free(i->roothash_signature_temp_path); + unlink_and_free(i->verity_temp_path); free(i->final_path); - free(i->roothash_path); free(i->settings_path); + free(i->roothash_path); + free(i->roothash_signature_path); + free(i->verity_path); free(i->image_root); free(i->local); + return mfree(i); } @@ -169,6 +170,16 @@ static void raw_pull_report_progress(RawPull *i, RawProgress p) { percent = 0; + if (i->checksum_job) { + percent += i->checksum_job->progress_percent * 5 / 100; + remain -= 5; + } + + if (i->signature_job) { + percent += i->signature_job->progress_percent * 5 / 100; + remain -= 5; + } + if (i->settings_job) { percent += i->settings_job->progress_percent * 5 / 100; remain -= 5; @@ -179,14 +190,14 @@ static void raw_pull_report_progress(RawPull *i, RawProgress p) { remain -= 5; } - if (i->checksum_job) { - percent += i->checksum_job->progress_percent * 5 / 100; + if (i->roothash_signature_job) { + percent += i->roothash_signature_job->progress_percent * 5 / 100; remain -= 5; } - if (i->signature_job) { - percent += i->signature_job->progress_percent * 5 / 100; - remain -= 5; + if (i->verity_job) { + percent += i->verity_job->progress_percent * 10 / 100; + remain -= 10; } if (i->raw_job) @@ -294,7 +305,7 @@ static int raw_pull_copy_auxiliary_file( local = strjoina(i->image_root, "/", i->local, suffix); - r = copy_file_atomic(*path, local, 0644, 0, 0, COPY_REFLINK | (i->force_local ? COPY_REPLACE : 0)); + r = copy_file_atomic(*path, local, 0644, 0, 0, COPY_REFLINK | (FLAGS_SET(i->flags, PULL_FORCE) ? COPY_REPLACE : 0)); if (r == -EEXIST) log_warning_errno(r, "File %s already exists, not replacing.", local); else if (r == -ENOENT) @@ -338,7 +349,7 @@ static int raw_pull_make_local_copy(RawPull *i) { p = strjoina(i->image_root, "/", i->local, ".raw"); - if (i->force_local) + if (FLAGS_SET(i->flags, PULL_FORCE)) (void) rm_rf(p, REMOVE_ROOT|REMOVE_PHYSICAL|REMOVE_SUBVOLUME); r = tempfn_random(p, NULL, &tp); @@ -373,14 +384,26 @@ static int raw_pull_make_local_copy(RawPull *i) { log_info("Created new local image '%s'.", i->local); - if (i->roothash) { + if (FLAGS_SET(i->flags, PULL_SETTINGS)) { + r = raw_pull_copy_auxiliary_file(i, ".nspawn", &i->settings_path); + if (r < 0) + return r; + } + + if (FLAGS_SET(i->flags, PULL_ROOTHASH)) { r = raw_pull_copy_auxiliary_file(i, ".roothash", &i->roothash_path); if (r < 0) return r; } - if (i->settings) { - r = raw_pull_copy_auxiliary_file(i, ".nspawn", &i->settings_path); + if (FLAGS_SET(i->flags, PULL_ROOTHASH_SIGNATURE)) { + r = raw_pull_copy_auxiliary_file(i, ".roothash.p7s", &i->roothash_signature_path); + if (r < 0) + return r; + } + + if (FLAGS_SET(i->flags, PULL_VERITY)) { + r = raw_pull_copy_auxiliary_file(i, ".verity", &i->verity_path); if (r < 0) return r; } @@ -394,14 +417,18 @@ static bool raw_pull_is_done(RawPull *i) { if (!PULL_JOB_IS_COMPLETE(i->raw_job)) return false; - if (i->roothash_job && !PULL_JOB_IS_COMPLETE(i->roothash_job)) - return false; - if (i->settings_job && !PULL_JOB_IS_COMPLETE(i->settings_job)) - return false; if (i->checksum_job && !PULL_JOB_IS_COMPLETE(i->checksum_job)) return false; if (i->signature_job && !PULL_JOB_IS_COMPLETE(i->signature_job)) return false; + if (i->settings_job && !PULL_JOB_IS_COMPLETE(i->settings_job)) + return false; + if (i->roothash_job && !PULL_JOB_IS_COMPLETE(i->roothash_job)) + return false; + if (i->roothash_signature_job && !PULL_JOB_IS_COMPLETE(i->roothash_signature_job)) + return false; + if (i->verity_job && !PULL_JOB_IS_COMPLETE(i->verity_job)) + return false; return true; } @@ -447,12 +474,18 @@ static void raw_pull_job_on_finished(PullJob *j) { assert(j->userdata); i = j->userdata; - if (j == i->roothash_job) { - if (j->error != 0) - log_info_errno(j->error, "Root hash file could not be retrieved, proceeding without."); - } else if (j == i->settings_job) { + if (j == i->settings_job) { if (j->error != 0) log_info_errno(j->error, "Settings file could not be retrieved, proceeding without."); + } else if (j == i->roothash_job) { + if (j->error != 0) + log_info_errno(j->error, "Root hash file could not be retrieved, proceeding without."); + } else if (j == i->roothash_signature_job) { + if (j->error != 0) + log_info_errno(j->error, "Root hash signature file could not be retrieved, proceeding without."); + } else if (j == i->verity_job) { + if (j->error != 0) + log_info_errno(j->error, "Verity integrity file could not be retrieved, proceeding without. %s", j->url); } else if (j->error != 0 && j != i->signature_job) { if (j == i->checksum_job) log_error_errno(j->error, "Failed to retrieve SHA256 checksum, cannot verify. (Try --verify=no?)"); @@ -463,10 +496,8 @@ static void raw_pull_job_on_finished(PullJob *j) { goto finish; } - /* This is invoked if either the download completed - * successfully, or the download was skipped because we - * already have the etag. In this case ->etag_exists is - * true. + /* This is invoked if either the download completed successfully, or the download was skipped because + * we already have the etag. In this case ->etag_exists is true. * * We only do something when we got all three files */ @@ -492,10 +523,14 @@ static void raw_pull_job_on_finished(PullJob *j) { } } - if (i->roothash_job) - i->roothash_job->disk_fd = safe_close(i->roothash_job->disk_fd); if (i->settings_job) i->settings_job->disk_fd = safe_close(i->settings_job->disk_fd); + if (i->roothash_job) + i->roothash_job->disk_fd = safe_close(i->roothash_job->disk_fd); + if (i->roothash_signature_job) + i->roothash_signature_job->disk_fd = safe_close(i->roothash_signature_job->disk_fd); + if (i->verity_job) + i->verity_job->disk_fd = safe_close(i->verity_job->disk_fd); r = raw_pull_determine_path(i, ".raw", &i->final_path); if (r < 0) @@ -610,6 +645,18 @@ static int raw_pull_job_on_open_disk_raw(PullJob *j) { return 0; } +static int raw_pull_job_on_open_disk_settings(PullJob *j) { + RawPull *i; + + assert(j); + assert(j->userdata); + + i = j->userdata; + assert(i->settings_job == j); + + return raw_pull_job_on_open_disk_generic(i, j, "settings", &i->settings_temp_path); +} + static int raw_pull_job_on_open_disk_roothash(PullJob *j) { RawPull *i; @@ -622,16 +669,28 @@ static int raw_pull_job_on_open_disk_roothash(PullJob *j) { return raw_pull_job_on_open_disk_generic(i, j, "roothash", &i->roothash_temp_path); } -static int raw_pull_job_on_open_disk_settings(PullJob *j) { +static int raw_pull_job_on_open_disk_roothash_signature(PullJob *j) { RawPull *i; assert(j); assert(j->userdata); i = j->userdata; - assert(i->settings_job == j); + assert(i->roothash_signature_job == j); - return raw_pull_job_on_open_disk_generic(i, j, "settings", &i->settings_temp_path); + return raw_pull_job_on_open_disk_generic(i, j, "roothash.p7s", &i->roothash_signature_temp_path); +} + +static int raw_pull_job_on_open_disk_verity(PullJob *j) { + RawPull *i; + + assert(j); + assert(j->userdata); + + i = j->userdata; + assert(i->verity_job == j); + + return raw_pull_job_on_open_disk_generic(i, j, "verity", &i->verity_temp_path); } static void raw_pull_job_on_progress(PullJob *j) { @@ -649,16 +708,15 @@ int raw_pull_start( RawPull *i, const char *url, const char *local, - bool force_local, - ImportVerify verify, - bool settings, - bool roothash) { + PullFlags flags, + ImportVerify verify) { int r; assert(i); assert(verify < _IMPORT_VERIFY_MAX); assert(verify >= 0); + assert(!(flags & ~PULL_FLAGS_MASK_RAW)); if (!http_url_is_valid(url)) return -EINVAL; @@ -673,10 +731,8 @@ int raw_pull_start( if (r < 0) return r; - i->force_local = force_local; + i->flags = flags; i->verify = verify; - i->settings = settings; - i->roothash = roothash; /* Queue job for the image itself */ r = pull_job_new(&i->raw_job, url, i->glue, i); @@ -692,17 +748,11 @@ int raw_pull_start( if (r < 0) return r; - if (roothash) { - r = pull_make_auxiliary_job(&i->roothash_job, url, raw_strip_suffixes, ".roothash", i->glue, raw_pull_job_on_finished, i); - if (r < 0) - return r; + r = pull_make_verification_jobs(&i->checksum_job, &i->signature_job, verify, url, i->glue, raw_pull_job_on_finished, i); + if (r < 0) + return r; - i->roothash_job->on_open_disk = raw_pull_job_on_open_disk_roothash; - i->roothash_job->on_progress = raw_pull_job_on_progress; - i->roothash_job->calc_checksum = verify != IMPORT_VERIFY_NO; - } - - if (settings) { + if (FLAGS_SET(flags, PULL_SETTINGS)) { r = pull_make_auxiliary_job(&i->settings_job, url, raw_strip_suffixes, ".nspawn", i->glue, raw_pull_job_on_finished, i); if (r < 0) return r; @@ -712,26 +762,40 @@ int raw_pull_start( i->settings_job->calc_checksum = verify != IMPORT_VERIFY_NO; } - r = pull_make_verification_jobs(&i->checksum_job, &i->signature_job, verify, url, i->glue, raw_pull_job_on_finished, i); - if (r < 0) - return r; + if (FLAGS_SET(flags, PULL_ROOTHASH)) { + r = pull_make_auxiliary_job(&i->roothash_job, url, raw_strip_suffixes, ".roothash", i->glue, raw_pull_job_on_finished, i); + if (r < 0) + return r; + + i->roothash_job->on_open_disk = raw_pull_job_on_open_disk_roothash; + i->roothash_job->on_progress = raw_pull_job_on_progress; + i->roothash_job->calc_checksum = verify != IMPORT_VERIFY_NO; + } + + if (FLAGS_SET(flags, PULL_ROOTHASH_SIGNATURE)) { + r = pull_make_auxiliary_job(&i->roothash_signature_job, url, raw_strip_suffixes, ".roothash.p7s", i->glue, raw_pull_job_on_finished, i); + if (r < 0) + return r; + + i->roothash_signature_job->on_open_disk = raw_pull_job_on_open_disk_roothash_signature; + i->roothash_signature_job->on_progress = raw_pull_job_on_progress; + i->roothash_signature_job->calc_checksum = verify != IMPORT_VERIFY_NO; + } + + if (FLAGS_SET(flags, PULL_VERITY)) { + r = pull_make_auxiliary_job(&i->verity_job, url, raw_strip_suffixes, ".verity", i->glue, raw_pull_job_on_finished, i); + if (r < 0) + return r; + + i->verity_job->on_open_disk = raw_pull_job_on_open_disk_verity; + i->verity_job->on_progress = raw_pull_job_on_progress; + i->verity_job->calc_checksum = verify != IMPORT_VERIFY_NO; + } r = pull_job_begin(i->raw_job); if (r < 0) return r; - if (i->roothash_job) { - r = pull_job_begin(i->roothash_job); - if (r < 0) - return r; - } - - if (i->settings_job) { - r = pull_job_begin(i->settings_job); - if (r < 0) - return r; - } - if (i->checksum_job) { i->checksum_job->on_progress = raw_pull_job_on_progress; i->checksum_job->on_not_found = pull_job_restart_with_sha256sum; @@ -749,5 +813,29 @@ int raw_pull_start( return r; } + if (i->settings_job) { + r = pull_job_begin(i->settings_job); + if (r < 0) + return r; + } + + if (i->roothash_job) { + r = pull_job_begin(i->roothash_job); + if (r < 0) + return r; + } + + if (i->roothash_signature_job) { + r = pull_job_begin(i->roothash_signature_job); + if (r < 0) + return r; + } + + if (i->verity_job) { + r = pull_job_begin(i->verity_job); + if (r < 0) + return r; + } + return 0; } diff --git a/src/import/pull-raw.h b/src/import/pull-raw.h index e1d450d9dfd..985bda4762c 100644 --- a/src/import/pull-raw.h +++ b/src/import/pull-raw.h @@ -5,6 +5,7 @@ #include "import-util.h" #include "macro.h" +#include "pull-common.h" typedef struct RawPull RawPull; @@ -15,4 +16,4 @@ RawPull* raw_pull_unref(RawPull *pull); DEFINE_TRIVIAL_CLEANUP_FUNC(RawPull*, raw_pull_unref); -int raw_pull_start(RawPull *pull, const char *url, const char *local, bool force_local, ImportVerify verify, bool settings, bool roothash); +int raw_pull_start(RawPull *pull, const char *url, const char *local, PullFlags flags, ImportVerify verify); diff --git a/src/import/pull-tar.c b/src/import/pull-tar.c index 728b0fe6855..1ee0db15604 100644 --- a/src/import/pull-tar.c +++ b/src/import/pull-tar.c @@ -40,19 +40,19 @@ struct TarPull { sd_event *event; CurlGlue *glue; + PullFlags flags; + ImportVerify verify; char *image_root; PullJob *tar_job; - PullJob *settings_job; PullJob *checksum_job; PullJob *signature_job; + PullJob *settings_job; TarPullFinished on_finished; void *userdata; char *local; - bool force_local; - bool settings; pid_t tar_pid; @@ -61,8 +61,6 @@ struct TarPull { char *settings_path; char *settings_temp_path; - - ImportVerify verify; }; TarPull* tar_pull_unref(TarPull *i) { @@ -75,22 +73,15 @@ TarPull* tar_pull_unref(TarPull *i) { } pull_job_unref(i->tar_job); - pull_job_unref(i->settings_job); pull_job_unref(i->checksum_job); pull_job_unref(i->signature_job); + pull_job_unref(i->settings_job); curl_glue_unref(i->glue); sd_event_unref(i->event); - if (i->temp_path) { - (void) rm_rf(i->temp_path, REMOVE_ROOT|REMOVE_PHYSICAL|REMOVE_SUBVOLUME); - free(i->temp_path); - } - - if (i->settings_temp_path) { - (void) unlink(i->settings_temp_path); - free(i->settings_temp_path); - } + rm_rf_subvolume_and_free(i->temp_path); + unlink_and_free(i->settings_temp_path); free(i->final_path); free(i->settings_path); @@ -163,11 +154,6 @@ static void tar_pull_report_progress(TarPull *i, TarProgress p) { percent = 0; - if (i->settings_job) { - percent += i->settings_job->progress_percent * 5 / 100; - remain -= 5; - } - if (i->checksum_job) { percent += i->checksum_job->progress_percent * 5 / 100; remain -= 5; @@ -178,6 +164,11 @@ static void tar_pull_report_progress(TarPull *i, TarProgress p) { remain -= 5; } + if (i->settings_job) { + percent += i->settings_job->progress_percent * 5 / 100; + remain -= 5; + } + if (i->tar_job) percent += i->tar_job->progress_percent * remain / 100; break; @@ -230,11 +221,11 @@ static int tar_pull_make_local_copy(TarPull *i) { if (!i->local) return 0; - r = pull_make_local_copy(i->final_path, i->image_root, i->local, i->force_local); + r = pull_make_local_copy(i->final_path, i->image_root, i->local, i->flags); if (r < 0) return r; - if (i->settings) { + if (FLAGS_SET(i->flags, PULL_SETTINGS)) { const char *local_settings; assert(i->settings_job); @@ -244,7 +235,7 @@ static int tar_pull_make_local_copy(TarPull *i) { local_settings = strjoina(i->image_root, "/", i->local, ".nspawn"); - r = copy_file_atomic(i->settings_path, local_settings, 0664, 0, 0, COPY_REFLINK | (i->force_local ? COPY_REPLACE : 0)); + r = copy_file_atomic(i->settings_path, local_settings, 0664, 0, 0, COPY_REFLINK | (FLAGS_SET(i->flags, PULL_FORCE) ? COPY_REPLACE : 0)); if (r == -EEXIST) log_warning_errno(r, "Settings file %s already exists, not replacing.", local_settings); else if (r == -ENOENT) @@ -264,12 +255,12 @@ static bool tar_pull_is_done(TarPull *i) { if (!PULL_JOB_IS_COMPLETE(i->tar_job)) return false; - if (i->settings_job && !PULL_JOB_IS_COMPLETE(i->settings_job)) - return false; if (i->checksum_job && !PULL_JOB_IS_COMPLETE(i->checksum_job)) return false; if (i->signature_job && !PULL_JOB_IS_COMPLETE(i->signature_job)) return false; + if (i->settings_job && !PULL_JOB_IS_COMPLETE(i->settings_job)) + return false; return true; } @@ -483,15 +474,15 @@ int tar_pull_start( TarPull *i, const char *url, const char *local, - bool force_local, - ImportVerify verify, - bool settings) { + PullFlags flags, + ImportVerify verify) { int r; assert(i); assert(verify < _IMPORT_VERIFY_MAX); assert(verify >= 0); + assert(!(flags & ~PULL_FLAGS_MASK_TAR)); if (!http_url_is_valid(url)) return -EINVAL; @@ -506,9 +497,8 @@ int tar_pull_start( if (r < 0) return r; - i->force_local = force_local; + i->flags = flags; i->verify = verify; - i->settings = settings; /* Set up download job for TAR file */ r = pull_job_new(&i->tar_job, url, i->glue, i); @@ -524,8 +514,13 @@ int tar_pull_start( if (r < 0) return r; + /* Set up download of checksum/signature files */ + r = pull_make_verification_jobs(&i->checksum_job, &i->signature_job, verify, url, i->glue, tar_pull_job_on_finished, i); + if (r < 0) + return r; + /* Set up download job for the settings file (.nspawn) */ - if (settings) { + if (FLAGS_SET(flags, PULL_SETTINGS)) { r = pull_make_auxiliary_job(&i->settings_job, url, tar_strip_suffixes, ".nspawn", i->glue, tar_pull_job_on_finished, i); if (r < 0) return r; @@ -535,21 +530,10 @@ int tar_pull_start( i->settings_job->calc_checksum = verify != IMPORT_VERIFY_NO; } - /* Set up download of checksum/signature files */ - r = pull_make_verification_jobs(&i->checksum_job, &i->signature_job, verify, url, i->glue, tar_pull_job_on_finished, i); - if (r < 0) - return r; - r = pull_job_begin(i->tar_job); if (r < 0) return r; - if (i->settings_job) { - r = pull_job_begin(i->settings_job); - if (r < 0) - return r; - } - if (i->checksum_job) { i->checksum_job->on_progress = tar_pull_job_on_progress; i->checksum_job->on_not_found = pull_job_restart_with_sha256sum; @@ -567,5 +551,11 @@ int tar_pull_start( return r; } + if (i->settings_job) { + r = pull_job_begin(i->settings_job); + if (r < 0) + return r; + } + return 0; } diff --git a/src/import/pull-tar.h b/src/import/pull-tar.h index 78d982cf5a0..414077549fd 100644 --- a/src/import/pull-tar.h +++ b/src/import/pull-tar.h @@ -5,6 +5,7 @@ #include "import-util.h" #include "macro.h" +#include "pull-common.h" typedef struct TarPull TarPull; @@ -15,4 +16,4 @@ TarPull* tar_pull_unref(TarPull *pull); DEFINE_TRIVIAL_CLEANUP_FUNC(TarPull*, tar_pull_unref); -int tar_pull_start(TarPull *pull, const char *url, const char *local, bool force_local, ImportVerify verify, bool settings); +int tar_pull_start(TarPull *pull, const char *url, const char *local, PullFlags flags, ImportVerify verify); diff --git a/src/import/pull.c b/src/import/pull.c index e80d8abe6f5..33e6196f794 100644 --- a/src/import/pull.c +++ b/src/import/pull.c @@ -19,11 +19,9 @@ #include "verbs.h" #include "web-util.h" -static bool arg_force = false; static const char *arg_image_root = "/var/lib/machines"; static ImportVerify arg_verify = IMPORT_VERIFY_SIGNATURE; -static bool arg_settings = true; -static bool arg_roothash = true; +static PullFlags arg_pull_flags = PULL_SETTINGS | PULL_ROOTHASH | PULL_ROOTHASH_SIGNATURE | PULL_VERITY; static int interrupt_signal_handler(sd_event_source *s, const struct signalfd_siginfo *si, void *userdata) { log_notice("Transfer aborted."); @@ -77,7 +75,7 @@ static int pull_tar(int argc, char *argv[], void *userdata) { "Local image name '%s' is not valid.", local); - if (!arg_force) { + if (!FLAGS_SET(arg_pull_flags, PULL_FORCE)) { r = image_find(IMAGE_MACHINE, local, NULL, NULL); if (r < 0) { if (r != -ENOENT) @@ -105,7 +103,7 @@ static int pull_tar(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to allocate puller: %m"); - r = tar_pull_start(pull, url, local, arg_force, arg_verify, arg_settings); + r = tar_pull_start(pull, url, local, arg_pull_flags & PULL_FLAGS_MASK_TAR, arg_verify); if (r < 0) return log_error_errno(r, "Failed to pull image: %m"); @@ -163,7 +161,7 @@ static int pull_raw(int argc, char *argv[], void *userdata) { "Local image name '%s' is not valid.", local); - if (!arg_force) { + if (!FLAGS_SET(arg_pull_flags, PULL_FORCE)) { r = image_find(IMAGE_MACHINE, local, NULL, NULL); if (r < 0) { if (r != -ENOENT) @@ -191,7 +189,7 @@ static int pull_raw(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to allocate puller: %m"); - r = raw_pull_start(pull, url, local, arg_force, arg_verify, arg_settings, arg_roothash); + r = raw_pull_start(pull, url, local, arg_pull_flags & PULL_FLAGS_MASK_RAW, arg_verify); if (r < 0) return log_error_errno(r, "Failed to pull image: %m"); @@ -214,6 +212,8 @@ static int help(int argc, char *argv[], void *userdata) { " 'checksum', 'signature'\n" " --settings=BOOL Download settings file with image\n" " --roothash=BOOL Download root hash file with image\n" + " --roothash-sigature=BOOL Download root hash signature file with image\n" + " --verity=BOOL Download verity file with image\n" " --image-root=PATH Image root directory\n\n" "Commands:\n" " tar URL [NAME] Download a TAR image\n" @@ -232,16 +232,20 @@ static int parse_argv(int argc, char *argv[]) { ARG_VERIFY, ARG_SETTINGS, ARG_ROOTHASH, + ARG_ROOTHASH_SIGNATURE, + ARG_VERITY, }; static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, - { "version", no_argument, NULL, ARG_VERSION }, - { "force", no_argument, NULL, ARG_FORCE }, - { "image-root", required_argument, NULL, ARG_IMAGE_ROOT }, - { "verify", required_argument, NULL, ARG_VERIFY }, - { "settings", required_argument, NULL, ARG_SETTINGS }, - { "roothash", required_argument, NULL, ARG_ROOTHASH }, + { "help", no_argument, NULL, 'h' }, + { "version", no_argument, NULL, ARG_VERSION }, + { "force", no_argument, NULL, ARG_FORCE }, + { "image-root", required_argument, NULL, ARG_IMAGE_ROOT }, + { "verify", required_argument, NULL, ARG_VERIFY }, + { "settings", required_argument, NULL, ARG_SETTINGS }, + { "roothash", required_argument, NULL, ARG_ROOTHASH }, + { "roothash-signature", required_argument, NULL, ARG_ROOTHASH_SIGNATURE }, + { "verity", required_argument, NULL, ARG_VERITY }, {} }; @@ -261,7 +265,7 @@ static int parse_argv(int argc, char *argv[]) { return version(); case ARG_FORCE: - arg_force = true; + arg_pull_flags |= PULL_FORCE; break; case ARG_IMAGE_ROOT: @@ -281,7 +285,7 @@ static int parse_argv(int argc, char *argv[]) { if (r < 0) return log_error_errno(r, "Failed to parse --settings= parameter '%s': %m", optarg); - arg_settings = r; + SET_FLAG(arg_pull_flags, PULL_SETTINGS, r); break; case ARG_ROOTHASH: @@ -289,7 +293,27 @@ static int parse_argv(int argc, char *argv[]) { if (r < 0) return log_error_errno(r, "Failed to parse --roothash= parameter '%s': %m", optarg); - arg_roothash = r; + SET_FLAG(arg_pull_flags, PULL_ROOTHASH, r); + + /* If we were asked to turn off the root hash, implicitly also turn off the root hash signature */ + if (!r) + SET_FLAG(arg_pull_flags, PULL_ROOTHASH_SIGNATURE, false); + break; + + case ARG_ROOTHASH_SIGNATURE: + r = parse_boolean(optarg); + if (r < 0) + return log_error_errno(r, "Failed to parse --roothash-signature= parameter '%s': %m", optarg); + + SET_FLAG(arg_pull_flags, PULL_ROOTHASH_SIGNATURE, r); + break; + + case ARG_VERITY: + r = parse_boolean(optarg); + if (r < 0) + return log_error_errno(r, "Failed to parse --verity= parameter '%s': %m", optarg); + + SET_FLAG(arg_pull_flags, PULL_VERITY, r); break; case '?': From ac71ece3c6ccc67dd5965c64026c879bcaec77f5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 16 Jan 2021 00:06:13 +0100 Subject: [PATCH 11/11] import: refactor how we do gpg validation Let's split out the actual gpg logic into a helper function, so that we can add alternative validations later on. --- src/import/pull-common.c | 143 +++++++++++++++++++++------------------ src/import/pull-common.h | 2 +- src/import/pull-raw.c | 2 +- src/import/pull-tar.c | 2 +- 4 files changed, 82 insertions(+), 67 deletions(-) diff --git a/src/import/pull-common.c b/src/import/pull-common.c index 8f295090038..403a0952bcf 100644 --- a/src/import/pull-common.c +++ b/src/import/pull-common.c @@ -363,77 +363,35 @@ static int verify_one(PullJob *checksum_job, PullJob *job) { return 1; } -int pull_verify(PullJob *main_job, - PullJob *roothash_job, - PullJob *settings_job, - PullJob *checksum_job, - PullJob *signature_job) { +static int verify_gpg( + const void *payload, size_t payload_size, + const void *signature, size_t signature_size) { _cleanup_close_pair_ int gpg_pipe[2] = { -1, -1 }; - _cleanup_close_ int sig_file = -1; char sig_file_path[] = "/tmp/sigXXXXXX", gpg_home[] = "/tmp/gpghomeXXXXXX"; _cleanup_(sigkill_waitp) pid_t pid = 0; bool gpg_home_created = false; - VerificationStyle style; int r; - assert(main_job); - assert(main_job->state == PULL_JOB_DONE); - - if (!checksum_job) - return 0; - - assert(main_job->calc_checksum); - assert(main_job->checksum); - - assert(checksum_job->state == PULL_JOB_DONE); - - if (!checksum_job->payload || checksum_job->payload_size <= 0) - return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), - "Checksum is empty, cannot verify."); - - r = verify_one(checksum_job, main_job); - if (r < 0) - return r; - - r = verify_one(checksum_job, roothash_job); - if (r < 0) - return r; - - r = verify_one(checksum_job, settings_job); - if (r < 0) - return r; - - if (!signature_job) - return 0; - - r = verification_style_from_url(checksum_job->url, &style); - if (r < 0) - return log_error_errno(r, "Failed to determine verification style from URL '%s': %m", checksum_job->url); - - if (style == VERIFICATION_PER_FILE) /* if this is per-file verification, then the signature is in the - * checksum job, let's thus ignore the signature job from now on, - * and use the checksum job as signature job. */ - signature_job = checksum_job; - - assert(signature_job->state == PULL_JOB_DONE); - - if (!signature_job->payload || signature_job->payload_size <= 0) - return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), - "Signature is empty, cannot verify."); + assert(payload || payload_size == 0); + assert(signature || signature_size == 0); r = pipe2(gpg_pipe, O_CLOEXEC); if (r < 0) return log_error_errno(errno, "Failed to create pipe for gpg: %m"); - sig_file = mkostemp(sig_file_path, O_RDWR); - if (sig_file < 0) - return log_error_errno(errno, "Failed to create temporary file: %m"); + if (signature_size > 0) { + _cleanup_close_ int sig_file = -1; - r = loop_write(sig_file, signature_job->payload, signature_job->payload_size, false); - if (r < 0) { - log_error_errno(r, "Failed to write to temporary file: %m"); - goto finish; + sig_file = mkostemp(sig_file_path, O_RDWR); + if (sig_file < 0) + return log_error_errno(errno, "Failed to create temporary file: %m"); + + r = loop_write(sig_file, signature, signature_size, false); + if (r < 0) { + log_error_errno(r, "Failed to write to temporary file: %m"); + goto finish; + } } if (!mkdtemp(gpg_home)) { @@ -462,7 +420,7 @@ int pull_verify(PullJob *main_job, NULL, /* dash */ NULL /* trailing NULL */ }; - unsigned k = ELEMENTSOF(cmd) - 6; + size_t k = ELEMENTSOF(cmd) - 6; /* Child */ @@ -478,8 +436,7 @@ int pull_verify(PullJob *main_job, cmd[k++] = strjoina("--homedir=", gpg_home); - /* We add the user keyring only to the command line - * arguments, if it's around since gpg fails + /* We add the user keyring only to the command line arguments, if it's around since gpg fails * otherwise. */ if (access(USER_KEYRING_PATH, F_OK) >= 0) cmd[k++] = "--keyring=" USER_KEYRING_PATH; @@ -487,7 +444,7 @@ int pull_verify(PullJob *main_job, cmd[k++] = "--keyring=" VENDOR_KEYRING_PATH; cmd[k++] = "--verify"; - if (style == VERIFICATION_PER_DIRECTORY) { + if (signature) { cmd[k++] = sig_file_path; cmd[k++] = "-"; cmd[k++] = NULL; @@ -501,7 +458,7 @@ int pull_verify(PullJob *main_job, gpg_pipe[0] = safe_close(gpg_pipe[0]); - r = loop_write(gpg_pipe[1], checksum_job->payload, checksum_job->payload_size, false); + r = loop_write(gpg_pipe[1], payload, payload_size, false); if (r < 0) { log_error_errno(r, "Failed to write to pipe: %m"); goto finish; @@ -522,7 +479,8 @@ int pull_verify(PullJob *main_job, } finish: - (void) unlink(sig_file_path); + if (signature_size > 0) + (void) unlink(sig_file_path); if (gpg_home_created) (void) rm_rf(gpg_home, REMOVE_ROOT|REMOVE_PHYSICAL); @@ -530,6 +488,63 @@ finish: return r; } +int pull_verify(ImportVerify verify, + PullJob *main_job, + PullJob *roothash_job, + PullJob *settings_job, + PullJob *checksum_job, + PullJob *signature_job) { + + VerificationStyle style; + int r; + + assert(main_job); + assert(main_job->state == PULL_JOB_DONE); + + if (verify == IMPORT_VERIFY_NO) + return 0; + + assert(main_job->calc_checksum); + assert(main_job->checksum); + assert(checksum_job); + assert(checksum_job->state == PULL_JOB_DONE); + + if (!checksum_job->payload || checksum_job->payload_size <= 0) + return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), + "Checksum is empty, cannot verify."); + + r = verify_one(checksum_job, main_job); + if (r < 0) + return r; + + r = verify_one(checksum_job, roothash_job); + if (r < 0) + return r; + + r = verify_one(checksum_job, settings_job); + if (r < 0) + return r; + + if (verify == IMPORT_VERIFY_CHECKSUM) + return 0; + + r = verification_style_from_url(checksum_job->url, &style); + if (r < 0) + return log_error_errno(r, "Failed to determine verification style from URL '%s': %m", checksum_job->url); + + if (style == VERIFICATION_PER_DIRECTORY) { + assert(signature_job); + assert(signature_job->state == PULL_JOB_DONE); + + if (!signature_job->payload || signature_job->payload_size <= 0) + return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), + "Signature is empty, cannot verify."); + + return verify_gpg(checksum_job->payload, checksum_job->payload_size, signature_job->payload, signature_job->payload_size); + } else + return verify_gpg(checksum_job->payload, checksum_job->payload_size, NULL, 0); +} + int verification_style_from_url(const char *url, VerificationStyle *ret) { _cleanup_free_ char *last = NULL; int r; diff --git a/src/import/pull-common.h b/src/import/pull-common.h index f24093e6e11..acbbab7eabf 100644 --- a/src/import/pull-common.h +++ b/src/import/pull-common.h @@ -27,7 +27,7 @@ int pull_make_path(const char *url, const char *etag, const char *image_root, co int pull_make_auxiliary_job(PullJob **ret, const char *url, int (*strip_suffixes)(const char *name, char **ret), const char *suffix, CurlGlue *glue, PullJobFinished on_finished, void *userdata); int pull_make_verification_jobs(PullJob **ret_checksum_job, PullJob **ret_signature_job, ImportVerify verify, const char *url, CurlGlue *glue, PullJobFinished on_finished, void *userdata); -int pull_verify(PullJob *main_job, PullJob *roothash_job, PullJob *settings_job, PullJob *checksum_job, PullJob *signature_job); +int pull_verify(ImportVerify verify, PullJob *main_job, PullJob *roothash_job, PullJob *settings_job, PullJob *checksum_job, PullJob *signature_job); typedef enum VerificationStyle { VERIFICATION_PER_FILE, /* SuSE-style ".sha256" files with inline gpg signature */ diff --git a/src/import/pull-raw.c b/src/import/pull-raw.c index 5b4525d456c..0985dddef37 100644 --- a/src/import/pull-raw.c +++ b/src/import/pull-raw.c @@ -542,7 +542,7 @@ static void raw_pull_job_on_finished(PullJob *j) { raw_pull_report_progress(i, RAW_VERIFYING); - r = pull_verify(i->raw_job, i->roothash_job, i->settings_job, i->checksum_job, i->signature_job); + r = pull_verify(i->verify, i->raw_job, i->roothash_job, i->settings_job, i->checksum_job, i->signature_job); if (r < 0) goto finish; diff --git a/src/import/pull-tar.c b/src/import/pull-tar.c index 1ee0db15604..b52569d20c5 100644 --- a/src/import/pull-tar.c +++ b/src/import/pull-tar.c @@ -336,7 +336,7 @@ static void tar_pull_job_on_finished(PullJob *j) { tar_pull_report_progress(i, TAR_VERIFYING); - r = pull_verify(i->tar_job, NULL, i->settings_job, i->checksum_job, i->signature_job); + r = pull_verify(i->verify, i->tar_job, NULL, i->settings_job, i->checksum_job, i->signature_job); if (r < 0) goto finish;