[PATCH 0/2 V2] Check if package signature is already cached

[Bryan Evenson]

When using package signature verification, the signature file is downloaded
on every download, install or upgrade call even if it is already present in
the cache. This is inconsistent with the IPK download behavior and can lead
to unnecessary upgrade issues if there is a faulty connection to the package
repository.

The signature file is now only downloaded if it is not already present in
cache. The signature file is also deleted when the IPK is deleted to
guarantee both are downloaded together on any future download attempt.

Verified proper operation for the following scenarios:
* opkg upgrade
* opkg --download-only upgrade; <disconnect access to package repository> opkg upgrade
* opkg download; opkg install
* opkg install
* opkg install or opkg upgrade with volatile cache enabled

In all cases, the package installation or upgrade works if both the IPK and
the signature are available. If the signature is missing from cache, then
opkg attempts to re-download the signature. If the IPK is missing from
cache, then the signature file is deleted and the IPK and signature are
both re-downloaded. If the package verification fails, then both the IPK
and the signature are deleted.

Signed-off-by: Bryan Evenson <evenso...@gmail.com>

Bryan Evenson (2):
opkg_download: Check if signature is cached; add pkg_remove_signature
pkg_verify: Remove signature file with IPK

libopkg/opkg_download.c | 40 +++++++++++++++++++++++++++++++++++++++-
libopkg/opkg_download.h | 1 +
libopkg/pkg.c | 7 +++++++
3 files changed, 47 insertions(+), 1 deletion(-)

--
2.17.1

[Bryan Evenson]

Checks if the signature already exists in cache before attempting the
download. Previously, every call to 'opkg upgrade' would download the
signature. This led to problems in the following scenario:

* Call 'opkg --download-only upgrade' to download any package upgrades.
* Disconnect the package repository (i.e. disconnect removable media)
* Call 'opkg upgrade'

Previously 'opkg upgrade' would fail because it could not download the
signature files.

Adds function for removing the signature file. The signature file
should be removed if the verification fails.

Signed-off-by: Bryan Evenson <evenso...@gmail.com>
---

libopkg/opkg_download.c | 40 +++++++++++++++++++++++++++++++++++++++-
libopkg/opkg_download.h | 1 +

2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/libopkg/opkg_download.c b/libopkg/opkg_download.c
index 5c74f66..e83aeda 100644
--- a/libopkg/opkg_download.c
+++ b/libopkg/opkg_download.c
@@ -25,6 +25,7 @@
#include <string.h>
#include <unistd.h>
#include <libgen.h>
+#include <sys/stat.h>

#include "opkg_download.h"
#include "opkg_message.h"
@@ -248,12 +249,45 @@ static char *get_pkg_url(pkg_t * pkg)
return url;
}

