[Rpm-maint] [PATCH v7 09/11] Add file signature support to package signing
Lubos Kardos
lkardos at redhat.com
Wed Jul 22 07:33:56 UTC 2015
----- Original Message -----
> From: "Fionnuala Gunter" <fionnuala.gunter at gmail.com>
> To: "Lubos Kardos" <lkardos at redhat.com>
> Cc: rpm-maint at lists.rpm.org, ffesti at redhat.org, "Mimi Zohar" <zohar at linux.vnet.ibm.com>, "fin gunter"
> <fin.gunter at hypori.com>
> Sent: Tuesday, July 21, 2015 9:56:19 PM
> Subject: Re: [PATCH v7 09/11] Add file signature support to package signing
>
> On Tue, Jul 21, 2015 at 3:18 AM, Lubos Kardos <lkardos at redhat.com> wrote:
>
> >
> > ----- Original Message -----
> > > From: "Fionnuala Gunter" <fionnuala.gunter at gmail.com>
> > > To: rpm-maint at lists.rpm.org
> > > Cc: ffesti at redhat.org, zohar at linux.vnet.ibm.com, lkardos at redhat.com,
> > "fin gunter" <fin.gunter at hypori.com>,
> > > fin at linux.vnet.ibm.com
> > > Sent: Tuesday, July 21, 2015 12:11:25 AM
> > > Subject: [PATCH v7 09/11] Add file signature support to package signing
> > >
> > > From: "fin at linux.vnet.ibm.com" <fin at linux.vnet.ibm.com>
> > >
> > > This patch modifies rpmSign to include file signatures in the header.
> > > Since the header is altered, the package digest and package+archive
> > > digest need to be recalculated and updated in the signature header.
> > > Defer resigning the header digests to replaceSignature().
> > >
> > > Changelog:
> > > - removed extraneous semicolon - Mimi
> > > - Update signature header digests only if necessary - Mimi
> > > - rename headerSize to more accurate name sigTargetSize - Fin
> > Maybe I should have been more clear. Renaming the variable is not enough.
> > It
> > still contains just header size but it should contain header size + size of
> > compressed payload. Have a look at inline comments.
> >
> I don't think that's right. According to rpm man page, the SHA1 digest is
> for the immutable header region, and the MD5 digest is for the combined
> header and payload.
I am speaking about the size not about the digests. That sigTargetSize is
passed to rpmGenerateSignature() where is stored in a signature header with
the tag RPMSIGTAG_SIZE/RPMSIGTAG_LONGSIZE. And you can have a look at
lib/rpmtag.h to find out what this tag means:
$ grep "RPMSIGTAG_SIZE\|RPMSIGTAG_LONGSIZE" lib/rpmtag.h
RPMSIGTAG_SIZE = 1000, /*!< internal Header+Payload size (32bit) in bytes. */
RPMSIGTAG_LONGSIZE = RPMTAG_LONGSIGSIZE, /*!< internal Header+Payload size (64bit) in bytes. */
You can also try to sign files in some package and examine the tag
RPMSIGTAG_SIZE/RPMSIGTAG_LONGSIZE before and after signing.
>
> >
> > Now I also noticed that you don't need to go through the same data twice.
> > The first time to calculate SHA1 and the second time to calculate MD5. You
> > can calculate both digest at the same time. Have a look at inline comments.
> >
> You are right about combining the MD5 calculation with SHA1, that can be
> simplified.
>
> >
> > > - deallocate sigp before it's overriden - Fin
> > > - fix dependency on ima - Fin
> > > - replace utd parameter with a local variable - Mimi
> > > ---
> > > lib/rpmsignfiles.c | 2 +-
> > > sign/rpmgensig.c | 185
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++--
> > > 2 files changed, 180 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/lib/rpmsignfiles.c b/lib/rpmsignfiles.c
> > > index 376f47e..414983b 100644
> > > --- a/lib/rpmsignfiles.c
> > > +++ b/lib/rpmsignfiles.c
> > > @@ -37,7 +37,7 @@ static char *signFile(const char *algo, const char
> > > *fdigest, int diglen, const c
> > > unsigned char signature[MAX_SIGNATURE_LENGTH];
> > > int siglen;
> > >
> > > -#ifndef IMAEVM
> > > +#ifndef WITH_IMAEVM
> > > rpmlog(RPMLOG_ERR, _("missing libimaevm\n"));
> > > return NULL;
> > > #endif
> > > diff --git a/sign/rpmgensig.c b/sign/rpmgensig.c
> > > index b0bed44..d5557dc 100644
> > > --- a/sign/rpmgensig.c
> > > +++ b/sign/rpmgensig.c
> > > @@ -17,9 +17,11 @@
> > > #include <rpm/rpmfileutil.h> /* rpmMkTemp() */
> > > #include <rpm/rpmlog.h>
> > > #include <rpm/rpmstring.h>
> > > +#include <rpmio/rpmio_internal.h>
> > >
> > > #include "lib/rpmlead.h"
> > > #include "lib/signature.h"
> > > +#include "lib/rpmsignfiles.h"
> > >
> > > #include "debug.h"
> > >
> > > @@ -471,9 +473,10 @@ static int replaceSignature(Header sigh, sigTarget
> > > sigt1, sigTarget sigt2)
> > > return rc;
> > > }
> > >
> > > -static void unloadImmutableRegion(Header *hdrp, rpmTagVal tag, rpmtd
> > utd)
> > > +static void unloadImmutableRegion(Header *hdrp, rpmTagVal tag)
> > > {
> > > - struct rpmtd_s copytd;
> > > + struct rpmtd_s copytd, td;
> > > + rpmtd utd = &td;
> > > Header nh;
> > > Header oh;
> > > HeaderIterator hi;
> > > @@ -498,13 +501,179 @@ static void unloadImmutableRegion(Header *hdrp,
> > > rpmTagVal tag, rpmtd utd)
> > > }
> > > }
> > >
> > > +static rpmRC replaceSigDigests(FD_t fd, const char *rpm, Header *sigp,
> > > + off_t sigStart, off_t sigTargetSize,
> > > + char *SHA1, uint8_t *MD5)
> > > +{
> > > + off_t archiveSize;
> > > + rpmRC rc = RPMRC_OK;
> > > +
> > > + 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 sigh */
> > > + rc = rpmGenerateSignature(SHA1, MD5, sigTargetSize, archiveSize,
> > fd);
> > > + if (rc != RPMRC_OK) {
> > > + rpmlog(RPMLOG_ERR, _("generateSignature 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;
> > > + }
> > > +
> > > + headerFree(*sigp);
> > > + rc = rpmReadSignature(fd, sigp, RPMSIGTYPE_HEADERSIG, NULL);
> > > + if (rc != RPMRC_OK) {
> > > + rpmlog(RPMLOG_ERR, _("rpmReadSignature failed\n"));
> > > + goto exit;
> > > + }
> > > +
> > > +exit:
> > > + return rc;
> > > +}
> > > +
> > > +static rpmRC includeFileSignatures(FD_t fd, const char *rpm,
> > > + Header *sigp, Header *hdrp,
> > > + off_t sigStart, off_t headerStart)
> > > +{
> > > + FD_t ofd = NULL;
> > > + char *trpm = NULL;
> > > + const char *key;
> > > + char *SHA1 = NULL;
> > > + uint8_t *MD5 = NULL;
> > > + unsigned char buf[32*BUFSIZ];
> > > + size_t sha1len;
> > > + size_t md5len;
> > > + off_t sigTargetSize;
> > > + rpmRC rc = RPMRC_OK;
> > > + struct rpmtd_s osigtd;
> > > + char *o_sha1 = NULL;
> > > + uint8_t o_md5[16];
> > > +
> > > +#ifndef WITH_IMAEVM
> > > + rpmlog(RPMLOG_ERR, _("missing libimaevm\n"));
> > > + return RPMRC_FAIL;
> > > +#endif
> > > + unloadImmutableRegion(hdrp, RPMTAG_HEADERIMMUTABLE);
> > > +
> > > + key = rpmExpand("%{?_file_signing_key}", NULL);
> > > +
> > > + rc = rpmSignFiles(*hdrp, key);
> > > + if (rc != RPMRC_OK) {
> > > + 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;
> > > + }
> > > +
> > //Start calculate also MD5
> > fdInitDigest(fd, PGPHASHALGO_MD5, 0);
> >
> > > + /* 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);
> > > + sigTargetSize = Ftell(fd) - headerStart;
> > //This has to be moved below code for writing archive to
> > //contain size of archive too.
> >
> > > +
> > > + /* 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);
> > //Here is the end of header + archive section
> > sigTargetSize = Ftell(fd) - headerStart
> > //Also end of calculating MD5
> > fdInitDigest(fd, PGPHASHALGO_MD5, 0);
> >
> > > +
> > //Following recalculation of MD5 is not needed.
> > > + /* 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 (headerGet(*sigp, RPMSIGTAG_MD5, &osigtd, HEADERGET_DEFAULT))
> > > + memcpy(o_md5, osigtd.data, 16);
> > > +
> > > + if (headerGet(*sigp, RPMSIGTAG_SHA1, &osigtd, HEADERGET_DEFAULT))
> > > + o_sha1 = xstrdup(osigtd.data);
> > > +
> > > + if (memcmp(MD5, o_md5, md5len) == 0 && strcmp(SHA1, o_sha1) == 0)
> > > + rpmlog(RPMLOG_WARNING,
> > > + _("%s already contains identical file signatures\n"),
> > > + rpm);
> > > + else
> > > + replaceSigDigests(fd, rpm, sigp, sigStart, sigTargetSize, SHA1,
> > MD5);
> > > +
> > > +exit:
> > > +if (ofd) (void) closeFile(&ofd);
> > > + return rc;
> > > +}
> > > +
> > > /** \ingroup rpmcli
> > > * Create/modify elements in signature header.
> > > * @param rpm path to package
> > > * @param deleting adding or deleting signature?
> > > + * @param signfiles sign files if non-zero
> > > * @return 0 on success, -1 on error
> > > */
> > > -static int rpmSign(const char *rpm, int deleting)
> > > +static int rpmSign(const char *rpm, int deleting, int signfiles)
> > > {
> > > FD_t fd = NULL;
> > > FD_t ofd = NULL;
> > > @@ -555,7 +724,11 @@ static int rpmSign(const char *rpm, int deleting)
> > > goto exit;
> > > }
> > >
> > > - unloadImmutableRegion(&sigh, RPMTAG_HEADERSIGNATURES, &utd);
> > > + if (signfiles) {
> > > + includeFileSignatures(fd, rpm, &sigh, &h, sigStart, headerStart);
> > > + }
> > > +
> > > + unloadImmutableRegion(&sigh, RPMTAG_HEADERSIGNATURES);
> > > origSigSize = headerSizeof(sigh, HEADER_MAGIC_YES);
> > >
> > > if (deleting) { /* Nuke all the signature tags. */
> > > @@ -704,7 +877,7 @@ int rpmPkgSign(const char *path, const struct
> > rpmSignArgs
> > > * args)
> > > }
> > > }
> > >
> > > - rc = rpmSign(path, 0);
> > > + rc = rpmSign(path, 0, args->signfiles);
> > >
> > > if (args) {
> > > if (args->hashalgo) {
> > > @@ -720,5 +893,5 @@ int rpmPkgSign(const char *path, const struct
> > rpmSignArgs
> > > * args)
> > >
> > > int rpmPkgDelSign(const char *path)
> > > {
> > > - return rpmSign(path, 1);
> > > + return rpmSign(path, 1, 0);
> > > }
> > > --
> > > 2.4.3
> > >
> > >
> >
> > Lubos
> >
>
Lubos
More information about the Rpm-maint
mailing list