Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xbps-install: add --verify-sig option. #384

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions bin/xbps-install/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ usage(bool fail)
" --reproducible Enable reproducible mode in pkgdb\n"
" -S, --sync Sync remote repository index\n"
" -u, --update Update target package(s)\n"
" --verify-local-sig Verify package signatures even for local repositories\n"
" -v, --verbose Verbose messages\n"
" -y, --yes Assume yes to all questions\n"
" -V, --version Show XBPS version\n");
Expand Down Expand Up @@ -118,6 +119,7 @@ main(int argc, char **argv)
{ "version", no_argument, NULL, 'V' },
{ "yes", no_argument, NULL, 'y' },
{ "reproducible", no_argument, NULL, 1 },
{ "verify-local-sig", no_argument, NULL, 2 },
{ NULL, 0, NULL, 0 }
};
struct xbps_handle xh;
Expand All @@ -138,6 +140,9 @@ main(int argc, char **argv)
case 1:
flags |= XBPS_FLAG_INSTALL_REPRO;
break;
case 2:
flags |= XBPS_FLAG_VERIFY_LOCAL_REPO;
break;
case 'A':
flags |= XBPS_FLAG_INSTALL_AUTO;
break;
Expand Down
3 changes: 3 additions & 0 deletions bin/xbps-install/xbps-install.1
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,9 @@ Performs a full system upgrade: all installed packages
.Pq except those on Sy hold , No see Fl -mode Sy hold No in Xr xbps-pkgdb 1
will be updated to the greatest
versions that were found in repositories.
.It Fl -verify-local-sig
Enables signature verification for local repositories in addition to
remote ones.
.It Fl v, Fl -verbose
Enables verbose messages.
.It Fl y, Fl -yes
Expand Down
36 changes: 35 additions & 1 deletion include/xbps.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,13 @@
*/
#define XBPS_FLAG_KEEP_CONFIG 0x00010000

/**
* @def XBPS_FLAG_VERIFY_LOCAL_REPO
* Verify package signatures even for local repositories.
* Must be set through the xbps_handle::flags member.
*/
#define XBPS_FLAG_VERIFY_LOCAL_REPO 0x00020000

/**
* @def XBPS_FETCH_CACHECONN
* Default (global) limit of cached connections used in libfetch.
Expand Down Expand Up @@ -681,6 +688,19 @@ struct xbps_handle {
int flags;
};

/**
* @struct xbps_sha256_digest xbps.h "xbps.h"
* @brief Structure to contain a binary SHA256 digest.
*/
struct xbps_sha256_digest {
/**
* @var buffer
*
* Buffer containing a SHA256 binary digest.
*/
unsigned char buffer[XBPS_SHA256_DIGEST_SIZE];
};

void xbps_dbg_printf(struct xbps_handle *, const char *, ...) __attribute__ ((format (printf, 2, 3)));
void xbps_dbg_printf_append(struct xbps_handle *, const char *, ...)__attribute__ ((format (printf, 2, 3)));
void xbps_error_printf(const char *, ...)__attribute__ ((format (printf, 1, 2)));
Expand Down Expand Up @@ -1951,6 +1971,20 @@ bool xbps_file_sha256_raw(unsigned char *dst, size_t len, const char *file);
*/
int xbps_file_sha256_check(const char *file, const char *sha256);

/**
* Compares the sha256 hash of the file \a file with the sha256
* string specified by \a sha256. The computed sha256 digest is stored into
* \a dst.
*
* @param[in] file Path to a file.
* @param[in] sha256 SHA256 hash to compare.
* @param[out] dst Destination struct.
*
* @return 0 if \a file and \a sha256 have the same hash, ERANGE
* if it differs, or any other errno value on error.
*/
int xbps_file_sha256_check_raw(const char *file, const char *sha256, struct xbps_sha256_digest *dst);

/**
* Verifies the RSA signature \a sigfile against \a digest with the
* RSA public-key associated in \a repo.
Expand All @@ -1962,7 +1996,7 @@ int xbps_file_sha256_check(const char *file, const char *sha256);
* @return True if the signature is valid, false otherwise.
*/
bool xbps_verify_signature(struct xbps_repo *repo, const char *sigfile,
unsigned char *digest);
const struct xbps_sha256_digest *digest);