+void pkg_remove_signature(pkg_t * pkg)
+{
+ char *pkg_url;
+ char *sig_url;
+ char *sig_ext;
+ char *sig_file;
+
+ pkg_url = get_pkg_url(pkg);
+ if (pkg_url) {
+
+ if (strcmp(opkg_config->signature_type, "gpg-asc") == 0)
+ sig_ext = "asc";
+ else
+ sig_ext = "sig";
+
+ sprintf_alloc(&sig_url, "%s.%s", pkg_url, sig_ext);
+ free(pkg_url);
+
+ sig_file = get_cache_location(sig_url);
+ unlink(sig_file);
+ free(sig_file);
+ free(sig_url);
+ }
+}
+
+/** \brief pkg_download_signature: download a package signature
+ * \details First checks if the signature has already been downloaded
+ *
+ * \param pkg the package associated with the signature
+ * \return The signature filename if success, NULL if error occurs
+ *
+ */
char *pkg_download_signature(pkg_t * pkg)
{
char *pkg_url;
char *sig_url;
char *sig_ext;
char *sig_file;
+ struct stat sig_stat;

pkg_url = get_pkg_url(pkg);
if (!pkg_url)
@@ -267,7 +301,11 @@ char *pkg_download_signature(pkg_t * pkg)
sprintf_alloc(&sig_url, "%s.%s", pkg_url, sig_ext);
free(pkg_url);

- sig_file = opkg_download_cache(sig_url, NULL, NULL);
+ sig_file = get_cache_location(sig_url);
+ if (stat(sig_file, &sig_stat)) {
+ free(sig_file);
+ sig_file = opkg_download_cache(sig_url, NULL, NULL);
+ }
free(sig_url);

return sig_file;
diff --git a/libopkg/opkg_download.h b/libopkg/opkg_download.h
index e88af85..c131232 100644
--- a/libopkg/opkg_download.h
+++ b/libopkg/opkg_download.h
@@ -36,6 +36,7 @@ int opkg_download(const char *src, const char *dest_file_name,
char *opkg_download_cache(const char *src, curl_progress_func cb, void *data);
int opkg_download_pkg(pkg_t * pkg);
int opkg_download_pkg_to_dir(pkg_t * pkg, const char *dir);
+void pkg_remove_signature(pkg_t * pkg);
char *pkg_download_signature(pkg_t * pkg);

/*
--
2.17.1

[Bryan Evenson]

The signature file ought to be removed if the IPK is not
present so both can be re-downloaded. Removes the signature
file if the IPK is not present or if the package verification

fails.

Signed-off-by: Bryan Evenson <evenso...@gmail.com>
---

libopkg/pkg.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/libopkg/pkg.c b/libopkg/pkg.c
index bd679db..70e39d3 100644
--- a/libopkg/pkg.c
+++ b/libopkg/pkg.c
@@ -1525,7 +1525,11 @@ int pkg_verify(pkg_t * pkg)
/* Exit with soft error 1 if the package doesn't exist.
* This allows the caller to download it without nasty
* messages in the error log.
+ * Remove any matching signature file if it exists.
*/
+ if (opkg_config->check_pkg_signature) {
+ pkg_remove_signature(pkg);
+ }
return 1;
}
else {
@@ -1585,6 +1589,9 @@ int pkg_verify(pkg_t * pkg)
opkg_msg(NOTICE, "Removing corrupt package file %s.\n",
pkg->local_filename);
unlink(pkg->local_filename);
+ if (opkg_config->check_pkg_signature) {
+ pkg_remove_signature(pkg);
+ }
return err;
}
else
--
2.17.1

[Alex Stewart]

Hey Bryan,

Thanks for the patch and for doing such thorough hand-testing.

I noticed while looking over your implementation that
`pkg_remove_signature()`, and the general design choice to unlink the
signature file in the case that the IPK data is invalid, is inspired by
similar behavior in `libopkg/pkg_src.c:pkg_src_verify()` - which helps
me to build confidence that we're doing the correct thing here.

As a maintenance request, could you break out the
`pkg_remove_signature()` function from `PATCH 1` and either: move it
into `PATCH 2` or have it live as its own atomic commit? It doesn't seem
to otherwise relate to 'PATCH 1'.

Otherwise, I'm happy with the patchset as it is.

Thanks,

--
Alex Stewart
Software Engineer - NI Real-Time OS
NI (National Instruments)

alex.s...@ni.com

[Bryan Evenson]

Alex,

Thanks for the feedback.  I'll send out a rearranged patchset later today.

Thanks,

Bryan

[Bryan Evenson]

When using package signature verification, the signature file is downloaded
on every download, install or upgrade call even if it is already present in
the cache. This is inconsistent with the IPK download behavior and can lead
to unnecessary upgrade issues if there is a faulty connection to the package
repository.

The signature file is now only downloaded if it is not already present in
cache. The signature file is also deleted when the IPK is deleted to
guarantee both are downloaded together on any future download attempt.

Verified proper operation for the following scenarios:
* opkg upgrade
* opkg --download-only upgrade; <disconnect access to package repository> opkg upgrade
* opkg download; opkg install
* opkg install
* opkg install or opkg upgrade with volatile cache enabled

In all cases, the package installation or upgrade works if both the IPK and
the signature are available. If the signature is missing from cache, then
opkg attempts to re-download the signature. If the IPK is missing from
cache, then the signature file is deleted and the IPK and signature are
both re-downloaded. If the package verification fails, then both the IPK
and the signature are deleted.

Signed-off-by: Bryan Evenson <evenso...@gmail.com>

Bryan Evenson (3):

opkg_download: Check if signature is cached

opkg_download: Add pkg_remove_signature()

pkg_verify: Remove signature file with IPK

libopkg/opkg_download.c | 40 +++++++++++++++++++++++++++++++++++++++-
libopkg/opkg_download.h | 1 +

[Bryan Evenson]

Checks if the signature already exists in cache before attempting the
download. Previously, every call to 'opkg upgrade' would download the
signature. This led to problems in the following scenario:

* Call 'opkg --download-only upgrade' to download any package upgrades.
* Disconnect the package repository (i.e. disconnect removable media)
* Call 'opkg upgrade'

Previously 'opkg upgrade' would fail because it could not download the
signature files.

Signed-off-by: Bryan Evenson <evenso...@gmail.com>
---

libopkg/opkg_download.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/libopkg/opkg_download.c b/libopkg/opkg_download.c
index 7c1cd43..6414d90 100644

--- a/libopkg/opkg_download.c
+++ b/libopkg/opkg_download.c
@@ -25,6 +25,7 @@
#include <string.h>
#include <unistd.h>
#include <libgen.h>
+#include <sys/stat.h>

#include "opkg_download.h"
#include "opkg_message.h"

@@ -255,12 +256,20 @@ static char *get_pkg_url(pkg_t * pkg)
return url;
}

