[Rpm-maint] [RFC PATCH v3 2/4] Sign package files and include signatures in package header
Panu Matilainen
pmatilai at laiskiainen.org
Fri Oct 31 08:45:25 UTC 2014
On 10/07/2014 11:19 PM, fin at linux.vnet.ibm.com wrote:
> From: Fionnuala Gunter <fin at linux.vnet.ibm.com>
>
> This patch extends the existing rpm signing tool to also sign package files
> and include them in the package header. It defines a tag,
> RPMTAG_FILESIGNATURES, an RPM macro %_file_signing_key, and new options
> fskpath, and signfiles.
>
> rpm --addsign [--signfiles [--fskpath <file signing key>]] PACKAGE_FILE ...
>
> The new option to rpmsign signs all the file digests included in the
> package. When a package is signed with the new option, the file digests are
> signed with libimaevm and the key %file_signing_key. The resulting
> signatures are included in the package header as an RPMTAG_FILESIGNATURES
> tag. Since the header is modified, the SHA1 and MD5 header digests are
> recalculated and inserted in the signature header.
>
> After including file signatures, the package is signed normally.
>
> Signed-off-by: Fionnuala Gunter <fin at linux.vnet.ibm.com>
> ---
> configure.ac | 8 ++
> doc/rpmsign.8 | 22 +++-
> lib/Makefile.am | 3 +-
> lib/rpmsignfiles.c | 130 ++++++++++++++++++++++
> lib/rpmsignfiles.h | 45 ++++++++
> lib/rpmtag.h | 1 +
> rpmpopt.in | 1 +
> rpmsign.c | 14 ++-
> sign/rpmgensig.c | 319 ++++++++++++++++++++++++++++++++++++++++++++++++-----
> sign/rpmsign.h | 7 +-
> 10 files changed, 520 insertions(+), 30 deletions(-)
> create mode 100644 lib/rpmsignfiles.c
> create mode 100644 lib/rpmsignfiles.h
>
> diff --git a/configure.ac b/configure.ac
> index cbb869f..21a6a95 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -658,6 +658,14 @@ AC_SUBST(WITH_SELINUX_LIB)
> AC_SUBST(WITH_SEMANAGE_LIB)
> AM_CONDITIONAL(SELINUX,[test "$with_selinux" = yes])
>
> +# libimaevm
> +with_iamevm=no
> +AC_ARG_WITH(imaevm, [AS_HELP_STRING([--with-imaevm],[build with imaevm support])])
> +if test "$with_imaevm" = yes ; then
> + AC_DEFINE(IMAEVM, 1, [Build with imaevm support?])
> + LIBS="$LIBS -limaevm"
> +fi
> +
> # libcap
> WITH_CAP_LIB=
> AC_ARG_WITH(cap, [AS_HELP_STRING([--with-cap],[build with capability support])],
> diff --git a/doc/rpmsign.8 b/doc/rpmsign.8
> index 53f2d70..b19f172 100644
> --- a/doc/rpmsign.8
> +++ b/doc/rpmsign.8
> @@ -2,11 +2,17 @@
> .SH NAME
> rpmsign \- RPM Package Signing
> .SH SYNOPSIS
> +.SS "SIGNING PACKAGES:"
> +.PP
>
> -\fBrpm\fR \fB--addsign|--resign\fR \fB\fIPACKAGE_FILE\fB\fR\fI ...\fR
> +\fBrpm\fR \fB--addsign|--resign\fR [\fBrpmsign-options\fR] \fB\fIPACKAGE_FILE\fB\fR\fI ...\fR
>
> \fBrpm\fR \fB--delsign\fR \fB\fIPACKAGE_FILE\fB\fR\fI ...\fR
>
> +.SS "rpmsign-options"
> +.PP
> + \fB--fskpath \fIKEY\fB\fR] [\fB--signfiles\fR]
> +
> .SH DESCRIPTION
> .PP
> Both of the \fB--addsign\fR and \fB--resign\fR
> @@ -20,6 +26,19 @@ there is no difference in behavior currently.
> .PP
> Delete all signatures from each package \fIPACKAGE_FILE\fR given.
>
> +.SS "SIGN OPTIONS"
> +.PP
> +.TP
> +\fB--fskpath \fIKEY\fB\fR
> +Used with \fB--signfiles\fR, use file signing key \fIKEY\fR.
> +.TP
> +\fB--signfiles\fR
> +Sign package files. The macro \fB%_binary_filedigest_algorithm\fR must be set
> +before building the package, and the macro must be set to a supported algorithm:
> +2, 8, 9, or 10, which represent SHA1, SHA256, SHA384, and SHA512, respectively.
> +The file signing key (RSA private key) can be configured on the command line
> +with \fB--fskpath\fR or the macro \fB%_file_signing_key\fR.
> +
> .SS "USING GPG TO SIGN PACKAGES"
> .PP
> In order to sign packages using GPG, \fBrpm\fR
> @@ -78,4 +97,5 @@ Marc Ewing <marc at redhat.com>
> Jeff Johnson <jbj at redhat.com>
> Erik Troan <ewt at redhat.com>
> Panu Matilainen <pmatilai at redhat.com>
> +Fionnuala Gunter <fin at linux.vnet.ibm.com>
> .fi
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index a65eb80..f80a47a 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -38,7 +38,8 @@ librpm_la_SOURCES = \
> verify.c rpmlock.c rpmlock.h misc.h relocation.c \
> rpmscript.h rpmscript.c legacy.c \
> rpmchroot.c rpmchroot.h \
> - rpmplugins.c rpmplugins.h rpmplugin.h rpmug.c rpmug.h
> + rpmplugins.c rpmplugins.h rpmplugin.h rpmug.c rpmug.h \
> + rpmsignfiles.c rpmsignfiles.h
>
> librpm_la_LDFLAGS = -version-info $(rpm_version_info)
>
> diff --git a/lib/rpmsignfiles.c b/lib/rpmsignfiles.c
> new file mode 100644
> index 0000000..9c33103
> --- /dev/null
> +++ b/lib/rpmsignfiles.c
> @@ -0,0 +1,130 @@
> +/**
> + * Copyright (C) 2014 IBM Corporation
> + *
> + * Author: Fionnuala Gunter <fin at linux.vnet.ibm.com>
> + */
> +
> +#include "system.h"
> +#include "imaevm.h"
> +
> +#include <rpm/rpmlog.h> /* rpmlog */
> +#include <rpm/rpmstring.h> /* rnibble */
> +#include <rpm/rpmpgp.h> /* rpmDigestLength */
> +#include "lib/header.h" /* HEADERGET_MINMEM */
> +#include "lib/rpmtypes.h" /* rpmRC */
> +
> +#include "lib/rpmsignfiles.h"
> +
> +static char *rpmDigestAlgo(uint32_t dalgo)
> +{
> + switch (dalgo) {
> + case 0:
> + case 1: return "md5";
> + case 2: return "sha1";
> + case 8: return "sha256";
> + case 9: return "sha384";
> + case 10: return "sha512";
> + default: return NULL;
> + }
> +}
This is returning pointers to string literals, the return type must be
"const char *"
> +
> +char *getDigestAlgo(Header h)
> +{
> + struct rpmtd_s digalgo;
> + uint32_t *dalgo;
> + char *algo;
> +
> + headerGet(h, RPMTAG_FILEDIGESTALGO, &digalgo, HEADERGET_MINMEM);
> + dalgo = rpmtdGetUint32(&digalgo);
> + if (!dalgo) {
> + rpmlog(RPMLOG_ERR, _("rpmtdGetUint32 failed\n"));
> + return NULL;
> + }
> + algo = rpmDigestAlgo(*dalgo);
> + return algo;
> +}
> +
> +int getDigestLength(Header h)
> +{
> + struct rpmtd_s digalgo;
> + uint32_t *dalgo;
> + int diglen;
> +
> + headerGet(h, RPMTAG_FILEDIGESTALGO, &digalgo, HEADERGET_MINMEM);
> + dalgo = rpmtdGetUint32(&digalgo);
> + if (!dalgo) {
> + rpmlog(RPMLOG_ERR, _("rpmtdGetUint32 failed\n"));
> + return -1;
> + }
> +
> + diglen = rpmDigestLength(*dalgo);
> + return diglen;
> +}
This is a really complicated way of getting a integer or two out of a
header. headerGetNumber(h, RPMTAG_FILEDIGESTALGO) will give you that it
one line, and I dont quite see a need for such helpers exported within rpm.
Also these assume RPMTAG_FILEDIGESTALGO always exists, it does not
(which means md5 is used)
> +
> +char *signFile(const char *algo, const char *fdigest, int diglen, const char *key)
> +{
> + char *fsignature;
> + unsigned char digest[BUFSIZ];
BUFSIZ? The digest length is known, calculate the right size from that.
> + unsigned char signature[BUFSIZ];
> + int siglen;
> +
> +#ifndef IMAEVM
> + rpmlog(RPMLOG_ERR, _("missing libimaevm\n"));
> + return NULL;
> +#endif
> +
> + /* convert file digest hex to binary */
> + memset(digest, 0, BUFSIZ);
> + for (int i = 0; i < diglen; ++i, fdigest += 2)
> + digest[i] = (rnibble(fdigest[0]) << 4) | rnibble(fdigest[1]);
> +
> + /* prepare file signature */
> + memset(signature, 0, BUFSIZ);
> + signature[0] = '\x03';
> +
> + /* calculate file signature */
> + siglen = sign_hash(algo, digest, diglen, key, signature+1);
This looks gets() -class unsafe to me. How do you guarantee the buffer
passed in is large enough? Certainly BUFSIZ seems wrong as its a
platform dependent thing, it might be "large enough" on Linux but then
its by no means obvious. How large are these signatures anyway?
> + if (siglen < 0) {
> + rpmlog(RPMLOG_ERR, _("sign_hash failed\n"));
> + return NULL;
> + }
> +
> + /* convert file signature binary to hex */
> + fsignature = pgpHexStr(signature, siglen+1);
> + return fsignature;
> +}
I guess sign_hash() is from the IMA library, and another library
requiring translating between hash algorithm values is understandable,
but there's no need to expose that to rpm at all. Everywhere in rpm,
digest algorithms are integer values, and that's how this should take
the argument as well. The rest of rpm does not need to care.
> +
> +rpmRC signFiles(Header h, const char *key)
> +{
> + struct rpmtd_s digests;
> + const char *algo;
> + const char *digest;
> + char *signature;
> + int diglen;
> +
> + algo = getDigestAlgo(h);
> + if (!algo) {
> + rpmlog(RPMLOG_ERR, _("getDigestAlgo failed\n"));
> + return RPMRC_FAIL;
> + }
> +
> + diglen = getDigestLength(h);
> + if (diglen < 0) {
> + rpmlog(RPMLOG_ERR, _("getDigestLength failed\n"));
> + return RPMRC_FAIL;
> + }
Almost certainly any error here would get double-logged, first from the
getDigestFoo() function and then here. The whole thing is as simple as
unsigned int algval = headerGetNumber(h, RPMTAG_FILEDIGESTALGO);
int diglen = rpmDigestLength(algval);
...and then pass the algval integer to your the conversion function to
get something suitable for sign_hash() and *then* check for the error.
There's little point i
> + headerGet(h, RPMTAG_FILEDIGESTS, &digests, HEADERGET_MINMEM);
> + while ((digest = rpmtdNextString(&digests))) {
> + signature = signFile(algo, digest, diglen, key);
> + if (!signature) {
> + rpmlog(RPMLOG_ERR, _("signFile failed\n"));
> + return RPMRC_FAIL;
> + }
> + if (!headerPutString(h, RPMTAG_FILESIGNATURES, signature)) {
> + rpmlog(RPMLOG_ERR, _("headerPutString failed\n"));
> + return RPMRC_FAIL;
> + }
> + }
> + return RPMRC_OK;
> +}
> diff --git a/lib/rpmsignfiles.h b/lib/rpmsignfiles.h
> new file mode 100644
> index 0000000..05459f0
> --- /dev/null
> +++ b/lib/rpmsignfiles.h
> @@ -0,0 +1,45 @@
> +#ifndef H_RPMSIGNFILES
> +#define H_RPMSIGNFILES
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/**
> + * Get file digest algorithm from header
> + * @param header package header
> + * @return file digest algorithm
> + */
> +char *getDigestAlgo(Header h);
> +
> +/**
> + * Get file digest length from header
> + * @param header package header
> + * @return file digest length
> + */
> +int getDigestLength(Header h);
> +
> +/**
> + * Sign a file digest with libimaevm
> + * @param algo file digest algorithm
> + * @param fdigest file digest hex
> + * @param diglen digest length
> + * @param key file signing key
> + * @return file signature hex
> + */
> +char *signFile(const char *algo, const char *fdigest, int diglen,
> + const char *key);
> +
> +/**
> + * Sign file digests in header and store the signatures in header
> + * @param h package header
> + * @param key signing key
> + * @return RPMRC_OK on success
> + */
> +rpmRC signFiles(Header h, const char *key);
Any new non-static functions added to the libraries should be rpm-prefixed.
> +#ifdef _cplusplus
> +}
> +#endif
> +
> +#endif /* H_RPMSIGNFILES */
> diff --git a/lib/rpmtag.h b/lib/rpmtag.h
> index 58b2479..bd6a36a 100644
> --- a/lib/rpmtag.h
> +++ b/lib/rpmtag.h
> @@ -329,6 +329,7 @@ typedef enum rpmTag_e {
> RPMTAG_SUPPLEMENTNEVRS = 5060, /* s[] extension */
> RPMTAG_ENHANCENEVRS = 5061, /* s[] extension */
> RPMTAG_ENCODING = 5062, /* s */
> + RPMTAG_FILESIGNATURES = 5063, /* s[] */
>
> RPMTAG_FIRSTFREE_TAG /*!< internal */
> } rpmTag;
> diff --git a/rpmpopt.in b/rpmpopt.in
> index 036ab4e..df5e2ec 100644
> --- a/rpmpopt.in
> +++ b/rpmpopt.in
> @@ -162,6 +162,7 @@ rpm alias --httpproxy --define '_httpproxy !#:+'
> rpm exec --addsign rpmsign --addsign
> rpm exec --delsign rpmsign --delsign
> rpm exec --resign rpmsign --resign
> +#rpm exec --signfiles rpmsign --signfiles
> rpm exec --checksig rpmkeys --checksig
> rpm exec -K rpmkeys --checksig
> rpm exec --import rpmkeys --import
> diff --git a/rpmsign.c b/rpmsign.c
> index b8e5598..a2692b9 100644
> --- a/rpmsign.c
> +++ b/rpmsign.c
> @@ -20,6 +20,9 @@ enum modes {
>
> static int mode = 0;
>
> +static int signfiles = 0;
> +static char * fileSigningKey = NULL;
> +
> static struct poptOption signOptsTable[] = {
> { "addsign", '\0', (POPT_ARG_VAL|POPT_ARGFLAG_OR), &mode, MODE_ADDSIGN,
> N_("sign package(s)"), NULL },
> @@ -27,6 +30,11 @@ static struct poptOption signOptsTable[] = {
> N_("sign package(s) (identical to --addsign)"), NULL },
> { "delsign", '\0', (POPT_ARG_VAL|POPT_ARGFLAG_OR), &mode, MODE_DELSIGN,
> N_("delete package signatures"), NULL },
> + { "signfiles", '\0', POPT_ARG_NONE, &signfiles, 0,
> + N_("sign package(s) files"), NULL},
> + { "fskpath", '\0', POPT_ARG_STRING, &fileSigningKey, 0,
> + N_("use file signing key <key>"),
> + N_("<key>") },
> POPT_TABLEEND
> };
>
> @@ -119,6 +127,10 @@ static int doSign(poptContext optCon)
> goto exit;
> }
>
> + if (fileSigningKey && signfiles) {
> + addMacro(NULL, "_file_signing_key", NULL, fileSigningKey, RMIL_GLOBAL);
> + }
> +
> /* XXX FIXME: eliminate obsolete getpass() usage */
> passPhrase = getpass(_("Enter pass phrase: "));
> passPhrase = (passPhrase != NULL) ? rstrdup(passPhrase) : NULL;
> @@ -127,7 +139,7 @@ static int doSign(poptContext optCon)
> fprintf(stderr, _("Pass phrase is good.\n"));
> rc = 0;
> while ((arg = poptGetArg(optCon)) != NULL) {
> - rc += rpmPkgSign(arg, NULL, passPhrase);
> + rc += rpmPkgSign(arg, NULL, passPhrase, signfiles);
> }
> } else {
> fprintf(stderr, _("Pass phrase check failed or gpg key expired\n"));
> diff --git a/sign/rpmgensig.c b/sign/rpmgensig.c
> index 0bd14e3..f07c87e 100644
> --- a/sign/rpmgensig.c
> +++ b/sign/rpmgensig.c
> @@ -1,4 +1,10 @@
> -/** \ingroup rpmcli
> +/**
> + * Copyright (C) 2014 IBM Corporation
> + *
> + * Author: Fionnuala Gunter <fin at linux.vnet.ibm.com>
> + * added support for file signatures
> + *
> + * \ingroup rpmcli
> * \file lib/rpmchecksig.c
> * Verify the signature of a package.
> */
> @@ -8,17 +14,23 @@
> #include <errno.h>
> #include <sys/wait.h>
> #include <popt.h>
> +#include <imaevm.h>
> +#include <ctype.h>
>
> #include <rpm/rpmlib.h> /* RPMSIGTAG & related */
> #include <rpm/rpmmacro.h>
> #include <rpm/rpmpgp.h>
> #include <rpm/rpmsign.h>
> -#include <rpm/rpmfileutil.h> /* rpmMkTemp() */
> +#include <rpm/rpmfileutil.h> /* rpmMkTemp() */
> #include <rpm/rpmlog.h>
> #include <rpm/rpmstring.h>
>
> #include "lib/rpmlead.h"
> #include "lib/signature.h"
> +#include "lib/header.h"
> +#include "lib/cpio.h" /* rpmcpioOpen, rpmcpioTell */
> +#include "lib/rpmsignfiles.h" /* signFiles */
> +#include "rpmio/rpmio_internal.h" /* fdInitDigest, fdFiniDigest */
>
> #include "debug.h"
>
> @@ -88,6 +100,10 @@ static int manageFile(FD_t *fdp, const char *fn, int flags)
>
> /**
> * Copy header+payload, calculating digest(s) on the fly.
> + * @param sfdp source file
> + * @param sfnp source path
> + * @param tfdp destination file
> + * @param tfnp destination path
> */
> static int copyFile(FD_t *sfdp, const char *sfnp,
> FD_t *tfdp, const char *tfnp)
> @@ -121,8 +137,6 @@ static int copyFile(FD_t *sfdp, const char *sfnp,
> rc = 0;
>
> exit:
> - if (*sfdp) (void) closeFile(sfdp);
> - if (*tfdp) (void) closeFile(tfdp);
> return rc;
> }
>
> @@ -458,14 +472,277 @@ static int replaceSignature(Header sigh, sigTarget sigt1, sigTarget sigt2,
> return rc;
> }
>
> +static rpmRC generateSignature(char *SHA1, uint8_t *MD5, rpm_loff_t size,
> + rpm_loff_t payloadSize, FD_t fd)
> +{
> + Header sig = NULL;
> + struct rpmtd_s td;
> + rpmTagVal sizetag;
> + rpmTagVal payloadtag;
> + rpm_tagtype_t typetag;
> + rpmRC rc = RPMRC_OK;
> + char *reservedSpace;
> + int spaceSize = 0;
> +
> + /* Prepare signature */
> + sig = rpmNewSignature();
> +
> + rpmtdReset(&td);
> + td.tag = RPMSIGTAG_SHA1;
> + td.count = 1;
> + td.type = RPM_STRING_TYPE;
> + td.data = SHA1;
> + headerPut(sig, &td, HEADERPUT_DEFAULT);
> +
> + rpmtdReset(&td);
> + td.tag = RPMSIGTAG_MD5;
> + td.count = 16;
> + td.type = RPM_BIN_TYPE;
> + td.data = MD5;
> + headerPut(sig, &td, HEADERPUT_DEFAULT);
> +
> + if (payloadSize < UINT32_MAX) {
> + sizetag = RPMSIGTAG_SIZE;
> + payloadtag = RPMSIGTAG_PAYLOADSIZE;
> + typetag = RPM_INT32_TYPE;
> + } else {
> + sizetag = RPMSIGTAG_LONGSIZE;
> + payloadtag = RPMSIGTAG_LONGARCHIVESIZE;
> + typetag = RPM_INT64_TYPE;
> + }
> +
> + rpmtdReset(&td);
> + td.tag = payloadtag;
> + td.count = 1;
> + td.type = typetag;
> + td.data = &payloadSize;
> + headerPut(sig, &td, HEADERPUT_DEFAULT);
> +
> + rpmtdReset(&td);
> + td.tag = sizetag;
> + td.count = 1;
> + td.type = typetag;
> + td.data = &size;
> + headerPut(sig, &td, HEADERPUT_DEFAULT);
> +
> + spaceSize = rpmExpandNumeric("%{__gpg_reserved_space}");
> + if(spaceSize > 0) {
> + reservedSpace = xcalloc(spaceSize, sizeof(char));
> + rpmtdReset(&td);
> + td.tag = RPMSIGTAG_RESERVEDSPACE;
> + td.count = spaceSize;
> + td.type = RPM_BIN_TYPE;
> + td.data = reservedSpace;
> + headerPut(sig, &td, HEADERPUT_DEFAULT);
> + free(reservedSpace);
> + }
> +
> + /* Reallocate the signature into one contiguous region. */
> + sig = headerReload(sig, RPMTAG_HEADERSIGNATURES);
> + if (sig == NULL) { /* XXX can't happen */
> + rpmlog(RPMLOG_ERR, _("Unable to reload signature header.\n"));
> + rc = RPMRC_FAIL;
> + goto exit;
> + }
> +
> + /* Write the signature section into the package. */
> + if (rpmWriteSignature(fd, sig)) {
> + rc = RPMRC_FAIL;
> + goto exit;
> + }
> +
> +exit:
> + rpmFreeSignature(sig);
> + return rc;
> +}
> +
> +static void unloadImmutableRegion(Header *hdrp, rpmTagVal tag, rpmtd utd)
> +{
> + struct rpmtd_s copytd;
> + Header nh;
> + Header oh;
> + HeaderIterator hi;
> +
> + if (headerGet(*hdrp, tag, utd, HEADERGET_DEFAULT)) {
> + nh = headerNew();
> + oh = headerCopyLoad(utd->data);
> + hi = headerInitIterator(oh);
> +
> + while (headerNext(hi, ©td)) {
> + if (copytd.data)
> + headerPut(nh, ©td, HEADERPUT_DEFAULT);
> + rpmtdFreeData(©td);
> + }
> +
> + headerFreeIterator(hi);
> + headerFree(oh);
> + rpmtdFreeData(utd);
> +
> + *hdrp = headerLink(nh);
> + headerFree(nh);
> + }
> +}
Refactoring to smaller/saner functions etc is welcome, but it needs to
be done as a separate patch. Ie first refactor to your liking in as many
patches as needed, then add the new functionality as a last separate patch.
> +/**
> + * Include file signatures in header
> + * @param fd
> + * @param rpm path to package
> + * @param sigp pointer to signature header
> + * @param hdrp pointer to header
> + * @param sigStart signature offset in rpm
> + * @param headerStart header offset in rpm
> + */
> +static rpmRC includeFileSignatures(FD_t fd, const char *rpm,
> + Header *sigp, Header *hdrp,
> + off_t sigStart, off_t headerStart)
> +{
> + FD_t ofd = NULL;
> + struct rpmtd_s td;
> + char *trpm = NULL;
> + const char *key;
> + char *SHA1 = NULL;
> + uint8_t *MD5 = NULL;
> + unsigned char buf[32*BUFSIZ];
> + size_t sha1len;
> + size_t md5len;
> + size_t headerSize;
> + off_t archiveSize;
> + rpmRC rc = RPMRC_OK;
> +
> + unloadImmutableRegion(hdrp, RPMTAG_HEADERIMMUTABLE, &td);
> +
> + key = rpmExpand("%{?_file_signing_key}", NULL);
> + if (rstreq(key, "")) {
> + rc = RPMRC_FAIL;
> + fprintf(stderr, _("You must set \"$$_file_signing_key\" in your macro file or on the command line with --fskpath\n"));
> + rpmlog(RPMLOG_ERR, _("no file signing key provided\n"));
> + goto exit;
> + }
> +
> + rc = signFiles(*hdrp, key);
> + if (rc != RPMRC_OK) {
> + rpmlog(RPMLOG_ERR, _("signFiles failed\n"));
> + goto exit;
> + }
> +
> + *hdrp = headerReload(*hdrp, RPMTAG_HEADERIMMUTABLE);
> + if (*hdrp == NULL) {
> + rc = RPMRC_FAIL;
> + rpmlog(RPMLOG_ERR, _("headerReload failed\n"));
> + goto exit;
> + }
> +
> + ofd = rpmMkTempFile(NULL, &trpm);
> + if (ofd == NULL || Ferror(ofd)) {
> + rc = RPMRC_FAIL;
> + rpmlog(RPMLOG_ERR, _("rpmMkTemp failed\n"));
> + goto exit;
> + }
> +
> + /* Copy archive to temp file */
> + if (copyFile(&fd, rpm, &ofd, trpm)) {
> + rc = RPMRC_FAIL;
> + rpmlog(RPMLOG_ERR, _("copyFile failed\n"));
> + goto exit;
> + }
> +
> + if (Fseek(fd, headerStart, SEEK_SET) < 0) {
> + rc = RPMRC_FAIL;
> + rpmlog(RPMLOG_ERR, _("Could not seek in file %s: %s\n"),
> + rpm, Fstrerror(fd));
> + goto exit;
> + }
> +
> + /* Write header to rpm and recalculate SHA1 */
> + fdInitDigest(fd, PGPHASHALGO_SHA1, 0);
> + rc = headerWrite(fd, *hdrp, HEADER_MAGIC_YES);
> + if (rc != RPMRC_OK) {
> + rpmlog(RPMLOG_ERR, _("headerWrite failed\n"));
> + goto exit;
> + }
> + fdFiniDigest(fd, PGPHASHALGO_SHA1, (void **)&SHA1, &sha1len, 1);
> + headerSize = Ftell(fd) - headerStart;
> +
> + /* Copy archive from temp file */
> + if (Fseek(ofd, 0, SEEK_SET) < 0) {
> + rc = RPMRC_FAIL;
> + rpmlog(RPMLOG_ERR, _("Could not seek in file %s: %s\n"),
> + rpm, Fstrerror(fd));
> + goto exit;
> + }
> + if (copyFile(&ofd, trpm, &fd, rpm)) {
> + rc = RPMRC_FAIL;
> + rpmlog(RPMLOG_ERR, _("copyFile failed\n"));
> + goto exit;
> + }
> + unlink(trpm);
> +
> + /* Recalculate MD5 digest of header+archive */
> + if (Fseek(fd, headerStart, SEEK_SET) < 0) {
> + rc = RPMRC_FAIL;
> + rpmlog(RPMLOG_ERR, _("Could not seek in file %s: %s\n"),
> + rpm, Fstrerror(fd));
> + goto exit;
> + }
> + fdInitDigest(fd, PGPHASHALGO_MD5, 0);
> +
> + while (Fread(buf, sizeof(buf[0]), sizeof(buf), fd) > 0)
> + ;
> + if (Ferror(fd)) {
> + rc = RPMRC_FAIL;
> + rpmlog(RPMLOG_ERR, _("Fread failed in file %s: %s\n"),
> + rpm, Fstrerror(fd));
> + goto exit;
> + }
> + fdFiniDigest(fd, PGPHASHALGO_MD5, (void **)&MD5, &md5len, 0);
> +
> + if (Fseek(fd, sigStart, SEEK_SET) < 0) {
> + rc = RPMRC_FAIL;
> + rpmlog(RPMLOG_ERR, _("Could not seek in file %s: %s\n"),
> + rpm, Fstrerror(fd));
> + goto exit;
> + }
> +
> + /* Get payload size from signature tag */
> + archiveSize = headerGetNumber(*sigp, RPMSIGTAG_PAYLOADSIZE);
> + if (!archiveSize) {
> + archiveSize = headerGetNumber(*sigp, RPMSIGTAG_LONGARCHIVESIZE);
> + }
> +
> + /* Replace old digests in signature */
> + rc = generateSignature(SHA1, MD5, headerSize, archiveSize, fd);
> + if (rc != RPMRC_OK) {
> + rpmlog(RPMLOG_ERR, _("insertDigests failed\n"));
> + goto exit;
> + }
> +
> + if (Fseek(fd, sigStart, SEEK_SET) < 0) {
> + rc = RPMRC_FAIL;
> + rpmlog(RPMLOG_ERR, _("Could not seek in file %s: %s\n"),
> + rpm, Fstrerror(fd));
> + goto exit;
> + }
> +
> + rc = rpmReadSignature(fd, sigp, RPMSIGTYPE_HEADERSIG, NULL);
> + if (rc != RPMRC_OK) {
> + rpmlog(RPMLOG_ERR, _("rpmReadSignature failed\n"));
> + goto exit;
> + }
> +
> +exit:
> + return rc;
> +}
> +
> /** \ingroup rpmcli
> * Create/modify elements in signature header.
> * @param rpm path to package
> * @param deleting adding or deleting signature?
> * @param passPhrase passPhrase (ignored when deleting)
> + * @param signfiles sign files if non-zero
> * @return 0 on success, -1 on error
> */
> -static int rpmSign(const char *rpm, int deleting, const char *passPhrase)
> +static int rpmSign(const char *rpm, int deleting, const char *passPhrase,
> + int signfiles)
> {
> FD_t fd = NULL;
> FD_t ofd = NULL;
> @@ -516,25 +793,15 @@ static int rpmSign(const char *rpm, int deleting, const char *passPhrase)
> goto exit;
> }
>
> - /* Dump the immutable region (if present). */
> - if (headerGet(sigh, RPMTAG_HEADERSIGNATURES, &utd, HEADERGET_DEFAULT)) {
> - struct rpmtd_s copytd;
> - Header nh = headerNew();
> - Header oh = headerCopyLoad(utd.data);
> - HeaderIterator hi = headerInitIterator(oh);
> - while (headerNext(hi, ©td)) {
> - if (copytd.data)
> - headerPut(nh, ©td, HEADERPUT_DEFAULT);
> - rpmtdFreeData(©td);
> + if (signfiles) {
> + rc = includeFileSignatures(fd, rpm, &sigh, &h, sigStart, headerStart);
> + if (rc != RPMRC_OK) {
> + rpmlog(RPMLOG_ERR, _("includeFileSignatures failed\n"));
> + goto exit;
> }
> - headerFreeIterator(hi);
> - headerFree(oh);
> - rpmtdFreeData(&utd);
> -
> - headerFree(sigh);
> - sigh = headerLink(nh);
> - headerFree(nh);
> }
> +
> + unloadImmutableRegion(&sigh, RPMTAG_HEADERSIGNATURES, &utd);
> origSigSize = headerSizeof(sigh, HEADER_MAGIC_YES);
Maybe I'm missing something but this certainly looks like its doing a
whole lot of work twice when files are being signed.
>
> if (deleting) { /* Nuke all the signature tags. */
> @@ -566,6 +833,7 @@ static int rpmSign(const char *rpm, int deleting, const char *passPhrase)
>
> /* Try to make new signature smaller to have size of original signature */
> rpmtdReset(&utd);
> +
> if (headerGet(sigh, RPMSIGTAG_RESERVEDSPACE, &utd, HEADERGET_MINMEM)) {
> int diff;
> int count;
> @@ -668,7 +936,8 @@ exit:
> }
>
> int rpmPkgSign(const char *path,
> - const struct rpmSignArgs * args, const char *passPhrase)
> + const struct rpmSignArgs * args, const char *passPhrase,
> + int signfiles)
> {
> int rc;
>
> @@ -684,7 +953,7 @@ int rpmPkgSign(const char *path,
> }
> }
>
> - rc = rpmSign(path, 0, passPhrase);
> + rc = rpmSign(path, 0, passPhrase, signfiles);
>
> if (args) {
> if (args->hashalgo) {
> @@ -700,5 +969,5 @@ int rpmPkgSign(const char *path,
>
> int rpmPkgDelSign(const char *path)
> {
> - return rpmSign(path, 1, NULL);
> + return rpmSign(path, 1, NULL, 0);
> }
> diff --git a/sign/rpmsign.h b/sign/rpmsign.h
> index 15b3e0f..0ec37c1 100644
> --- a/sign/rpmsign.h
> +++ b/sign/rpmsign.h
> @@ -11,6 +11,8 @@ extern "C" {
> struct rpmSignArgs {
> char *keyid;
> pgpHashAlgo hashalgo;
> + int signFiles;
> + const char *fileSigningKey;
> /* ... what else? */
> };
>
> @@ -19,10 +21,11 @@ struct rpmSignArgs {
> * @param path path to package
> * @param args signing parameters (or NULL for defaults)
> * @param passPhrase passphrase for the signing key
> + * @param signfiles sign files if non-zero
> * @return 0 on success
> */
> -int rpmPkgSign(const char *path,
> - const struct rpmSignArgs * args, const char *passPhrase);
> +int rpmPkgSign(const char *path, const struct rpmSignArgs * args,
> + const char *passPhrase, int signfiles);
You're already adding a signFiles integer to struct rpmSignArgs. Why
should it ALSO be passed as a separate argument, breaking not only ABI
but the API as well? Changing internal interfaces is fine (ie the
rpmSign() case) but public API shouldn't be broken without a good
reason, and never when there's just no good reason to do so.
Oh and btw, this patch is still too large to swallow in one gulp. This
is only a cursory review and I haven't really looked at the actual
functionality here at all yet.
- Panu -
More information about the Rpm-maint
mailing list