/**
* Verifies the RSA signature of \a fname with the RSA public-key associated
Expand Down
3 changes: 3 additions & 0 deletions include/xbps_api_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,4 +161,7 @@ struct xbps_repo HIDDEN *xbps_regget_repo(struct xbps_handle *,
const char *);
int HIDDEN xbps_conf_init(struct xbps_handle *);

/* util */
int HIDDEN xbps_file_sig_path(char *str, size_t len, char **sigsuffix, const char *fmt, ...);

#endif /* !_XBPS_API_IMPL_H_ */
92 changes: 60 additions & 32 deletions lib/transaction_fetch.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <string.h>
#include <errno.h>
#include <unistd.h>
#include <limits.h>

#include "xbps_api_impl.h"

Expand All @@ -37,6 +38,7 @@ verify_binpkg(struct xbps_handle *xhp, xbps_dictionary_t pkgd)
struct xbps_repo *repo;
const char *pkgver, *repoloc, *sha256;
char *binfile;
struct xbps_sha256_digest digest;
int rv = 0;

xbps_dictionary_get_cstring_nocopy(pkgd, "repository", &repoloc);
Expand All @@ -47,44 +49,60 @@ verify_binpkg(struct xbps_handle *xhp, xbps_dictionary_t pkgd)
return ENOMEM;
}
/*
* For pkgs in local repos check the sha256 hash.
* For pkgs in remote repos check the RSA signature.
* For all pkgs, check the sha256 hash.
* For pkgs in local repos, check the RSA sig if requested.
* For pkgs in remote repos, always check the RSA signature.
*/
if ((repo = xbps_rpool_get_repo(repoloc)) == NULL) {
rv = errno;
xbps_dbg_printf(xhp, "%s: failed to get repository "
"%s: %s\n", pkgver, repoloc, strerror(errno));
goto out;
}
if (repo->is_remote) {
/* remote repo */

/* check sha256 */
xbps_set_cb_state(xhp, XBPS_STATE_VERIFY, 0, pkgver,
"%s: verifying SHA256 hash...", pkgver);
xbps_dictionary_get_cstring_nocopy(pkgd, "filename-sha256", &sha256);
if ((rv = xbps_file_sha256_check_raw(binfile, sha256, &digest)) != 0) {
xbps_set_cb_state(xhp, XBPS_STATE_VERIFY_FAIL, rv, pkgver,
"%s: SHA256 hash is not valid: %s", pkgver, strerror(rv));
goto out;
}

if (repo->is_remote || xhp->flags & XBPS_FLAG_VERIFY_LOCAL_REPO) {
char sigfile[PATH_MAX];
if ((rv = xbps_file_sig_path(sigfile, sizeof sigfile, NULL, "%s.sig", binfile))) {
xbps_set_cb_state(xhp, XBPS_STATE_VERIFY_FAIL, rv,
pkgver, "[trans] can't access signature for `%s': %s",
pkgver, strerror(rv));
goto out;
}

/* check RSA sig */
xbps_set_cb_state(xhp, XBPS_STATE_VERIFY, 0, pkgver,
"%s: verifying RSA signature...", pkgver);

if (!xbps_verify_file_signature(repo, binfile)) {
char *sigfile;
if (!xbps_verify_signature(repo, sigfile, &digest)) {
rv = EPERM;
xbps_set_cb_state(xhp, XBPS_STATE_VERIFY_FAIL, rv, pkgver,
"%s: the RSA signature is not valid!", pkgver);
xbps_set_cb_state(xhp, XBPS_STATE_VERIFY_FAIL, rv, pkgver,
"%s: removed pkg archive and its signature.", pkgver);
(void)remove(binfile);
sigfile = xbps_xasprintf("%s.sig", binfile);
(void)remove(sigfile);
free(sigfile);
goto out;
}
} else {
/* local repo */
xbps_set_cb_state(xhp, XBPS_STATE_VERIFY, 0, pkgver,
"%s: verifying SHA256 hash...", pkgver);
xbps_dictionary_get_cstring_nocopy(pkgd, "filename-sha256", &sha256);
if ((rv = xbps_file_sha256_check(binfile, sha256)) != 0) {
xbps_set_cb_state(xhp, XBPS_STATE_VERIFY_FAIL, rv, pkgver,
"%s: SHA256 hash is not valid: %s", pkgver, strerror(rv));
if (repo->is_remote) {
/*
* Don't remove files from local repositories, since we might
* not be the "owner"; with the XBPS cache, we are the owners.
*/
const char *errmsg;
if (remove(binfile) == 0 && remove(sigfile) == 0) {
errmsg = "removed pkg archive and its signature";
} else {
errmsg = "there was an error removing pkg archive and its signature";
}
xbps_set_cb_state(xhp, XBPS_STATE_VERIFY_FAIL, rv, pkgver,
"%s: %s.", pkgver, errmsg);
}
goto out;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This goto is, and was, excessive.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to keep it so people didn't forget to add it back if it became necessary to add more stuff to do after the conditionals are done. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine, let it stay.

}

}
out:
free(binfile);
Expand All @@ -95,10 +113,11 @@ static int
download_binpkg(struct xbps_handle *xhp, xbps_dictionary_t repo_pkgd)
{
struct xbps_repo *repo;
/* FIXME: increase max length for remote requests? */
char buf[PATH_MAX];
char *sigsuffix;
const char *pkgver, *arch, *fetchstr, *repoloc;
unsigned char digest[XBPS_SHA256_DIGEST_SIZE] = {0};
struct xbps_sha256_digest digest = {0};
int rv = 0;

xbps_dictionary_get_cstring_nocopy(repo_pkgd, "repository", &repoloc);
Expand All @@ -108,8 +127,12 @@ download_binpkg(struct xbps_handle *xhp, xbps_dictionary_t repo_pkgd)
xbps_dictionary_get_cstring_nocopy(repo_pkgd, "pkgver", &pkgver);
xbps_dictionary_get_cstring_nocopy(repo_pkgd, "architecture", &arch);

snprintf(buf, sizeof buf, "%s/%s.%s.xbps.sig", repoloc, pkgver, arch);
sigsuffix = buf+(strlen(buf)-sizeof (".sig")+1);
if ((rv = xbps_file_sig_path(buf, sizeof buf, &sigsuffix, "%s/%s.%s.xbps.sig", repoloc, pkgver, arch))) {
xbps_set_cb_state(xhp, XBPS_STATE_DOWNLOAD_FAIL, rv,
pkgver, "[trans] failed to create signature request for `%s': %s",
pkgver, strerror(rv));
return rv;
}

xbps_set_cb_state(xhp, XBPS_STATE_DOWNLOAD, 0, pkgver,
"Downloading `%s' signature (from `%s')...", pkgver, repoloc);
Expand All @@ -129,8 +152,8 @@ download_binpkg(struct xbps_handle *xhp, xbps_dictionary_t repo_pkgd)
xbps_set_cb_state(xhp, XBPS_STATE_DOWNLOAD, 0, pkgver,
"Downloading `%s' package (from `%s')...", pkgver, repoloc);

if ((rv = xbps_fetch_file_sha256(xhp, buf, NULL, digest,
sizeof digest)) == -1) {
if ((rv = xbps_fetch_file_sha256(xhp, buf, NULL, digest.buffer,
sizeof digest.buffer)) == -1) {
rv = fetchLastErrCode ? fetchLastErrCode : errno;
fetchstr = xbps_fetch_error_string();
xbps_set_cb_state(xhp, XBPS_STATE_DOWNLOAD_FAIL, rv,
Expand All @@ -143,8 +166,12 @@ download_binpkg(struct xbps_handle *xhp, xbps_dictionary_t repo_pkgd)
xbps_set_cb_state(xhp, XBPS_STATE_VERIFY, 0, pkgver,
"%s: verifying RSA signature...", pkgver);

snprintf(buf, sizeof buf, "%s/%s.%s.xbps.sig", xhp->cachedir, pkgver, arch);
sigsuffix = buf+(strlen(buf)-sizeof (".sig")+1);
if ((rv = xbps_file_sig_path(buf, sizeof buf, &sigsuffix, "%s/%s.%s.xbps.sig", xhp->cachedir, pkgver, arch))) {
xbps_set_cb_state(xhp, XBPS_STATE_DOWNLOAD_FAIL, rv,
pkgver, "[trans] failed to create signature path for `%s': %s",
pkgver, strerror(rv));
return rv;
}

if ((repo = xbps_rpool_get_repo(repoloc)) == NULL) {
rv = errno;
Expand All @@ -157,7 +184,8 @@ download_binpkg(struct xbps_handle *xhp, xbps_dictionary_t repo_pkgd)
* If digest is not set, binary package was not downloaded,
* i.e. 304 not modified, verify by file instead.
*/
if (*digest) {
/* FIXME: this rejects hashes that start with 0 */
if (digest.buffer[0]) {
*sigsuffix = '\0';
if (!xbps_verify_file_signature(repo, buf)) {
rv = EPERM;
Expand All @@ -168,7 +196,7 @@ download_binpkg(struct xbps_handle *xhp, xbps_dictionary_t repo_pkgd)
(void)remove(buf);
}
} else {
if (!xbps_verify_signature(repo, buf, digest)) {
if (!xbps_verify_signature(repo, buf, &digest)) {
rv = EPERM;
/* remove signature */
(void)remove(buf);
Expand Down
21 changes: 21 additions & 0 deletions lib/util.c
Original file line number Diff line number Diff line change
Expand Up @@ -699,3 +699,24 @@ xbps_patterns_match(xbps_array_t patterns, const char *path)

return match;
}

int HIDDEN xbps_file_sig_path(char *str, size_t len, char **sigsuffix, const char *fmt, ...)
{
va_list ap;
ssize_t w;
char *sig;

va_start(ap, fmt);
/* Bail out if resulting string is too long. */
if ((w = vsnprintf(str, len, fmt, ap)) >= (ssize_t)len) {
return ENAMETOOLONG;
}
sig = str + (w - sizeof(".sig")+1);
/* Check that we actually received a signature path. */
if (strcmp(sig, ".sig"))
return EINVAL;
if (sigsuffix)
*sigsuffix = sig;

return 0;
}
28 changes: 17 additions & 11 deletions lib/util_hash.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,18 +160,16 @@ xbps_file_sha256(char *dst, size_t dstlen, const char *file)
}

static bool
sha256_digest_compare(const char *sha256, size_t shalen,
const unsigned char *digest, size_t digestlen)
sha256_digest_compare(const char *sha256,
const struct xbps_sha256_digest *digest_struct)
{
const unsigned char *digest = digest_struct->buffer;

size_t shalen = strlen(sha256);
assert(shalen == XBPS_SHA256_SIZE - 1);
if (shalen != XBPS_SHA256_SIZE -1)
return false;

assert(digestlen == XBPS_SHA256_DIGEST_SIZE);
if (digestlen != XBPS_SHA256_DIGEST_SIZE)
return false;

for (; *sha256;) {
if (*digest / 16 < 10) {
if (*sha256++ != '0' + *digest / 16)
Expand All @@ -194,22 +192,30 @@ sha256_digest_compare(const char *sha256, size_t shalen,
}

int
xbps_file_sha256_check(const char *file, const char *sha256)
xbps_file_sha256_check_raw(const char *file, const char *sha256,
struct xbps_sha256_digest *dst)
{
unsigned char digest[XBPS_SHA256_DIGEST_SIZE];

assert(file != NULL);
assert(sha256 != NULL);
assert(dst != NULL);

if (!xbps_file_sha256_raw(digest, sizeof digest, file))
if (!xbps_file_sha256_raw(dst->buffer, sizeof dst->buffer, file))
return errno;

if (!sha256_digest_compare(sha256, strlen(sha256), digest, sizeof digest))
if (!sha256_digest_compare(sha256, dst))
return ERANGE;

return 0;
}

int
xbps_file_sha256_check(const char *file, const char *sha256)
{
struct xbps_sha256_digest digest;

return xbps_file_sha256_check_raw(file, sha256, &digest);
}

static const char *
file_hash_dictionary(xbps_dictionary_t d, const char *key, const char *file)
{
Expand Down
Loading