This release removes certificate pinning to enable the client to communicate with a CDN for update operations, minimizing syscalls to fdopen/fclose during file downloads and using the default curl write handler to improve performance, and returning proper return codes when calling check-update.
Signed-off-by: Tudor Marcu <tudor.marcu@intel.com>
Now that the number of pending downloads is kept below a certain limit
(see poll_fewer_than()) it is possible to open files before starting
the transfer. Using the default curl write handler and explicit
open/close of the file makes the code simpler.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
The previous approach was to open/fdopen/fclose the file for each
chunk that gets passed from curl. This incurrs a huge performance hit
when close() triggers a hashing of the file content on systems where
integrity protection via IMA is enabled.
Now the file is opened only once and kept open until the download is
complete. In addition, the unnecessary usage of C file IO is avoided.
The semantic is changed as little as possible:
- file gets created only after the first chunk of data arrived
- file descriptors do not leak to child processes (O_CLOEXEC)
- data gets appended to existing files (via O_APPEND, used
to keep the code simple and avoid an additional lseek)
- data gets flushed explicitly for each chunk (via fdatasync(),
which somewhat approximates the effect that an explicit
close() may have had)
As an additional improvement, failures during close() are checked. To
keep error handling as much as before, the completion function which has
the close() takes the current curl error code and replaces it if it
encounters a write error.
[v2 of the patch with fixes by Dmitry Rozhkov, see https://github.com/pohly/swupd-client/pull/1]
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
To enable swupd-client to communicate with a CDN for updates, the custom
cert pinning must be removed.
Signed-off-by: Tudor Marcu <tudor.marcu@intel.com>
This release changes swupd-client to ignore xattrs by default, fixing errors on systems where xattrs were enabled thus changing the hash of files on system and mismatching hashes in update manifests; printing diagnostic info on network-connectivity problems to easier debug update failures, and fixing some build warnings from autoreconf.
Signed-off-by: Tudor Marcu <tudor.marcu@intel.com>
When running 'autoreconf', aclocal warns that the "m4" directory does
not exist, even though AC_CONFIG_MACRO_DIR in configure.ac declares that
it should exist.
To avoid the warning, track the directory in the git tree.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
Certain types of network-connectivity problems hit this curl error, so
print a diagnostic message when it occurs.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
Swupd used to use a sorted blob of extended file attributes as part of
the data that was used to verify the contents. This caused problems if
extra attributes were added. In particular if a clearlinux system was
being run in a container that had selinux enabled in the base OS.
Add a configure option to allow the attributes to be considered or
not. Based on advice, the default is to ignore them, as this is what
everyone expects.
Tidied up passing a --selinux flag to tar, this is also handled by a new
config option.
Signed-off-by: Icarus Sparry <icarus.w.sparry@intel.com>
Swupd used to use a sorted blob of extended file attributes as part of
the data that was used to verify the contents. This caused problems if
extra attributes were added. In particular if a clearlinux system was
being run in a container that had selinux enabled in the base OS.
Add a configure option to allow the attributes to be considered or
not. Based on advice, the default is to ignore them, as this is what
everyone expects.
Tidied up passing a --selinux flag to tar, this is also handled by a new
config option.
Signed-off-by: Icarus Sparry <icarus.w.sparry@intel.com>
This release includes major updates to various components of swupd-client:
A rework of the bundle subscription code, making the consolidation steps
simpler, reporting bundle changes for updates instead of "manifest" changes,
accounting fixes for the update stats, and reworking the update list creation
logic to massively improve performance by not calling verify_file() when
not needed.
The interface for functional tests was changed to better accommodate updating
test cases when output changes (because of code changes/fixes), such that
each test case has a single expected output that is tracked as a whole,
rather than individual output lines.
Many performance enchancements to drastically reduce client update time:
- Cutting down the amount of recursion when adding subscribed bundles.
- Caching the latest version number.
- Caching manifest hash checks if they succeed.
- Checking only the manifest headers on paths where the whole manifest does not
need to be parsed.
- Enabling multiplexing in curl.
- Setting nosync on staging dir removal operations.
- Fixing statistics and time reporting for updates.
Signed-off-by: Tudor Marcu <tudor.marcu@intel.com>
We can't sort the update list inside a helper function
that only gets a pointer to the list; the list head can get
updated as part of the sort, and this update to the list
head doesn't get reflected in the caller, leading
to a corrupted linked list.
Signed-off-by: Tudor Marcu <tudor.marcu@intel.com>
Adding CURLPIPE_MULTIPLEX | CURLPIPE_HTTP1 is now possible with libcurl >=
7.43.0. The value 1 tells it to first check if multiplexing is supported on
a connection before attempting any transfers.
Signed-off-by: Tudor Marcu <tudor.marcu@intel.com>
We spend a lot of wasted time parsing the entire manifest in some areas, when
only the header is needed. This patch adds a header_only flag that tells the
appropriate load manifest functions to quit early after the header is read,
instead of parsing the entire (possibly very large) manifest. Another
optimization is calling fopen() with the 'm' flag, which tries to use mmap to
access the file (for reading only).
Signed-off-by: Tudor Marcu <tudor.marcu@intel.com>
This patch cuts down the amount of recursion that happens by skipping a
subsequent rescursive calls to add_subscriptions if we hit a bundle that is
already subscribed.
Signed-off-by: Tudor Marcu <tudor.marcu@intel.com>
It's not clear to me why the 'swupd search' output for the functional
tests is so much different than running it outside that environment, but
regardless, the most interesting output line is what is grepped for.
This output disparity needs further debugging.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
The bundle-remove subcommand returns a unique error code for invalid
usage, so check that instead. This also avoids the need to track
bundle-remove --help output, or ignore it.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
There are several output lines that are not very interesting to check
for functional tests, so ignore those lines completely for testing by
adding some specific regular expressions for matching.
This also enables detection of unexpected error messages that may arise
when running the tests.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
This commit adds "lines-checked" files for every test that checks
swupd-client output and removes the old bash-array-style checks from
the test scripts.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
This commit adds a new helper function for functional tests that enables
a more streamlined mechanism for declaring a fixed set of output lines
to check for and to compare against the swupd-client output.
To use this new interface, lines of output to be checked for a given
test will live in the "lines-checked" file within the test directory,
with the swupd-client output dumped to "lines-output". Each line of
"lines-checked" is either interpreted as a literal string, or as a
regular expression; regular expression lines are denoted with the
"REGEXP:" prefix, and all other lines are literal strings.
Note that swupd-client may emit more output lines than those checked for
in "lines-checked", and that the checked lines should be declared in
order.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
Instead of printing the stats for what changed overall in the MoM, it's
more interesting to report what changed on their system. That is,
reporting which bundles are new, have changed, or (in the future) have
been deleted.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
The purpose of calling verify_file() in create_update_list() was to
minimize extraneous fullfile downloads when a minversion bump occurs
server-side, and the update crosses that version.
However, the implementation was buggy; verify_file() was called far too
often, namely for any file on the system that did not change. This
commit fixes the issue.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
To more accurately calculate what changed during an update, we keep two
subscription lists: one for the set of installed bundles in the current
version, and one for the new set of bundles that will be installed after
an update completes.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
After the refactor of update list creation, and the segregation of
current/latest subscription lists, reassigning last_change like this
doesn't operate correctly, so remove it.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
The "latest" version will always be newer than the "current" version, so
only the the greater-than check is needed.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
There may be extra files at the tail end of the filelist for the latest
version, in which case we want to account them as new files.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
To avoid calling the version-setting function multiple times for a
single subscription list for 'update', simply reference both MoM manifests
in the function. For other subcommands that only reference a single MoM
manifest, the second argument should be NULL.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
A forthcoming commit will make 'update' understand two subscription
lists, one each for the current and latest versions, so using a single
global list will no longer be sufficient.
Instead, allocate a subs list for each subcommand that needs it, and
adjust helper function signatures that need to read/write the subs list.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
Because the 'search' subcommand does not download any packs, the subs
list entries do not need to be versioned; the versioning is only used
for pack downloads.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
The memory is freed later on by swupd_deinit(), so
install_bundles_frontend() does not need to free it.
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
This release includes fixes for handling and removing files and directories,
notably a fix that caused swupd to fail on docker containers due to the overlay
filesystem behaviour, and reading system clock from a better clock source not
hindered by time skew or ntp adjustments.
Signed-off-by: Tudor Marcu <tudor.marcu@intel.com>
Man 3 readdir_r states: "This function is deprecated; use readdir(3)
instead."
Further, it also reads:
```
* In the current POSIX.1 specification (POSIX.1-2008), readdir(3) is
not required to be thread-safe. However, in modern
implementations (including the glibc implementation), concurrent
calls to readdir(3) that specify different directory streams are
thread-safe. Therefore, the use of readdir_r() is generally
unnecessary in multithreaded programs. In cases where multiple
threads must read from the same directory stream, using readdir(3)
with external synchronization is still preferable to the use of
readdir_r(), for the reasons given in the points above.
```
I don't expect swupd-client to be used with anything other than
glibc at this moment, and I don't see that the current code is
unsafely being used in multithreaded code, so we should avoid
using the deprecated interface.
Besides being a better clock source, this clock source does not
change with time skew. We also don't print the time taken unless
we actually do something significant.
When a file doesn't verify as part of verify_fix_path, prior to
downloading and staging the new file, first remove the old file staging
content.
This change was required due to a failure with swupd-client when run
inside Docker due to /var/lib/swupd being layered under multiple mount
points because of separate Docker Run commands. This caused rename(2) to
fail when the expected layout of /var/lib/swupd is to be the same mount
point.
With this change in place, the rename source content that would be from
a different mount is removed and redownloaded into the same mount as the
rename target.
Because swupd-client on Clear Linux may fork/exec clr-boot-manager near
the end of its operation, which itself checks for file descriptor leaks
before exiting, it will report leaks for files it did not open (i.e.
inherited from the parent process, swupd).
To resolve this, set O_CLOEXEC when opening the cert file using the "e"
flag (glibc extension).
Signed-off-by: Patrick McCarty <patrick.mccarty@intel.com>
Tudor Marcu