+/** \brief pkg_download_signature: download a package signature
+ * \details First checks if the signature has already been downloaded
+ *
+ * \param pkg the package associated with the signature
+ * \return The signature filename if success, NULL if error occurs
+ *
+ */
char *pkg_download_signature(pkg_t * pkg)
{
char *pkg_url;
char *sig_url;
char *sig_ext;
char *sig_file;
+ struct stat sig_stat;

pkg_url = get_pkg_url(pkg);
if (!pkg_url)

@@ -274,7 +283,11 @@ char *pkg_download_signature(pkg_t * pkg)

sprintf_alloc(&sig_url, "%s.%s", pkg_url, sig_ext);
free(pkg_url);

- sig_file = opkg_download_cache(sig_url, NULL, NULL);
+ sig_file = get_cache_location(sig_url);
+ if (stat(sig_file, &sig_stat)) {
+ free(sig_file);
+ sig_file = opkg_download_cache(sig_url, NULL, NULL);
+ }
free(sig_url);

return sig_file;

--
2.17.1

[Bryan Evenson]

Add a function to delete the package signature file.

Signed-off-by: Bryan Evenson <evenso...@gmail.com>
---

libopkg/opkg_download.c | 25 +++++++++++++++++++++++++
libopkg/opkg_download.h | 1 +
2 files changed, 26 insertions(+)

diff --git a/libopkg/opkg_download.c b/libopkg/opkg_download.c
index 6414d90..4086f83 100644
--- a/libopkg/opkg_download.c
+++ b/libopkg/opkg_download.c
@@ -256,6 +256,31 @@ static char *get_pkg_url(pkg_t * pkg)
return url;
}

+void pkg_remove_signature(pkg_t * pkg)
+{
+ char *pkg_url;
+ char *sig_url;
+ char *sig_ext;
+ char *sig_file;
+
+ pkg_url = get_pkg_url(pkg);
+ if (pkg_url) {
+
+ if (strcmp(opkg_config->signature_type, "gpg-asc") == 0)
+ sig_ext = "asc";
+ else
+ sig_ext = "sig";
+

+ sprintf_alloc(&sig_url, "%s.%s", pkg_url, sig_ext);
+ free(pkg_url);
+
+ sig_file = get_cache_location(sig_url);

+ unlink(sig_file);
+ free(sig_file);
+ free(sig_url);
+ }
+}
+

/** \brief pkg_download_signature: download a package signature

* \details First checks if the signature has already been downloaded

*

[Bryan Evenson]

The signature file ought to be removed if the IPK is not
present so both can be re-downloaded. Removes the signature
file if the IPK is not present or if the package verification
fails.

Signed-off-by: Bryan Evenson <evenso...@gmail.com>
---

libopkg/pkg.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/libopkg/pkg.c b/libopkg/pkg.c

index 31abdd1..db10ccf 100644
--- a/libopkg/pkg.c
+++ b/libopkg/pkg.c
@@ -1531,7 +1531,11 @@ int pkg_verify(pkg_t * pkg)

/* Exit with soft error 1 if the package doesn't exist.
* This allows the caller to download it without nasty
* messages in the error log.
+ * Remove any matching signature file if it exists.
*/
+ if (opkg_config->check_pkg_signature) {
+ pkg_remove_signature(pkg);
+ }
return 1;
}
else {

@@ -1591,6 +1595,9 @@ int pkg_verify(pkg_t * pkg)

[Alex Stewart]

Looks good to me! I'll merge merge this tomorrow, if there are no other
objections.

Thanks for contributing!

[Alex Stewart]

Merged to `master` as commit b52c702d0c6e00b3304ac26d517f9a93b63db1c8.

https://git.yoctoproject.org/opkg/commit/?id=b52c702d0c6e00b3304ac26d517f9a93b63db1c8

Thanks,