> All my concerns seem to have been addressed, thanks for your patience and all
> the work on this!
Thanks Panu!
Really appreciate your help with this!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Merged #1203 into master.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1203#event-3729073410___
Rpm-maint mailing list
@pmatilai approved this pull request.
All my concerns seem to have been addressed, thanks for your patience and all
the work on this!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Alrighty, I think we're good to go here. With 4.16 just branched off this is a
good time to bring in new stuff for a little soak time - with stuff like this
there's almost always something to tweak that only time will bring up.
--
You are receiving this because you are subscribed to this
@jessorensen commented on this pull request.
> @@ -14,6 +14,7 @@ DISTCHECK_CONFIGURE_FLAGS = \
--with-audit \
--with-selinux \
--with-imaevm \
+ --with-fsverity \
Hi Panu,
No worries, hope you had a good break! I have been swamped with another project
the last
@pmatilai commented on this pull request.
> @@ -14,6 +14,7 @@ DISTCHECK_CONFIGURE_FLAGS = \
--with-audit \
--with-selinux \
--with-imaevm \
+ --with-fsverity \
Sorry for the silence, I've been on a vacation. We need to get this past the CI
to consider merging.
> > RPM doesn't actually need the fsverity utility to be present, but it does
> > need libfsverity
>
> Yup, the library is what I meant by my comment, not the utility. Thanks for
> adding the check.
>
> I'll need to take closer look at the updated version but overall I think its
> in fair
fsverity-utils-1.1 which includes fsverity-utils-devel is now available in
rawhide and Fedora 32, so it should be possible to build this now. Let me know
if you hit any issues.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on
> > RPM doesn't actually need the fsverity utility to be present, but it does
> > need libfsverity
>
> Yup, the library is what I meant by my comment, not the utility. Thanks for
> adding the check.
>
> I'll need to take closer look at the updated version but overall I think its
> in fair
> RPM doesn't actually need the fsverity utility to be present, but it does
> need libfsverity
Yup, the library is what I meant by my comment, not the utility. Thanks for
adding the check.
I'll need to take closer look at the updated version but overall I think its in
fair shape for this
I have pushed a fix for the configure issue, and configure should fail is one
specifies --with-fsverity and it isn't available.
Apologies if I messed something up, autoconf/automake and I do not get along.
--
You are receiving this because you are subscribed to this thread.
Reply to this email
> Oh, sorry, I've forgot to update "status" here.
>
> We can't merge a patch that fails the CI tests - this fails because fsverity
> is enabled in the CI but the library doesn't exist in Fedora 32. Hardly
> surprising as the library version isn't even released upstream AFAICS. That
> can be
Oh, sorry, I've forgot to update "status" here.
We can't merge a patch that fails the CI tests - this fails because fsverity is
enabled in the CI but the library doesn't exist in Fedora 32. Hardly surprising
as the library version isn't even released upstream AFAICS. That can be worked
around
I rebased the branch to make sure it applies cleanly to master. I also added an
additional patch, introducing the --verity-algo argument to rpmsign, allowing
the user to specify the algorithm to use for the verity signatures.
Is there anything else you would like me to address at this point?
I have pushed the update - let me know if there's anything else that needs
addressing.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
> Okay, this sounds like its headed to the right direction then, I agree this
> seems like something where the kernel needs to deal with it because it's the
> only thing that can.
>
> I see block size is an argument passed to the ioctl() that enables this
> fsverity for a file, but what does
Okay, this sounds like its headed to the right direction then, I agree this
seems like something where the kernel needs to deal with it because it's the
only thing that can.
I see block size is an argument passed to the ioctl() that enables this
fsverity for a file, but what does that actually
> I have been thinking a fair bit about this and I see a couple of options:
>
> 1. We could in principle generate signatures for every supported page size.
> This would require adding more tags, ie. one for each page size.
> 2. Do not install signatures if the page size doesn't match the
> Ok, good. For now I think we need to concentrate on the fundamental problem
> of architecture dependency. While most architectures today use 4K pages,
> being common doesn't make it arch independent, and then there even are
> architectures where this is configurable (eg aarch64). A noarch
Ok, good. For now I think we need to concentrate on the fundamental problem of
architecture dependency. While most architectures today use 4K pages, being
common doesn't make it arch independent, and then there are architectures where
this is configurable (eg aarch64). A noarch package cannot
I have pushed an updated patchset into the repo. I think it addresses
everything we discussed, including getting rid of the LENGTH and BLKSZ tags,
adding the --delfilesign option to rpmsign, and switches to base64 encoding.
Let me know if you find anything else that needs addressing or if I
@jessorensen commented on this pull request.
> + rpmlog(RPMLOG_DEBUG, "fsverity not supported by file system for
> %s\n",
+ path);
+ break;
+ case EOPNOTSUPP:
+ rpmlog(RPMLOG_DEBUG, "fsverity not enabled on file system for %s\n",
+
@jessorensen commented on this pull request.
> @@ -430,6 +438,10 @@ typedef enum rpmSigTag_e {
RPMSIGTAG_SHA256 = RPMTAG_SHA256HEADER,
RPMSIGTAG_FILESIGNATURES = RPMTAG_SIG_BASE + 18,
RPMSIGTAG_FILESIGNATURELENGTH = RPMTAG_SIG_BASE + 19,
+
@pmatilai commented on this pull request.
> + rpmlog(RPMLOG_DEBUG, "fsverity not supported by file system for
> %s\n",
+ path);
+ break;
+ case EOPNOTSUPP:
+ rpmlog(RPMLOG_DEBUG, "fsverity not enabled on file system for %s\n",
+
@pmatilai commented on this pull request.
> if (deleting) { /* Nuke all the signature tags. */
deleteSigs(sigh);
+ deleteFileSigs(sigh);
Yeah, having a single option for both types of file signatures should be plenty.
--
You are receiving this because you are subscribed
@pmatilai commented on this pull request.
> +}
+
+rpmlog(RPMLOG_DEBUG, _("key: %s\n"), key);
+rpmlog(RPMLOG_DEBUG, _("cert: %s\n"), cert);
+
+compr = headerGetString(h, RPMTAG_PAYLOADCOMPRESSOR);
+rpmio_flags = rstrscat(NULL, "r.", compr ? compr : "gzip", NULL);
+
+gzdi
@pmatilai commented on this pull request.
> @@ -430,6 +438,10 @@ typedef enum rpmSigTag_e {
RPMSIGTAG_SHA256 = RPMTAG_SHA256HEADER,
RPMSIGTAG_FILESIGNATURES = RPMTAG_SIG_BASE + 18,
RPMSIGTAG_FILESIGNATURELENGTH = RPMTAG_SIG_BASE + 19,
+
@jessorensen commented on this pull request.
> @@ -3,7 +3,8 @@
include $(top_srcdir)/rpm.am
AM_CFLAGS = @RPMCFLAGS@
-AM_CPPFLAGS = -I$(top_builddir) -I$(top_srcdir) -I$(top_builddir)/include/
+AM_CPPFLAGS = -I$(top_builddir) -I$(top_srcdir) -I$(top_builddir)/include/ \
+
@jessorensen commented on this pull request.
> + rpmlog(RPMLOG_DEBUG, "fsverity not supported by file system for
> %s\n",
+ path);
+ break;
+ case EOPNOTSUPP:
+ rpmlog(RPMLOG_DEBUG, "fsverity not enabled on file system for %s\n",
+
@jessorensen commented on this pull request.
> if (deleting) { /* Nuke all the signature tags. */
deleteSigs(sigh);
+ deleteFileSigs(sigh);
> The IMA signatures originally were covered by package signature, but that
> breaks some fundamental rpm rules so it was changed in
@jessorensen commented on this pull request.
> +}
+
+rpmlog(RPMLOG_DEBUG, _("key: %s\n"), key);
+rpmlog(RPMLOG_DEBUG, _("cert: %s\n"), cert);
+
+compr = headerGetString(h, RPMTAG_PAYLOADCOMPRESSOR);
+rpmio_flags = rstrscat(NULL, "r.", compr ? compr : "gzip", NULL);
+
+
@jessorensen commented on this pull request.
> @@ -430,6 +438,10 @@ typedef enum rpmSigTag_e {
RPMSIGTAG_SHA256 = RPMTAG_SHA256HEADER,
RPMSIGTAG_FILESIGNATURES = RPMTAG_SIG_BASE + 18,
RPMSIGTAG_FILESIGNATURELENGTH = RPMTAG_SIG_BASE + 19,
+
@jessorensen commented on this pull request.
> @@ -430,6 +438,10 @@ typedef enum rpmSigTag_e {
RPMSIGTAG_SHA256 = RPMTAG_SHA256HEADER,
RPMSIGTAG_FILESIGNATURES = RPMTAG_SIG_BASE + 18,
RPMSIGTAG_FILESIGNATURELENGTH = RPMTAG_SIG_BASE + 19,
+
@pmatilai commented on this pull request.
> + rpmlog(RPMLOG_DEBUG, "fsverity not supported by file system for
> %s\n",
+ path);
+ break;
+ case EOPNOTSUPP:
+ rpmlog(RPMLOG_DEBUG, "fsverity not enabled on file system for %s\n",
+
@pmatilai commented on this pull request.
> +}
+
+static char *rpmVeritySignFile(rpmfi fi, size_t *sig_size, char *key,
+ char *keypass, char *cert, uint16_t algo,
+ uint32_t block_size)
+{
+struct libfsverity_merkle_tree_params
@pmatilai commented on this pull request.
> @@ -430,6 +438,10 @@ typedef enum rpmSigTag_e {
RPMSIGTAG_SHA256 = RPMTAG_SHA256HEADER,
RPMSIGTAG_FILESIGNATURES = RPMTAG_SIG_BASE + 18,
RPMSIGTAG_FILESIGNATURELENGTH = RPMTAG_SIG_BASE + 19,
+
@pmatilai commented on this pull request.
> @@ -430,6 +438,10 @@ typedef enum rpmSigTag_e {
RPMSIGTAG_SHA256 = RPMTAG_SHA256HEADER,
RPMSIGTAG_FILESIGNATURES = RPMTAG_SIG_BASE + 18,
RPMSIGTAG_FILESIGNATURELENGTH = RPMTAG_SIG_BASE + 19,
+
@pmatilai commented on this pull request.
> +}
+
+rpmlog(RPMLOG_DEBUG, _("key: %s\n"), key);
+rpmlog(RPMLOG_DEBUG, _("cert: %s\n"), cert);
+
+compr = headerGetString(h, RPMTAG_PAYLOADCOMPRESSOR);
+rpmio_flags = rstrscat(NULL, "r.", compr ? compr : "gzip", NULL);
+
+gzdi
@pmatilai commented on this pull request.
> if (deleting) { /* Nuke all the signature tags. */
deleteSigs(sigh);
+ deleteFileSigs(sigh);
The IMA signatures originally were covered by package signature, but that
breaks some fundamental rpm rules so it was changed in a
@pmatilai commented on this pull request.
> @@ -3,7 +3,8 @@
include $(top_srcdir)/rpm.am
AM_CFLAGS = @RPMCFLAGS@
-AM_CPPFLAGS = -I$(top_builddir) -I$(top_srcdir) -I$(top_builddir)/include/
+AM_CPPFLAGS = -I$(top_builddir) -I$(top_srcdir) -I$(top_builddir)/include/ \
+ -I$(includedir)
@pmatilai commented on this pull request.
> @@ -494,6 +505,36 @@ static rpmRC includeFileSignatures(Header *sigp, Header
> *hdrp)
#endif
}
+static rpmRC includeVeritySignatures(FD_t fd, Header *sigp, Header *hdrp)
+{
+#ifdef WITH_FSVERITY
+rpmRC rc;
+char *key =
@jessorensen commented on this pull request.
> +digest_hex = pgpHexStr(digest->digest, digest->digest_size);
+rpmlog(RPMLOG_DEBUG, _("file(size %li): %s: digest(%i): %s, idx %i\n"),
+ file_size, rpmfiFN(fi), digest->digest_size, digest_hex,
+ rpmfiFX(fi));
+
+
@jessorensen commented on this pull request.
> @@ -166,8 +184,9 @@ int main(int argc, char *argv[])
argerror(_("no arguments given"));
}
-#ifdef WITH_IMAEVM
-if (fileSigningKey && !(sargs.signflags & RPMSIGN_FLAG_IMA)) {
+#if defined(WITH_IMAEVM) || defined(WITH_FSVERITY)
+
@jessorensen commented on this pull request.
> @@ -494,6 +505,36 @@ static rpmRC includeFileSignatures(Header *sigp, Header
> *hdrp)
#endif
}
+static rpmRC includeVeritySignatures(FD_t fd, Header *sigp, Header *hdrp)
+{
+#ifdef WITH_FSVERITY
+rpmRC rc;
+char *key =
@jessorensen commented on this pull request.
> @@ -3,7 +3,8 @@
include $(top_srcdir)/rpm.am
AM_CFLAGS = @RPMCFLAGS@
-AM_CPPFLAGS = -I$(top_builddir) -I$(top_srcdir) -I$(top_builddir)/include/
+AM_CPPFLAGS = -I$(top_builddir) -I$(top_srcdir) -I$(top_builddir)/include/ \
+
@jessorensen commented on this pull request.
> + rpmlog(RPMLOG_DEBUG, "fsverity not supported by file system for
> %s\n",
+ path);
+ break;
+ case EOPNOTSUPP:
+ rpmlog(RPMLOG_DEBUG, "fsverity not enabled on file system for %s\n",
+
@jessorensen commented on this pull request.
> +}
+
+static char *rpmVeritySignFile(rpmfi fi, size_t *sig_size, char *key,
+ char *keypass, char *cert, uint16_t algo,
+ uint32_t block_size)
+{
+struct libfsverity_merkle_tree_params
@jessorensen commented on this pull request.
> + * Copyright (C) 2020 Facebook
+ *
+ * Author: Jes Sorensen
+ */
+
+#include "system.h"
+
+#include /* RPMSIGTAG & related */
+#include /* rpmlog */
+#include
+#include /* rpmDigestLength */
@jessorensen commented on this pull request.
> if (deleting) { /* Nuke all the signature tags. */
deleteSigs(sigh);
+ deleteFileSigs(sigh);
>From my understanding, the package signature covers the file signatures, so we
>cannot remove them without invalidating the package
@jessorensen commented on this pull request.
> @@ -71,6 +71,18 @@ void headerMergeLegacySigs(Header h, Header sigh)
case RPMSIGTAG_FILESIGNATURELENGTH:
td.tag = RPMTAG_FILESIGNATURELENGTH;
break;
+ case RPMSIGTAG_VERITYSIGNATURES:
+ td.tag =
@jessorensen commented on this pull request.
> @@ -396,6 +397,16 @@ static void deleteSigs(Header sigh)
headerDel(sigh, RPMSIGTAG_PGP5);
}
+static void deleteFileSigs(Header sigh)
+{
+headerDel(sigh, RPMSIGTAG_FILESIGNATURELENGTH);
+headerDel(sigh, RPMSIGTAG_FILESIGNATURES);
+
@jessorensen commented on this pull request.
> @@ -116,8 +116,12 @@ struct rpmfiles_s {
int digestalgo;/*!< File digest algorithm */
int signaturelength; /*!< File signature length */
+int veritysiglength; /*!< Verity signature length */
+uint16_t
@jessorensen commented on this pull request.
> +}
+
+rpmlog(RPMLOG_DEBUG, _("key: %s\n"), key);
+rpmlog(RPMLOG_DEBUG, _("cert: %s\n"), cert);
+
+compr = headerGetString(h, RPMTAG_PAYLOADCOMPRESSOR);
+rpmio_flags = rstrscat(NULL, "r.", compr ? compr : "gzip", NULL);
+
+
@jessorensen commented on this pull request.
> @@ -430,6 +438,10 @@ typedef enum rpmSigTag_e {
RPMSIGTAG_SHA256 = RPMTAG_SHA256HEADER,
RPMSIGTAG_FILESIGNATURES = RPMTAG_SIG_BASE + 18,
RPMSIGTAG_FILESIGNATURELENGTH = RPMTAG_SIG_BASE + 19,
+
@pmatilai requested changes on this pull request.
Various things to address, to a large part due to unfortunate use of file
signing as the example, and hopefully significant simplification is possible,
but overall I think we're on the manageable side.
--
You are receiving this because you
@pmatilai commented on this pull request.
> @@ -494,6 +505,36 @@ static rpmRC includeFileSignatures(Header *sigp, Header
> *hdrp)
#endif
}
+static rpmRC includeVeritySignatures(FD_t fd, Header *sigp, Header *hdrp)
+{
+#ifdef WITH_FSVERITY
+rpmRC rc;
+char *key =
@pmatilai commented on this pull request.
> @@ -166,8 +184,9 @@ int main(int argc, char *argv[])
argerror(_("no arguments given"));
}
-#ifdef WITH_IMAEVM
-if (fileSigningKey && !(sargs.signflags & RPMSIGN_FLAG_IMA)) {
+#if defined(WITH_IMAEVM) || defined(WITH_FSVERITY)
+
@pmatilai commented on this pull request.
> @@ -3,7 +3,8 @@
include $(top_srcdir)/rpm.am
AM_CFLAGS = @RPMCFLAGS@
-AM_CPPFLAGS = -I$(top_builddir) -I$(top_srcdir) -I$(top_builddir)/include/
+AM_CPPFLAGS = -I$(top_builddir) -I$(top_srcdir) -I$(top_builddir)/include/ \
+ -I$(includedir)
@pmatilai commented on this pull request.
> + rpmlog(RPMLOG_DEBUG, "fsverity not supported by file system for
> %s\n",
+ path);
+ break;
+ case EOPNOTSUPP:
+ rpmlog(RPMLOG_DEBUG, "fsverity not enabled on file system for %s\n",
+
@pmatilai commented on this pull request.
> +}
+
+static char *rpmVeritySignFile(rpmfi fi, size_t *sig_size, char *key,
+ char *keypass, char *cert, uint16_t algo,
+ uint32_t block_size)
+{
+struct libfsverity_merkle_tree_params
@pmatilai commented on this pull request.
> + * Copyright (C) 2020 Facebook
+ *
+ * Author: Jes Sorensen
+ */
+
+#include "system.h"
+
+#include /* RPMSIGTAG & related */
+#include /* rpmlog */
+#include
+#include /* rpmDigestLength */
+#include
@pmatilai commented on this pull request.
> if (deleting) { /* Nuke all the signature tags. */
deleteSigs(sigh);
+ deleteFileSigs(sigh);
I think deleting file signatures needs to be a separate thing from the main
package signatures, you might want to delete one but not the
@pmatilai commented on this pull request.
> @@ -396,6 +397,16 @@ static void deleteSigs(Header sigh)
headerDel(sigh, RPMSIGTAG_PGP5);
}
+static void deleteFileSigs(Header sigh)
+{
+headerDel(sigh, RPMSIGTAG_FILESIGNATURELENGTH);
+headerDel(sigh, RPMSIGTAG_FILESIGNATURES);
+
@pmatilai commented on this pull request.
> @@ -71,6 +71,18 @@ void headerMergeLegacySigs(Header h, Header sigh)
case RPMSIGTAG_FILESIGNATURELENGTH:
td.tag = RPMTAG_FILESIGNATURELENGTH;
break;
+ case RPMSIGTAG_VERITYSIGNATURES:
+ td.tag =
@pmatilai commented on this pull request.
> +digest_hex = pgpHexStr(digest->digest, digest->digest_size);
+rpmlog(RPMLOG_DEBUG, _("file(size %li): %s: digest(%i): %s, idx %i\n"),
+ file_size, rpmfiFN(fi), digest->digest_size, digest_hex,
+ rpmfiFX(fi));
+
+
@pmatilai commented on this pull request.
> @@ -116,8 +116,12 @@ struct rpmfiles_s {
int digestalgo;/*!< File digest algorithm */
int signaturelength; /*!< File signature length */
+int veritysiglength; /*!< Verity signature length */
+uint16_t
@pmatilai commented on this pull request.
> +}
+
+rpmlog(RPMLOG_DEBUG, _("key: %s\n"), key);
+rpmlog(RPMLOG_DEBUG, _("cert: %s\n"), cert);
+
+compr = headerGetString(h, RPMTAG_PAYLOADCOMPRESSOR);
+rpmio_flags = rstrscat(NULL, "r.", compr ? compr : "gzip", NULL);
+
+gzdi
@pmatilai commented on this pull request.
> @@ -430,6 +438,10 @@ typedef enum rpmSigTag_e {
RPMSIGTAG_SHA256 = RPMTAG_SHA256HEADER,
RPMSIGTAG_FILESIGNATURES = RPMTAG_SIG_BASE + 18,
RPMSIGTAG_FILESIGNATURELENGTH = RPMTAG_SIG_BASE + 19,
+
@pmatilai commented on this pull request.
> @@ -430,6 +438,10 @@ typedef enum rpmSigTag_e {
RPMSIGTAG_SHA256 = RPMTAG_SHA256HEADER,
RPMSIGTAG_FILESIGNATURES = RPMTAG_SIG_BASE + 18,
RPMSIGTAG_FILESIGNATURELENGTH = RPMTAG_SIG_BASE + 19,
+
69 matches
Mail list logo
