from:"Stefan Berger"

Re: [Rpm-maint] [rpm-software-management/rpm] Also apply signatures to config files (#374)

2018-02-11 Thread Stefan Berger

The --replacefiles option seems to work on an equivalent of a regex matching 
all files (`.*`). You are saying 'What rpm lacks is an ability to apply 
--replacefiles to only some of the %config files in the packages being 
installed in a single transaction'. What other choice do we have then than to 
give the user control over picking the files either by exact match or, to be 
more flexible, regular expressions. Regexs would let him choose 'some of the 
%config files' as you say and not necessarily all of them.

I don't understand your last paragraph: 'disable %config (and %ghost) in 
packaging and metadata' -- do you mean to patch the signature creation side in 
rpmsign ?



-- 
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/374#issuecomment-364798174___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Also apply signatures to config files (#374)

2018-02-10 Thread Stefan Berger

@n3npq: I am not sure how your suggestion of 'a per-file override of an 
inherited per-transaction AND mask would provide the ability to disable 
RPMFILE_CONFIG on a per-file basis' would translate into an implementation. 
Would we want this to be IMA specific? Maybe a list of regular expressions for 
%config file patterns that mask out the RPMFILE_CONFIG file bit?

-- 
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/374#issuecomment-364664890___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Also apply signatures to config files (#374)

2018-02-10 Thread Stefan Berger

@n3npq: Re 'Adding the ability to change the ima signature in the xattr after 
installation, so that the modified, not the original %config template, would 
(at least) change my opinion, similarly for %ghost. But that isn't what is 
being proposed.': How would that work without including the private key in the 
RPM file? I would say any post-installation fixes to these %config files need 
to be done locally either through signing the file with a local key once it is 
deemed immutable after all editing is done, or adjust the IMA policy in such a 
way that this file will not be appraised.

-- 
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/374#issuecomment-364664430___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Also apply signatures to config files (#374)

2018-02-10 Thread Stefan Berger

Please see https://github.com/rpm-software-management/rpm/issues/364 for the 
request.

-- 
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/374#issuecomment-364657716___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Also apply signatures to config files (#374)

2018-02-10 Thread Stefan Berger

@n3npq All I can say that I have a user who wants to have signatures written on 
%config files. This is what is driving this patch.

-- 
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/374#issuecomment-364657612___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Also apply signatures to config files (#374)

2018-02-09 Thread Stefan Berger

So from the documentation at 
http://ftp.rpm.org/max-rpm/s1-rpm-inside-files-list-directives.html I take that 
the file is neither packaged nor installed. Since it's not packaged, the RPM 
also doesn't carry a signature and we cannot write a signature out. If someone 
wants to write signatures out for %config files by activating the macro, he 
needs to be aware that these are mutable files and deal with the fact that 
their signature may get lost or end up being wrong. a %ghost file is then more 
or less a case of a %config file that will never have a signature written out.

-- 
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/374#issuecomment-364622965___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Also apply signatures to config files (#374)

2018-02-09 Thread Stefan Berger

@n3npq  With this patch we would basically allow everything to be signed for 
which we have signatures since we previously only filtered out %config files 
that were not executable. If a %ghost file has a signature stored in the rpm, 
it would at least now have it written out as well. If %ghost files are not even 
written out (per docu: 'There are times when a file should be owned by the 
package but not installed.') then ... why would we need to sign them?

-- 
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/374#issuecomment-364573457___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Also apply signatures to config files (#374)

2018-02-09 Thread Stefan Berger

Good point. Using .init now. :-)

-- 
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/374#issuecomment-364449325___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Also apply signatures to config files (#374)

2018-02-09 Thread Stefan Berger

@pmatilai I updated the patch to use `%_ima_sign_config_files`.

-- 
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/374#issuecomment-364436703___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Also apply signatures to config files (#374)

2018-02-08 Thread Stefan Berger

I am using the variable `_write_signatures_on_config_files`. Maybe it should be 
`_write_ima_signatures_on_config_files`?

-- 
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/374#issuecomment-364244313___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Also apply signatures to config files (#374)

2018-02-05 Thread Stefan Berger

@pmatilai  So, tested it now. It works for me. 

-- 
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/374#issuecomment-363114593___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Also apply signatures to config files (#374)

2018-02-05 Thread Stefan Berger

I just pushed an update but I haven't tested it, yet. Any comments on it?

-- 
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/374#issuecomment-363098710___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Also apply signatures to config files (#374)

2018-02-05 Thread Stefan Berger

I'll try to look at it this week. I suppose we can introduce a new command line 
option and an option for the macros file? Any suggestion? Is there another 
option that already works like this with a command line option and an option in 
the macros file ?

-- 
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/374#issuecomment-363060254___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Request to apply IMA signatures to files even if deemed a configuration file (#364)

2017-12-27 Thread Stefan Berger

This PR https://github.com/rpm-software-management/rpm/pull/374 now addresses 
the issue.

-- 
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/issues/364#issuecomment-354154257___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Request to apply IMA signatures to files even if deemed a configuration file (#364)

2017-12-06 Thread Stefan Berger

A potential side-effect of having signatures applied to configuration files is 
that the configuration files may be modified by the user or programs / 
post-installation scripts and the signature on these files may become invalid 
or be removed as part of the modification of the configuration file. That 'a 
file installed by an RPM will have the correct signature' will therefore not be 
true for those configuration files.

-- 
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/issues/364#issuecomment-349682417___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Problem with --signfiles for files that are hardlinked together (#333)

2017-11-02 Thread Stefan Berger

@pmatilai Would it be possible to have these 4 patches applied to the latest 
rpm built for Fedora 26 and later?

-- 
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/issues/333#issuecomment-341506488___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Write the content of a file with hard links into the first file (#342)

2017-10-26 Thread Stefan Berger

@stefanberger pushed 3 commits.

01a97c6  Create first hard link file and keep open, write it at end
f05ea9c  remove redundant 'nocontent' parameter from expandRegular
c07b93d  Remove redundant 'exclusive' parameter from expandRegular


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/342/files/b2fa119ea239de89eca422b089cef5b0b9bfde05..c07b93daa1fff28591fb493ccb9037a46d80a167
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Write the content of a file with hard links into the first file (#342)

2017-10-26 Thread Stefan Berger

@stefanberger pushed 1 commit.

b2fa119  split off function wfd_open() to open a file


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/342/files/74ea0379c03fd3acd9fc03cf19e695526fab5b27..b2fa119ea239de89eca422b089cef5b0b9bfde05
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Write the content of a file with hard links into the first file (#342)

2017-10-26 Thread Stefan Berger

Reopened #342.

-- 
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/342#event-1313092365___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Write the content of a file with hard links into the first file (#342)

2017-10-26 Thread Stefan Berger

Closed #342.

-- 
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/342#event-1313091691___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Write the content of a file with hard links into the first file (#342)

2017-10-26 Thread Stefan Berger

Odd, the symlink test case works on my system...

-- 
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/342#issuecomment-339842010___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Problem with --signfiles for files that are hardlinked together (#333)

2017-10-26 Thread Stefan Berger

@patrickc25000 I will have to patch the rpm package you are using with these 
patches and have you test it...

-- 
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/issues/333#issuecomment-339839375___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [rpm-software-management/rpm] Write the content of a file with hard links into the first file (#342)

2017-10-26 Thread Stefan Berger

This series of patches attempts to address the errors we are seeing when 
installing RPMs that contain hard links and an IMA policy that measures on 
reading and writing of files. The problem has been explained in issue #333.

The solution is to open the first file that is created empty but now keep the 
file descriptor to that file open and complete the writing of the file at the 
last file (hard link) that actually has the file content.
You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/342

-- Commit Summary --

  * Split off function wfd_close() to close a file
  * split off function wfd_open() to open a file
  * Create first hard link file and keep open, write it at end
  * remove redundant 'nocontent' parameter from expandRegular
  * Remove redundant 'exclusive' parameter from expandRegular

-- File Changes --

M lib/fsm.c (97)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/342.patch
https://github.com/rpm-software-management/rpm/pull/342.diff

-- 
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/342
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Problem with --signfiles for files that are hardlinked together (#333)

2017-10-26 Thread Stefan Berger

The problem seems to be that only the last entry has the file content... so the 
empty file that's created first cannot be written since the RPM entry currently 
being processed doesn't have the data. Then a couple of hard links may get 
created and only the last entry (hardlink) found there has the content that can 
then be written out.

-- 
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/issues/333#issuecomment-339712089___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Problem with --signfiles for files that are hardlinked together (#333)

2017-10-26 Thread Stefan Berger

@pmatilai Is there a  reason that in case of hard links the file gets written 
only after all the hard links have been created? It looks a bit complicated ...

-- 
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/issues/333#issuecomment-339674525___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Problem with --signfiles for files that are hardlinked together (#333)

2017-10-25 Thread Stefan Berger

@pmatilai Is the problem limited to the fsmMkfile() function? I suppose this is 
where the hard links are created. Would the solution be to do things in a 
different order in that function ?

-- 
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/issues/333#issuecomment-339472149___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH v3 2/2] ima-plugin: Move the IMA plugin to the fsm_file_prepare hook

2016-09-23 Thread Stefan Berger

Since newly installed files may be invoked by post install scriptlets,
we need to have them signed before the scriptlets are executed.
Therefore, we now move the IMA plugin to the fsm_file_prepare hook.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 plugins/ima.c | 43 +++
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/plugins/ima.c b/plugins/ima.c
index 76c7d3d..9264708 100644
--- a/plugins/ima.c
+++ b/plugins/ima.c
@@ -1,9 +1,11 @@
+#include 
 #include 
 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "lib/rpmfs.h"
@@ -35,38 +37,39 @@ static int check_zero_hdr(const unsigned char *fsig, size_t 
siglen)
return (memcmp(fsig, _hdr, sizeof(zero_hdr)) == 0);
 }
 
-static rpmRC ima_psm_post(rpmPlugin plugin, rpmte te, int res)
+static rpmRC ima_fsm_file_prepare(rpmPlugin plugin, rpmfi fi,
+  const char *path,
+  const char *dest,
+  mode_t file_mode, rpmFsmOp op)
 {
-   rpmfi fi = rpmteFI(te);
-   const char *fpath;
const unsigned char * fsig = NULL;
size_t len;
-   int rc = 0;
+   int rc = RPMRC_OK;
+   rpmFileAction action = XFO_ACTION(op);
 
-   if (fi == NULL) {
-   rc = RPMERR_BAD_MAGIC;
+   if (!fi || !path || XFA_SKIPPING(action))
goto exit;
-   }
 
-   while (rpmfiNext(fi) >= 0) {
-   /* Don't install signatures for (mutable) files marked
-* as config files unless they are also executable.
-*/
-   if (rpmfiFFlags(fi) & RPMFILE_CONFIG) {
-   if (!(rpmfiFMode(fi) & (S_IXUSR|S_IXGRP|S_IXOTH)))
-   continue;
-   }
+   /* Don't install signatures for (mutable) files marked
+* as config files unless they are also executable.
+*/
+   if (rpmfiFFlags(fi) & RPMFILE_CONFIG) {
+   if (!(rpmfiFMode(fi) & (S_IXUSR|S_IXGRP|S_IXOTH)))
+   goto exit;
+   }
 
-   fsig = rpmfiFSignature(fi, );
-   if (fsig && (check_zero_hdr(fsig, len) == 0)) {
-   fpath = rpmfiFN(fi);
-   lsetxattr(fpath, XATTR_NAME_IMA, fsig, len, 0);
+   fsig = rpmfiFSignature(fi, );
+   if (fsig && (check_zero_hdr(fsig, len) == 0)) {
+   if (lsetxattr(path, XATTR_NAME_IMA, fsig, len, 0) < 0) {
+   rpmlog(RPMLOG_ERR, "ima: could not apply signature on '%s': 
%s\n", path, strerror(errno));
+   rc = RPMRC_FAIL;
}
}
+
 exit:
return rc;
 }
 
 struct rpmPluginHooks_s ima_hooks = {
-   .psm_post = ima_psm_post,
+   .fsm_file_prepare = ima_fsm_file_prepare,
 };
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH v3 0/2] Fixes for file signatures

2016-09-23 Thread Stefan Berger

The following series of patches addresses some issues with signatures on
files. In particular:

- some files marked as config files are also executables and therefore
  need to have the signature applied

- some RPM packages require that the files be signed when the post
  install scriptlets are run since they may invoke executables that
  were just installed; so we move the IMA plugin from the psm_post hook
  to the fsm_file_prepare hook.

   Regards,
  Stefan

Stefan Berger (2):
  ima-plugin: Have executable configuration files signed
  ima-plugin: Move the IMA plugin to the fsm_file_prepare hook

 plugins/ima.c | 38 +++---
 1 file changed, 23 insertions(+), 15 deletions(-)

-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH v2 0/4] Fixes for file signatures

2016-09-23 Thread Stefan Berger

Panu Matilainen <pmati...@laiskiainen.org> wrote on 09/23/2016 03:30:54 
PM:

> From: Panu Matilainen <pmati...@laiskiainen.org>
> To: Stefan Berger/Watson/IBM@IBMUS
> Cc: fionnuala.gun...@gmail.com, rpm-maint@lists.rpm.org, Stefan 
> Berger <stef...@linux.vnet.ibm.com>
> Date: 09/23/2016 03:31 PM
> Subject: Re: [Rpm-maint] [PATCH v2 0/4] Fixes for file signatures
> 
> On 09/23/2016 07:43 PM, Stefan Berger wrote:
> > Panu Matilainen <pmati...@laiskiainen.org> wrote on 09/23/2016 
07:50:15
> > AM:
> >
> >
> >>>>
> >>>> So... to achieve all this and actually behave correct in the face 
of
> >>>> skipped files  - whether due to color, netshared path or other file
> >>>> policies - the IMA plugin should really just do what the selinux
> > plugin
> >>>> does and use fsm_file_prepare hook for its task, which after all is
> >>>> highly similar anyway.
> >>>
> >>> Has the file been written when fsm_file_prepare is called? Otherwise
> > it
> >>> seems better to do it in fsm_file_post.
> >>
> >> Yes, the entire file has been created but not yet moved to its final
> >> destination. That's why it gets two path parameters: "path" for the
> >> actual current filename which has a temporary suffix, and "dest" 
which
> >> is the actual destination filename. So this is really the best place 
to
> >> do any metadata work because then the file actually ready when it 
gets
> >> renamed to its final distination (ie without the suffix).
> >
> > For some mysterious reason dnf now exists in an update when I run in 
the
> > fsm_file_prepare hook. After that, when telling dnf to install a 
package,
> > it enumerates all kinds of locks that it unlocks. Do you know what may 
be
> > the cause for this ?
> 
> A bug in the code, causing a crash? Like I said, what I posted is 
> entirely untested, it was just to point you in the general direction.
> 
> My first guess would be NULL fi tripping up one of the rpmfiFoo() calls, 

> reading through http://rpm.org/wiki/DevelDocs/Plugins reminded me that 
> fi can be NULL (on unowned directories).
> 
> So change the start to eg:
> 
>  /* Ignore skipped files and unowned directories */
>  if (XFA_SKIPPING(action) || fi == NULL)
>  goto exit;
> 

So it must have been the fi being NULL. For some reason though it wasn't a 
pointer...

   Stefan

> >
> > Following these issues, I would like to try to meve it to the
> > fsm_file_post hook.
> 
> I fail to see how that would accomplish anything at all.
> 
>- Panu -
> 
> 


___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH v2 0/4] Fixes for file signatures

2016-09-23 Thread Stefan Berger

Panu Matilainen  wrote on 09/23/2016 07:50:15 
AM:


> >>
> >> So... to achieve all this and actually behave correct in the face of
> >> skipped files  - whether due to color, netshared path or other file
> >> policies - the IMA plugin should really just do what the selinux 
plugin
> >> does and use fsm_file_prepare hook for its task, which after all is
> >> highly similar anyway.
> >
> > Has the file been written when fsm_file_prepare is called? Otherwise 
it
> > seems better to do it in fsm_file_post.
> 
> Yes, the entire file has been created but not yet moved to its final 
> destination. That's why it gets two path parameters: "path" for the 
> actual current filename which has a temporary suffix, and "dest" which 
> is the actual destination filename. So this is really the best place to 
> do any metadata work because then the file actually ready when it gets 
> renamed to its final distination (ie without the suffix).

For some mysterious reason dnf now exists in an update when I run in the 
fsm_file_prepare hook. After that, when telling dnf to install a package, 
it enumerates all kinds of locks that it unlocks. Do you know what may be 
the cause for this ?

Following these issues, I would like to try to meve it to the 
fsm_file_post hook.

   Stefan


___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH v2 0/4] Fixes for file signatures

2016-09-23 Thread Stefan Berger

Panu Matilainen <pmati...@laiskiainen.org> wrote on 09/23/2016 04:15:22 
AM:

> From: Panu Matilainen <pmati...@laiskiainen.org>
> To: Stefan Berger <stef...@linux.vnet.ibm.com>, rpm-maint@lists.rpm.org
> Cc: Stefan Berger/Watson/IBM@IBMUS, fionnuala.gun...@gmail.com
> Date: 09/23/2016 04:15 AM
> Subject: Re: [Rpm-maint] [PATCH v2 0/4] Fixes for file signatures
> 
> On 09/22/2016 08:30 PM, Stefan Berger wrote:
> > The following series of patches addresses some issues with signatures 
on
> > files. In particular:
> >
> > - some files marked as config files are also executables and therefore
> >   need to have a signature applied
> > - the IMA plugin may only run on package install cycle rather than the
> >   remove cycle, which would apply the previous versions' signatures on
> >   the files
> > - some RPM packages require that the files be signed when the post
> >   install scriptlets are run since they may invoke executables that
> >   were just installed; so we introduce two new hooks, fsm_pre and
> >   fsm_post. We move the IMA plugin from the psm_post hook to the
> >   fsm_post hook.
> >
> >Regards,
> >   Stefan
> >
> > Stefan Berger (4):
> >   ima-plugin: Have executable configuration files signed
> >   ima-plugin: Only run the IMA plugin on package installation
> >   rpmplugins: Introduce new fsm_pre and fsm_post hooks
> >   IMA: Move the IMA plugin to the fsm_post hook
> >
> >  lib/psm.c|  6 +-
> >  lib/rpmplugin.h  |  6 ++
> >  lib/rpmplugins.c | 35 +++
> >  lib/rpmplugins.h | 19 +++
> >  plugins/ima.c| 32 ++--
> >  5 files changed, 87 insertions(+), 11 deletions(-)
> >
> 
> So... to achieve all this and actually behave correct in the face of 
> skipped files  - whether due to color, netshared path or other file 
> policies - the IMA plugin should really just do what the selinux plugin 
> does and use fsm_file_prepare hook for its task, which after all is 
> highly similar anyway.

Has the file been written when fsm_file_prepare is called? Otherwise it 
seems better to do it in fsm_file_post.

Btw, what do fsm, tsm, and psm stand for ?


> 
> Something like this (mind you, non-compiled, never mind tested code 
ahead):
> 
> static rpmRC ima_file_prepare(rpmPlugin plugin, rpmfi fi,
>const char *path, const char *dest,
>mode_t file_mode, rpmFsmOp op)
> 
> {
>  const unsigned char * fsig = NULL;
>  size_t len;
>  rpmRC rc = RPMRC_OK;
> 
>  /* Ignore skipped files */
>  if (XFA_SKIPPING(action))
>  goto exit;

Good to know

> 
>  /* Don't install signatures for (mutable) config files */
>  if (rpmfiFFlags(fi) & RPMFILE_CONFIG) {
>  if (!rpmfiFMode(fi) & (S_IXUSR|S_IXGRP|S_IXOTH))
>  goto exit;
>  }
> 
>  fsig = rpmfiFSignature(fi, );
>  if (fsig && (check_zero_hdr(fsig, len) == 0)) {
>  if (lsetxattr(fpath, XATTR_NAME_IMA, fsig, len, 0) < 0)
>  rc = RPMRC_FAIL;
>  }

Thanks.

Stefan

> 
> exit:
>  return rc;
> }
> 
> struct rpmPluginHooks_s ima_hooks = {
>  .fsm_file_prepare = ima_file_prepare,
> };
> 
> 
> 
> That hook will only get called on file creation so you dont need to 
> separately weed out erasures, skipped files are ignored as they should 
> be and all the signatures will be in place by the time %post runs.
> 
> As a diff, that's nothing more than:
>   plugins/ima.c | 34 ++
>   1 file changed, 18 insertions(+), 16 deletions(-)
> 
>- Panu -
> 


___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH v2 3/4] rpmplugins: Introduce new fsm_pre and fsm_post hooks

2016-09-23 Thread Stefan Berger

Panu Matilainen <pmati...@laiskiainen.org> wrote on 09/23/2016 03:03:48 
AM:

> From: Panu Matilainen <pmati...@laiskiainen.org>
> To: Stefan Berger <stef...@linux.vnet.ibm.com>, rpm-maint@lists.rpm.org
> Cc: Stefan Berger/Watson/IBM@IBMUS, fionnuala.gun...@gmail.com
> Date: 09/23/2016 03:03 AM
> Subject: Re: [Rpm-maint] [PATCH v2 3/4] rpmplugins: Introduce new 
> fsm_pre and fsm_post hooks
> 
> On 09/22/2016 08:30 PM, Stefan Berger wrote:
> > Introduce fsm_pre and fsm_post hooks, which are invoked
> > before and after the package files are installed.
> >
> > Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
> [...]
> > diff --git a/lib/rpmplugins.h b/lib/rpmplugins.h
> > index 39762c3..3702526 100644
> > --- a/lib/rpmplugins.h
> > +++ b/lib/rpmplugins.h
> > @@ -167,6 +167,25 @@ rpmRC rpmpluginsCallFsmFilePrepare(rpmPlugins
> plugins, rpmfi fi,
> > const char *path, const char 
*dest,
> > mode_t mode, rpmFsmOp op);
> >
> > +/** \ingroup rpmplugins
> > + * Call the fsm pre plugin hook.
> > + * @param plugins   plugins structure
> > + * @param te  processed transaction element
> > + * @return  RPMRC_OK on success, RPMRC_FAIL otherwise
> > + */
> > +RPM_GNUC_INTERNAL
> > +rpmRC rpmpluginsCallFsmPre(rpmPlugins plugins, rpmte te);
> > +
> > +/** \ingroup rpmplugins
> > + * Call the fsm post plugin hook.
> > + * @param plugins   plugins structure
> > + * @param te  processed transaction element
> > + * @param res  fsm result code
> > + * @return  RPMRC_OK on success, RPMRC_FAIL otherwise
> > + */
> > +RPM_GNUC_INTERNAL
> > +rpmRC rpmpluginsCallFsmPost(rpmPlugins plugins, rpmte te, int rc);
> > +
> 
> Any particular reason not to pass the associated rpmfiles to these hooks 

> too? That would be the main object of these hooks, regardless of what 
> your particular needs in this case are.

I only passed the parameters that I actually need. I can add the files to 
that as well, if other plugins may require them.

> 
> Or actually to really follow the generic style of the plugin interface, 
> you'd *only* pass the files "object" and not the transaction element, 
> you can get and store that from PsmPre already.

PsmPre also passes the rpmte. PsmPre run after the post install scriptlet 
runs, so therefore we cannot use it.

> 
> ...or alternatively you could install the signatures from one of the 
> FsmFile hooks where the file-level action is available, then you dont 
> need access to the transaction element. And actually it'd probably be 
> more correct too because files can get skipped for various reasons, in 
> which case you should not add the signature either.

Good, I'll try to move it there...

   Stefan

> 
> So technically none of this is actually needed. Unless I'm missing 
> something, I'm not at all familiar with IMA really.
> 
>- Panu -
> 
> 
> 
>- Panu -
> 
> >  #ifdef __cplusplus
> >  }
> >  #endif
> >
> 


___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH v2 1/4] ima-plugin: Have executable configuration files signed

2016-09-23 Thread Stefan Berger

Panu Matilainen <pmati...@laiskiainen.org> wrote on 09/23/2016 02:44:48 
AM:

> From: Panu Matilainen <pmati...@laiskiainen.org>
> To: Stefan Berger <stef...@linux.vnet.ibm.com>, rpm-maint@lists.rpm.org
> Cc: Stefan Berger/Watson/IBM@IBMUS, fionnuala.gun...@gmail.com
> Date: 09/23/2016 02:45 AM
> Subject: Re: [Rpm-maint] [PATCH v2 1/4] ima-plugin: Have executable 
> configuration files signed
> 
> On 09/22/2016 08:30 PM, Stefan Berger wrote:
> > Some configuration files are executables and so they require the
> > signature in the extended attribute. If they are not executable,
> > they can be skipped.
> >
> > Examples for configuration files that are also executables are
> > the grub files in /etc/grub.d.
> >
> > Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
> > ---
> >  plugins/ima.c | 25 +
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/plugins/ima.c b/plugins/ima.c
> > index be15ecf..81ed194 100644
> > --- a/plugins/ima.c
> > +++ b/plugins/ima.c
> > @@ -41,7 +41,8 @@ static rpmRC ima_psm_post(rpmPlugin plugin, 
> rpmte te, int res)
> > const char *fpath;
> > const unsigned char * fsig = NULL;
> > size_t len;
> > -   int rc = 0;
> > +   int rc = 0, n;
> > +   struct stat statbuf;
> >
> > if (fi == NULL) {
> > rc = RPMERR_BAD_MAGIC;
> > @@ -49,13 +50,21 @@ static rpmRC ima_psm_post(rpmPlugin plugin, 
> rpmte te, int res)
> > }
> >
> > while (rpmfiNext(fi) >= 0) {
> > -   /* Don't install signatures for (mutable) config files */
> > -   if (!(rpmfiFFlags(fi) & RPMFILE_CONFIG)) {
> > -  fpath = rpmfiFN(fi);
> > -  fsig = rpmfiFSignature(fi, );
> > -  if (fsig && (check_zero_hdr(fsig, len) == 0)) {
> > -  lsetxattr(fpath, XATTR_NAME_IMA, fsig, len, 0);
> > -  }
> > +   /* Don't install signatures for (mutable) files marked
> > +* as config files unless they are also executable.
> > +*/
> > +   if (rpmfiFFlags(fi) & RPMFILE_CONFIG) {
> > +   n = rpmfiStat(fi, 0, );
> > +   if (n != 0)
> > +   continue;
> > +   if ((statbuf.st_mode & 0111) == 0)
> > +   continue;
> 
> rpmfiStat() is not wrong, but it does far more than you need here and 
> complicates things a little. You could just use rpmfiFMode() for the 
> mode bits.
> 
> Also generally it's preferred to avoid magic numbers when it can be 
> easily expressed with defined names, (S_IXUSR|S_IXGRP|S_IXOTH) is easier 

> for the reader than 0111.

Ok, I'll fix that.

> 
> As for the change itself, I think it also points to
> a) packaging bug in grub2-tools - except for those *_custom files these 
> are *not* configuration files that you're supposed to touch
> b) overall insanity of grub2 - inspect any of the files in /etc/grub.d, 
> trying to keep in mind these are bootloader "configuration"...
> 
> All that aside, I wonder about this change: what will happen now if the 
> user legitimately edits one of those executable %config files? The 
> signature no longer match, does that mean it'll fail to execute?

The change is necessary. There are two ways to run these scripts:

bash 00_header  or  ./00_header

The latter needs the signature, while the former does (currently) not need 
it. Following tests with grub the scripts seem to be executed using the 
latter method and fail if they don't have a signature.

Once IMA Appraisal is active, you would have to regard the system as being 
in lock-down mode. If you make changes to these files, you'd have to sign 
them, otherwise they will fail executing.

   Stefan

> 
>- Panu -
> 


___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH v2 4/4] IMA: Move the IMA plugin to the fsm_post hook

2016-09-22 Thread Stefan Berger

Move the IMA plugin to the fsm_post hook. Check whether the given
return code indicates and error, and do nothing in case it does
show an error. There is nothing to clean up, so we can do that.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 plugins/ima.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/plugins/ima.c b/plugins/ima.c
index 4a419b0..8dfc04d 100644
--- a/plugins/ima.c
+++ b/plugins/ima.c
@@ -35,7 +35,7 @@ static int check_zero_hdr(const unsigned char *fsig, size_t 
siglen)
return (memcmp(fsig, _hdr, sizeof(zero_hdr)) == 0);
 }
 
-static rpmRC ima_psm_post(rpmPlugin plugin, rpmte te, int res)
+static rpmRC ima_fsm_post(rpmPlugin plugin, rpmte te, int res)
 {
rpmfi fi = rpmteFI(te);
const char *fpath;
@@ -44,7 +44,7 @@ static rpmRC ima_psm_post(rpmPlugin plugin, rpmte te, int res)
int rc = 0, n;
struct stat statbuf;
 
-   if (rpmteType(te) != TR_ADDED)
+   if (res || rpmteType(te) != TR_ADDED)
return 0;
 
if (fi == NULL) {
@@ -75,5 +75,5 @@ exit:
 }
 
 struct rpmPluginHooks_s ima_hooks = {
-   .psm_post = ima_psm_post,
+   .fsm_post = ima_fsm_post,
 };
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH v2 3/4] rpmplugins: Introduce new fsm_pre and fsm_post hooks

2016-09-22 Thread Stefan Berger

Introduce fsm_pre and fsm_post hooks, which are invoked
before and after the package files are installed.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 lib/psm.c|  6 +-
 lib/rpmplugin.h  |  6 ++
 lib/rpmplugins.c | 35 +++
 lib/rpmplugins.h | 19 +++
 4 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/lib/psm.c b/lib/psm.c
index 341441b..ddd38d1 100644
--- a/lib/psm.c
+++ b/lib/psm.c
@@ -586,11 +586,13 @@ static rpmRC rpmpsmUnpack(rpmpsm psm)
 rpmpsmNotify(psm, RPMCALLBACK_INST_PROGRESS, 0);
 
 if (!(rpmtsFlags(psm->ts) & RPMTRANS_FLAG_JUSTDB)) {
-   if (rpmfilesFC(psm->files) > 0) {
+   fsmrc = rpmpluginsCallFsmPre(rpmtsPlugins(psm->ts), psm->te);
+   if (!fsmrc && rpmfilesFC(psm->files) > 0) {
fsmrc = rpmPackageFilesInstall(psm->ts, psm->te, psm->files,
   psm, );
saved_errno = errno;
}
+   rpmpluginsCallFsmPost(rpmtsPlugins(psm->ts), psm->te, fsmrc);
 }
 
 /* XXX make sure progress reaches 100% */
@@ -627,10 +629,12 @@ static rpmRC rpmpsmRemove(rpmpsm psm)
 
 /* XXX should't we log errors from here? */
 if (!(rpmtsFlags(psm->ts) & RPMTRANS_FLAG_JUSTDB)) {
+   fsmrc = rpmpluginsCallFsmPre(rpmtsPlugins(psm->ts), psm->te);
if (rpmfilesFC(psm->files) > 0) {
fsmrc = rpmPackageFilesRemove(psm->ts, psm->te, psm->files,
  psm, );
}
+   rpmpluginsCallFsmPost(rpmtsPlugins(psm->ts), psm->te, fsmrc);
 }
 /* XXX make sure progress reaches 100% */
 rpmpsmNotify(psm, RPMCALLBACK_UNINST_PROGRESS, psm->total);
diff --git a/lib/rpmplugin.h b/lib/rpmplugin.h
index fd81aec..b6f230c 100644
--- a/lib/rpmplugin.h
+++ b/lib/rpmplugin.h
@@ -41,6 +41,8 @@ typedef rpmRC (*plugin_init_func)(rpmPlugin plugin, rpmts ts);
 typedef void (*plugin_cleanup_func)(rpmPlugin plugin);
 typedef rpmRC (*plugin_tsm_pre_func)(rpmPlugin plugin, rpmts ts);
 typedef rpmRC (*plugin_tsm_post_func)(rpmPlugin plugin, rpmts ts, int res);
+typedef rpmRC (*plugin_fsm_pre_func)(rpmPlugin plugin, rpmte te);
+typedef rpmRC (*plugin_fsm_post_func)(rpmPlugin plugin, rpmte te, int res);
 typedef rpmRC (*plugin_psm_pre_func)(rpmPlugin plugin, rpmte te);
 typedef rpmRC (*plugin_psm_post_func)(rpmPlugin plugin, rpmte te, int res);
 typedef rpmRC (*plugin_scriptlet_pre_func)(rpmPlugin plugin,
@@ -60,6 +62,8 @@ typedef rpmRC (*plugin_fsm_file_prepare_func)(rpmPlugin 
plugin, rpmfi fi,
  const char* path,
  const char *dest,
  mode_t file_mode, rpmFsmOp op);
+typedef rpmRC (*plugin_fsm_pre_func)(rpmPlugin plugin, rpmte te);
+typedef rpmRC (*plugin_fsm_post_func)(rpmPlugin plugin, rpmte te, int res);
 
 typedef struct rpmPluginHooks_s * rpmPluginHooks;
 struct rpmPluginHooks_s {
@@ -80,6 +84,8 @@ struct rpmPluginHooks_s {
 plugin_fsm_file_pre_func   fsm_file_pre;
 plugin_fsm_file_post_func  fsm_file_post;
 plugin_fsm_file_prepare_func   fsm_file_prepare;
+plugin_fsm_pre_funcfsm_pre;
+plugin_fsm_post_func   fsm_post;
 };
 
 #ifdef __cplusplus
diff --git a/lib/rpmplugins.c b/lib/rpmplugins.c
index 97e5d30..98cd5ca 100644
--- a/lib/rpmplugins.c
+++ b/lib/rpmplugins.c
@@ -401,3 +401,38 @@ rpmRC rpmpluginsCallFsmFilePrepare(rpmPlugins plugins, 
rpmfi fi,
 
 return rc;
 }
+
+rpmRC rpmpluginsCallFsmPre(rpmPlugins plugins, rpmte te)
+{
+plugin_fsm_pre_func hookFunc;
+int i;
+rpmRC rc = RPMRC_OK;
+
+for (i = 0; i < plugins->count; i++) {
+   rpmPlugin plugin = plugins->plugins[i];
+   RPMPLUGINS_SET_HOOK_FUNC(fsm_pre);
+   if (hookFunc && hookFunc(plugin, te) == RPMRC_FAIL) {
+   rpmlog(RPMLOG_ERR, "Plugin %s: hook fsm_pre failed\n", 
plugin->name);
+   rc = RPMRC_FAIL;
+   }
+}
+
+return rc;
+}
+
+rpmRC rpmpluginsCallFsmPost(rpmPlugins plugins, rpmte te, int res)
+{
+plugin_fsm_post_func hookFunc;
+int i;
+rpmRC rc = RPMRC_OK;
+
+for (i = 0; i < plugins->count; i++) {
+   rpmPlugin plugin = plugins->plugins[i];
+   RPMPLUGINS_SET_HOOK_FUNC(fsm_post);
+   if (hookFunc && hookFunc(plugin, te, res) == RPMRC_FAIL) {
+   rpmlog(RPMLOG_WARNING, "Plugin %s: hook fsm_post failed\n", 
plugin->name);
+   }
+}
+
+return rc;
+}
diff --git a/lib/rpmplugins.h b/lib/rpmplugins.h
index 39762c3..3702526 100644
--- a/lib/rpmplugins.h
+++ b/lib/rpmplugins.h
@@ -167,6 +167,25 @@ rpmRC rpmpluginsCallFsmFilePrepare(rpmPlugins plugins, 
rpmfi fi,
const char *path, const char *dest,

[Rpm-maint] [PATCH v2 1/4] ima-plugin: Have executable configuration files signed

2016-09-22 Thread Stefan Berger

Some configuration files are executables and so they require the
signature in the extended attribute. If they are not executable,
they can be skipped.

Examples for configuration files that are also executables are
the grub files in /etc/grub.d.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 plugins/ima.c | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/plugins/ima.c b/plugins/ima.c
index be15ecf..81ed194 100644
--- a/plugins/ima.c
+++ b/plugins/ima.c
@@ -41,7 +41,8 @@ static rpmRC ima_psm_post(rpmPlugin plugin, rpmte te, int res)
const char *fpath;
const unsigned char * fsig = NULL;
size_t len;
-   int rc = 0;
+   int rc = 0, n;
+   struct stat statbuf;
 
if (fi == NULL) {
rc = RPMERR_BAD_MAGIC;
@@ -49,13 +50,21 @@ static rpmRC ima_psm_post(rpmPlugin plugin, rpmte te, int 
res)
}
 
while (rpmfiNext(fi) >= 0) {
-   /* Don't install signatures for (mutable) config files */
-   if (!(rpmfiFFlags(fi) & RPMFILE_CONFIG)) {
-   fpath = rpmfiFN(fi);
-   fsig = rpmfiFSignature(fi, );
-   if (fsig && (check_zero_hdr(fsig, len) == 0)) {
-   lsetxattr(fpath, XATTR_NAME_IMA, fsig, len, 0);
-   }
+   /* Don't install signatures for (mutable) files marked
+* as config files unless they are also executable.
+*/
+   if (rpmfiFFlags(fi) & RPMFILE_CONFIG) {
+   n = rpmfiStat(fi, 0, );
+   if (n != 0)
+   continue;
+   if ((statbuf.st_mode & 0111) == 0)
+   continue;
+   }
+
+   fsig = rpmfiFSignature(fi, );
+   if (fsig && (check_zero_hdr(fsig, len) == 0)) {
+   fpath = rpmfiFN(fi);
+   lsetxattr(fpath, XATTR_NAME_IMA, fsig, len, 0);
}
}
 exit:
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 0/3] Fixes for file signatures

2016-09-21 Thread Stefan Berger

The following series of patches addresses some issues with signatures on
files. In particular:

- some files marked as config files are also executables and therefore
  need to have a signature applied
- the IMA plugin may only run on package install cycle rather than the
  remove cycle, which would apply the previous versions' signatures on
  the files
- some RPM packages require that the files be signed when the post
  install scriptlets are run since they may invoke executables that
  were just installed; so we have to also run the IMA plugin on the
  scriptlet_pre plugin hook, but have to extend that hook with the rpmte
  parameter type

   Regards,
  Stefan

Stefan Berger (3):
  ima-plugin: Have executable configuration files signed
  ima-plugin: Only run the IMA plugin on package installation
  plugins: Pass rpmte to scriptlet_pre and call IMA plugin in this hook

 lib/rpmplugin.h   |  3 ++-
 lib/rpmplugins.c  |  5 +++--
 lib/rpmplugins.h  |  3 ++-
 lib/rpmscript.c   |  5 +++--
 lib/rpmscript.h   |  3 ++-
 lib/transaction.c |  2 +-
 plugins/ima.c | 38 ++
 7 files changed, 43 insertions(+), 16 deletions(-)

-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH 3/3] plugins: Pass rpmte to scriptlet_pre and call IMA plugin in this hook

2016-09-21 Thread Stefan Berger

Stefan Berger <stef...@linux.vnet.ibm.com> wrote on 09/21/2016 02:04:08 
PM:

> From: Stefan Berger <stef...@linux.vnet.ibm.com>
> To: rpm-maint@lists.rpm.org
> Cc: fionnuala.gun...@gmail.com, stef...@linux.vnet.ibm.com, 
> zo...@linux.vnet.ibm.com, Stefan Berger/Watson/IBM@IBMUS
> Date: 09/21/2016 02:04 PM
> Subject: [PATCH 3/3] plugins: Pass rpmte to scriptlet_pre and call 
> IMA plugin in this hook
> 
> The IMA plugin needs to also be called before the post installation
> scriptlet is run. The reason for this is that some post installation
> scriptlets invoke the tools that were just installed. The invocatin
> fails, if the signatures have not been applied, yet. Therefore, we
> invoke the IMA plugin with the scriptlet_pre hook.
> 
> To be able to do the work in the scriptlet_pre hook, we also need to
> pass the tpmte parameter all the way through.
> 
> An example for an RPM that invokes its own programs is coreutils,
> which will invoke /bin/mv in the post installation script.
> 
> Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
> ---
>  lib/rpmplugin.h   |  3 ++-
>  lib/rpmplugins.c  |  5 +++--
>  lib/rpmplugins.h  |  3 ++-
>  lib/rpmscript.c   |  5 +++--
>  lib/rpmscript.h   |  3 ++-
>  lib/transaction.c |  2 +-
>  plugins/ima.c | 10 ++
>  7 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/rpmplugin.h b/lib/rpmplugin.h
> index fd81aec..98205db 100644
> --- a/lib/rpmplugin.h
> +++ b/lib/rpmplugin.h
> @@ -44,7 +44,8 @@ typedef rpmRC (*plugin_tsm_post_func)(rpmPlugin 
> plugin, rpmts ts, int res);
>  typedef rpmRC (*plugin_psm_pre_func)(rpmPlugin plugin, rpmte te);
>  typedef rpmRC (*plugin_psm_post_func)(rpmPlugin plugin, rpmte te, int 
res);
>  typedef rpmRC (*plugin_scriptlet_pre_func)(rpmPlugin plugin,
> -  const char *s_name, int type);
> +  const char *s_name, int type,
> +  rpmte te);


I am obviously modifying a public interface here. This modification does 
no harm to other plugins living in the rpm git tree since none of them is 
called in this callback hook. Are there any plugins that live outside the 
tree that would now not compile anymore? Another solution would be to 
introduce a plugin_scriptlet_pre_te_func.

   Stefan


___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 2/3] ima-plugin: Only run the IMA plugin on package installation

2016-09-21 Thread Stefan Berger

We want to prevent that the IMA plugin applies signatures of the older
version of files. So we have to check whether we are in the install
(TR_ADDED) or remove (TR_REMOVED) cycle of a package. We only apply
signatures in the install cycle.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 plugins/ima.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/plugins/ima.c b/plugins/ima.c
index 81ed194..4a419b0 100644
--- a/plugins/ima.c
+++ b/plugins/ima.c
@@ -44,6 +44,9 @@ static rpmRC ima_psm_post(rpmPlugin plugin, rpmte te, int res)
int rc = 0, n;
struct stat statbuf;
 
+   if (rpmteType(te) != TR_ADDED)
+   return 0;
+
if (fi == NULL) {
rc = RPMERR_BAD_MAGIC;
goto exit;
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 1/3] ima-plugin: Have executable configuration files signed

2016-09-21 Thread Stefan Berger

Some configuration files are executables and so they require the
signature in the extended attribute. If they are not executable,
they can be skipped.

Examples for configuration files that are also executables are
the grub files in /etc/grub.d.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 plugins/ima.c | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/plugins/ima.c b/plugins/ima.c
index be15ecf..81ed194 100644
--- a/plugins/ima.c
+++ b/plugins/ima.c
@@ -41,7 +41,8 @@ static rpmRC ima_psm_post(rpmPlugin plugin, rpmte te, int res)
const char *fpath;
const unsigned char * fsig = NULL;
size_t len;
-   int rc = 0;
+   int rc = 0, n;
+   struct stat statbuf;
 
if (fi == NULL) {
rc = RPMERR_BAD_MAGIC;
@@ -49,13 +50,21 @@ static rpmRC ima_psm_post(rpmPlugin plugin, rpmte te, int 
res)
}
 
while (rpmfiNext(fi) >= 0) {
-   /* Don't install signatures for (mutable) config files */
-   if (!(rpmfiFFlags(fi) & RPMFILE_CONFIG)) {
-   fpath = rpmfiFN(fi);
-   fsig = rpmfiFSignature(fi, );
-   if (fsig && (check_zero_hdr(fsig, len) == 0)) {
-   lsetxattr(fpath, XATTR_NAME_IMA, fsig, len, 0);
-   }
+   /* Don't install signatures for (mutable) files marked
+* as config files unless they are also executable.
+*/
+   if (rpmfiFFlags(fi) & RPMFILE_CONFIG) {
+   n = rpmfiStat(fi, 0, );
+   if (n != 0)
+   continue;
+   if ((statbuf.st_mode & 0111) == 0)
+   continue;
+   }
+
+   fsig = rpmfiFSignature(fi, );
+   if (fsig && (check_zero_hdr(fsig, len) == 0)) {
+   fpath = rpmfiFN(fi);
+   lsetxattr(fpath, XATTR_NAME_IMA, fsig, len, 0);
}
}
 exit:
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH] rpmsign: Use default hash algo if RPMTAG_FILEDIGESTALGO missing

2016-08-09 Thread Stefan Berger

Use the default hash algorithm md5 on RPMs that do not contain the
RPMTAG_FILEDIGESTALGO. This may be the case if the default hash
algorithm used on files is md5 and thus no RPMTAG_FILEDIGESTALGO is
being written (see build/files.c:genCpioListAndHeader()).

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 lib/rpmsignfiles.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/rpmsignfiles.c b/lib/rpmsignfiles.c
index 3cd2b1a..87e4e42 100644
--- a/lib/rpmsignfiles.c
+++ b/lib/rpmsignfiles.c
@@ -133,10 +133,9 @@ rpmRC rpmSignFiles(Header h, const char *key, char 
*keypass)
 
 algo = headerGetNumber(h, RPMTAG_FILEDIGESTALGO);
 if (!algo) {
-   rpmlog(RPMLOG_ERR, _("missing RPMTAG_FILEDIGESTALGO\n"));
-   return RPMRC_FAIL;
-}
-if (algo < 0 || algo >= ARRAY_SIZE(hash_algo_name)) {
+/* use default algorithm */
+algo = PGPHASHALGO_MD5;
+} else if (algo < 0 || algo >= ARRAY_SIZE(hash_algo_name)) {
rpmlog(RPMLOG_ERR, _("File digest algorithm id is invalid"));
return RPMRC_FAIL;
 }
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 1/5] Fix indentation and formatting

2016-06-06 Thread Stefan Berger

Fix the indentation and formatting in signature related files.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 lib/rpmsignfiles.c | 12 ++--
 sign/rpmgensig.c   |  3 ++-
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/rpmsignfiles.c b/lib/rpmsignfiles.c
index 61ea33e..95ac851 100644
--- a/lib/rpmsignfiles.c
+++ b/lib/rpmsignfiles.c
@@ -138,13 +138,13 @@ rpmRC rpmSignFiles(Header h, const char *key, char 
*keypass)
return RPMRC_FAIL;
 }
 
-   headerDel(h, RPMTAG_FILESIGNATURELENGTH);
-   headerDel(h, RPMTAG_FILESIGNATURES);
-   siglen = signatureLength(algoname, diglen, key, keypass);
-   headerPutUint32(h, RPMTAG_FILESIGNATURELENGTH, , 1);
+headerDel(h, RPMTAG_FILESIGNATURELENGTH);
+headerDel(h, RPMTAG_FILESIGNATURES);
+siglen = signatureLength(algoname, diglen, key, keypass);
+headerPutUint32(h, RPMTAG_FILESIGNATURELENGTH, , 1);
 
-   headerGet(h, RPMTAG_FILEDIGESTS, , HEADERGET_MINMEM);
-   while ((digest = rpmtdNextString())) {
+headerGet(h, RPMTAG_FILEDIGESTS, , HEADERGET_MINMEM);
+while ((digest = rpmtdNextString())) {
signature = signFile(algoname, digest, diglen, key, keypass);
if (!signature) {
rpmlog(RPMLOG_ERR, _("signFile failed\n"));
diff --git a/sign/rpmgensig.c b/sign/rpmgensig.c
index 9341fa8..77b6d5a 100644
--- a/sign/rpmgensig.c
+++ b/sign/rpmgensig.c
@@ -655,7 +655,8 @@ static rpmRC includeFileSignatures(FD_t fd, const char *rpm,
replaceSigDigests(fd, rpm, sigp, sigStart, sigTargetSize, SHA1, MD5);
 
 exit:
-if (ofd)(void) closeFile();
+if (ofd)
+   (void) closeFile();
 return rc;
 }
 
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 3/5] Check range of algo index parameter before accessing array with it

2016-06-06 Thread Stefan Berger

Check the range of the algo index parameter before using it for
accessing an array.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 lib/rpmsignfiles.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/lib/rpmsignfiles.c b/lib/rpmsignfiles.c
index b7d9ccc..97a5be4 100644
--- a/lib/rpmsignfiles.c
+++ b/lib/rpmsignfiles.c
@@ -32,6 +32,8 @@ static const char *hash_algo_name[] = {
 [PGPHASHALGO_SHA224]   = "sha224",
 };
 
+#define ARRAY_SIZE(a)  (sizeof(a) / sizeof(a[0]))
+
 char *get_fskpass(void)
 {
 struct termios flags, tmp_flags;
@@ -130,6 +132,10 @@ rpmRC rpmSignFiles(Header h, const char *key, char 
*keypass)
rpmlog(RPMLOG_ERR, _("missing RPMTAG_FILEDIGESTALGO\n"));
return RPMRC_FAIL;
 }
+if (algo < 0 || algo >= ARRAY_SIZE(hash_algo_name)) {
+   rpmlog(RPMLOG_ERR, _("File digest algorithm id is invalid"));
+   return RPMRC_FAIL;
+}
 
 diglen = rpmDigestLength(algo);
 algoname = hash_algo_name[algo];
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 4/5] Extend header size to 64MB due to file signatures

2016-06-06 Thread Stefan Berger

Extend the header size to 64MB in case an RPM has a lot of files
and the file signatures do not fit within the current limit of 16MB.

An example for an RPM with many files is kcbench-data-4.0. It contains
more than 52000 files. With each signature with a 2048 bit key requiring
256 bytes plus a preamble, its representation in text from, and other
overhead, the size of the header (index length and data length) exceeds
32Mb.

If this RPM's files have been signed using this patch, older versions
of the rpm tool will report the header being too large. So this
failure is expected then.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 lib/header.c  | 2 +-
 lib/header_internal.h | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/header.c b/lib/header.c
index 81f2038..ae292f9 100644
--- a/lib/header.c
+++ b/lib/header.c
@@ -99,7 +99,7 @@ struct headerToken_s {
 /** \ingroup header
  * Maximum no. of bytes permitted in a header.
  */
-static const size_t headerMaxbytes = (32*1024*1024);
+static const size_t headerMaxbytes = (64*1024*1024);
 
 #defineINDEX_MALLOC_SIZE   8
 
diff --git a/lib/header_internal.h b/lib/header_internal.h
index bbe2097..410ad58 100644
--- a/lib/header_internal.h
+++ b/lib/header_internal.h
@@ -45,9 +45,10 @@ struct indexEntry_s {
 
 /**
  * Sanity check on data size and/or offset and/or count.
- * This check imposes a limit of 16 MB, more than enough.
+ * This check imposes a limit of 64 MB -- file signatures
+ * may require a lot of space in the header.
  */
-#define HEADER_DATA_MAX 0x00ff
+#define HEADER_DATA_MAX 0x03ff
 #define hdrchkData(_nbytes) ((_nbytes) & (~HEADER_DATA_MAX))
 
 /**
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 2/5] Fix various memory leaks in file signature related functions.

2016-06-06 Thread Stefan Berger

Fix various memory leaks in file signature related functions.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 lib/rpmsignfiles.c |  2 ++
 rpmsign.c  |  4 +++-
 sign/rpmgensig.c   | 24 +---
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/lib/rpmsignfiles.c b/lib/rpmsignfiles.c
index 95ac851..b7d9ccc 100644
--- a/lib/rpmsignfiles.c
+++ b/lib/rpmsignfiles.c
@@ -152,10 +152,12 @@ rpmRC rpmSignFiles(Header h, const char *key, char 
*keypass)
goto exit;
}
if (!headerPutString(h, RPMTAG_FILESIGNATURES, signature)) {
+   free(signature);
rpmlog(RPMLOG_ERR, _("headerPutString failed\n"));
rc = RPMRC_FAIL;
goto exit;
}
+   free(signature);
 }
 
 exit:
diff --git a/rpmsign.c b/rpmsign.c
index a61981a..ddbc5c5 100644
--- a/rpmsign.c
+++ b/rpmsign.c
@@ -60,6 +60,7 @@ static int doSign(poptContext optCon)
 char * passPhrase = NULL;
 char * name = rpmExpand("%{?_gpg_name}", NULL);
 struct rpmSignArgs sig = {NULL, 0, 0};
+char *key = NULL;
 
 if (rstreq(name, "")) {
fprintf(stderr, _("You must set \"%%_gpg_name\" in your macro file\n"));
@@ -71,7 +72,7 @@ static int doSign(poptContext optCon)
 }
 
 if (signfiles) {
-   const char *key = rpmExpand("%{?_file_signing_key}", NULL);
+   key = rpmExpand("%{?_file_signing_key}", NULL);
if (rstreq(key, "")) {
fprintf(stderr, _("You must set \"$$_file_signing_key\" in your 
macro file or on the command line with --fskpath\n"));
goto exit;
@@ -102,6 +103,7 @@ static int doSign(poptContext optCon)
 }
 
 exit:
+free(key);
 free(passPhrase);
 free(name);
 return rc;
diff --git a/sign/rpmgensig.c b/sign/rpmgensig.c
index 77b6d5a..0b23db3 100644
--- a/sign/rpmgensig.c
+++ b/sign/rpmgensig.c
@@ -576,8 +576,10 @@ static rpmRC includeFileSignatures(FD_t fd, const char 
*rpm,
 key = rpmExpand("%{?_file_signing_key}", NULL);
 
 keypass = rpmExpand("%{_file_signing_key_password}", NULL);
-if (rstreq(keypass, ""))
+if (rstreq(keypass, "")) {
+   free(keypass);
keypass = NULL;
+}
 
 rc = rpmSignFiles(*hdrp, key, keypass);
 if (rc != RPMRC_OK) {
@@ -641,11 +643,15 @@ static rpmRC includeFileSignatures(FD_t fd, const char 
*rpm,
 sigTargetSize = Ftell(fd) - headerStart;
 fdFiniDigest(fd, PGPHASHALGO_MD5, (void **), , 0);
 
-if (headerGet(*sigp, RPMSIGTAG_MD5, , HEADERGET_DEFAULT))
+if (headerGet(*sigp, RPMSIGTAG_MD5, , HEADERGET_DEFAULT)) {
memcpy(o_md5, osigtd.data, 16);
+   rpmtdFreeData();
+}
 
-if (headerGet(*sigp, RPMSIGTAG_SHA1, , HEADERGET_DEFAULT))
+if (headerGet(*sigp, RPMSIGTAG_SHA1, , HEADERGET_DEFAULT)) {
o_sha1 = xstrdup(osigtd.data);
+   rpmtdFreeData();
+}
 
 if (memcmp(MD5, o_md5, md5len) == 0 && strcmp(SHA1, o_sha1) == 0)
rpmlog(RPMLOG_WARNING,
@@ -655,6 +661,12 @@ static rpmRC includeFileSignatures(FD_t fd, const char 
*rpm,
replaceSigDigests(fd, rpm, sigp, sigStart, sigTargetSize, SHA1, MD5);
 
 exit:
+free(trpm);
+free(MD5);
+free(SHA1);
+free(o_sha1);
+free(keypass);
+free(key);
 if (ofd)
(void) closeFile();
 return rc;
@@ -675,7 +687,7 @@ static int rpmSign(const char *rpm, int deleting, int 
signfiles)
 char *trpm = NULL;
 Header sigh = NULL;
 Header h = NULL;
-char * msg = NULL;
+char *msg = NULL;
 int res = -1; /* assume failure */
 rpmRC rc;
 struct rpmtd_s utd;
@@ -693,7 +705,6 @@ static int rpmSign(const char *rpm, int deleting, int 
signfiles)
 
 if ((rc = rpmLeadRead(fd, , NULL, )) != RPMRC_OK) {
rpmlog(RPMLOG_ERR, "%s: %s\n", rpm, msg);
-   free(msg);
goto exit;
 }
 
@@ -702,14 +713,12 @@ static int rpmSign(const char *rpm, int deleting, int 
signfiles)
 if (rc != RPMRC_OK) {
rpmlog(RPMLOG_ERR, _("%s: rpmReadSignature failed: %s"), rpm,
(msg && *msg ? msg : "\n"));
-   msg = _free(msg);
goto exit;
 }
 
 headerStart = Ftell(fd);
 if (rpmReadHeader(NULL, fd, , ) != RPMRC_OK) {
rpmlog(RPMLOG_ERR, _("%s: headerRead failed: %s\n"), rpm, msg);
-   msg = _free(msg);
goto exit;
 }
 
@@ -845,6 +854,7 @@ exit:
 rpmFreeSignature(sigh);
 headerFree(h);
 rpmLeadFree(lead);
+free(msg);
 
 /* Clean up intermediate target */
 if (trpm) {
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 5/5] Fix handling of zero-length file digests

2016-06-06 Thread Stefan Berger

Do not try to convert a zero-length file digest to a binary representation.
Zero-length file digests may stem from directory entries and symbolic links.
Return an empty signature in this case.

Returning an empty signature results in the ima.so plugin getting a sequence
of zeroes that it would write into security.ima xattr. Check for a signature
consisting of only zeroes and do not write it into the filesystem.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 lib/rpmsignfiles.c |  4 
 plugins/ima.c  | 36 +++-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/lib/rpmsignfiles.c b/lib/rpmsignfiles.c
index 97a5be4..3cd2b1a 100644
--- a/lib/rpmsignfiles.c
+++ b/lib/rpmsignfiles.c
@@ -82,6 +82,10 @@ const char *key, char *keypass)
 
 /* convert file digest hex to binary */
 memset(digest, 0, diglen);
+/* some entries don't have a digest - we return an empty signature */
+if (strlen(fdigest) != diglen * 2)
+return strdup("");
+
 for (int i = 0; i < diglen; ++i, fdigest += 2)
digest[i] = (rnibble(fdigest[0]) << 4) | rnibble(fdigest[1]);
 
diff --git a/plugins/ima.c b/plugins/ima.c
index 0dfdd8b..2b998d0 100644
--- a/plugins/ima.c
+++ b/plugins/ima.c
@@ -12,6 +12,40 @@
 
 #define XATTR_NAME_IMA "security.ima"
 
+/* security.ima signature header */
+struct signature_v2_hdr {
+   uint8_t type;
+   uint8_t version;
+   uint8_t hash_algo;
+   uint32_t keyid;
+   uint16_t sig_size;
+   uint8_t sig[0];
+} __attribute__((PACKED));
+
+static const struct signature_v2_hdr zero_hdr = {
+   .type = 0,
+   .version = 0,
+   .hash_algo = 0,
+   .keyid = 0,
+   .sig_size = 0,
+};
+
+/*
+ * check_zero_hdr: Check the signature for a zero header
+ *
+ * Check whether the given signature has a header with all zeros
+ *
+ * Returns -1 in case the signature is too short to compare
+ * (invalid signature), 0 in case the header is not only zeroes,
+ * and 1 if it is only zeroes.
+ */
+static int check_zero_hdr(const unsigned char *fsig, size_t siglen)
+{
+   if (siglen < sizeof(zero_hdr))
+   return -1;
+   return (memcmp(fsig, _hdr, sizeof(zero_hdr)) == 0);
+}
+
 static rpmRC ima_psm_post(rpmPlugin plugin, rpmte te, int res)
 {
rpmfi fi = rpmteFI(te);
@@ -30,7 +64,7 @@ static rpmRC ima_psm_post(rpmPlugin plugin, rpmte te, int res)
if (!(rpmfiFFlags(fi) & RPMFILE_CONFIG)) {
fpath = rpmfiFN(fi);
fsig = rpmfiFSignature(fi, );
-   if (fsig) {
+   if (fsig && (check_zero_hdr(fsig, len) == 0)) {
lsetxattr(fpath, XATTR_NAME_IMA, fsig, len, 0);
}
}
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [rpm-software-management/rpm] Fix off-by-one error (#68)

2016-05-25 Thread Stefan Berger

> @@ -104,7 +104,7 @@ static int base64_decode_value(unsigned char value_in)
>  {
>   static const int decoding[] = 
> {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
>   value_in -= 43;
> - if (value_in > sizeof(decoding)/sizeof(int))
> + if (value_in >= sizeof(decoding)/sizeof(int))

my 2 cents: also change to sizeof(decoding)/sizeof(decoding[0])

---
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/68/files/0964912b94f9f48a0a812fbfbb2f996dbd93eff0#r64628600___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH v2 1/2] Extend header size to 256MB due to file signatures

2016-04-29 Thread Stefan Berger

"Rpm-maint" <rpm-maint-boun...@lists.rpm.org> wrote on 04/29/2016 01:42:06 
PM:

> 
> On Fri, 29 Apr 2016, Stefan Berger wrote:
> 
> > From: Stefan Berger <stef...@us.ibm.com>
> > 
> > Extend the header size to 256MB in case an RPM has a lot of files
> > and the file signatures do not fit within the current limit of 16MB.
> 
> a (huge) fixed extension allocation, rather than writing a 
> 'chained block allocation on demand' mechanism seems 
> 'un-Unixy' and a kludge 
> 
> ... in a current install of 1900 packages, only 300 are of a 
> size greater than 256MBy.  The penalty for a repository to 
> carry around that unused slack space is an installation image 
> is huge

No space is allocated (for file signatures) inside an RPM's header that 
isn't needed. 

So maybe the title of the commit message is misleading and it should say 
that we are extending the 'limit' of the header size to 256MB. There are 
checks in the code that make rpm fail if the header becomes larger than 
that limit. 16MB weren't enough, so we extended the limit to 256MB. 
There's no space allocation for 256MB per package that would just carry 
around zeroes, if that's what you meant.

   Stefan

> 
> I would recommend that if a bigger header space requirement 
> exists, that it be done in a fashion other than 'using a 
> bigger hammer'
> 
> -- Russ herrold
> ___
> Rpm-maint mailing list
> Rpm-maint@lists.rpm.org
> http://lists.rpm.org/mailman/listinfo/rpm-maint
> 

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH 4/5] Extend header size to 64MB due to file signatures

2016-04-29 Thread Stefan Berger

Lubos Kardos <lkar...@redhat.com> wrote on 04/29/2016 06:40:16 AM:

> 
> It is just a thought. Rpm transaction can be divided in two phases.
> In the first phase in the beginning of transaction rpm loads all file 
infos to
> perform transactions checks and then releases them. In the second phase 
rpm
> reloads single file infos to install single packages in row. The memory 
peak
> happens in the first phase when all file infos are loaded. These file 
infos
> contain also file signatures but in the first phase they needn't to 
contain
> them because the signature checking is performed only in the second 
phase.
> 
> So if the file signatures blow up the file infos so much so we need 
> to increase
> maximum header size then maybe it would be nice not to load file 
signatures
> into file infos during the first phase of transaction when the rpm 
memory peak
> happens.

I agreed and it would be a separate patch.

I didn't look very deeply, but how does one detect the phases? I suppose 
the part
to skip would be in lib/trpmfi.c::rpmfilePopulate where the flag 
RPMFI_NOFILESIGNATURES is
checked. If that's the case, maybe the 1st phase could call this function 
with this
flag always set?

Stefan

> 
> Lubos
> 
> - Original Message -
> > From: "Florian Festi" <ffe...@redhat.com>
> > To: "Stefan Berger" <stef...@us.ibm.com>
> > Cc: rpm-maint@lists.rpm.org
> > Sent: Friday, April 29, 2016 10:27:39 AM
> > Subject: Re: [Rpm-maint] [PATCH 4/5] Extend header size to 64MB 
> due to file signatures
> > 
> > On 04/27/2016 09:47 PM, Stefan Berger wrote:
> > > "Rpm-maint" <rpm-maint-boun...@lists.rpm.org> wrote on 04/27/2016
> > > 05:50:54 AM:
> > > 
> > > 
> > >>
> > >> Well changing header size limit needs a bit more thought. The main
> > >> problem is that packages with bigger header will look broken on 
older
> > >> rpm versions and the usual way of dealing with this (adding 
rpmlib()
> > >> Requires) won't work it needs reading the header.
> > > 
> > > These huge headers are only occurring in a few very large packages 
and
> > > only if one applies the per-file signatures. So most users probably
> > > won't notice.
> > > 
> > >>
> > >> Also I wonder if we should increase the header size even more, to 
get
> > >> rid of this topic for a longer time. I thought about 256MB which 
gives a
> > >> 4 times increase over the 16MB. I am kinda tempted to go even 
further.
> > >> Otoh the limit is there for a reason. And having rpm chew through 
one GB
> > >> of broken data doesn't sound like a pleasant experience.
> > > 
> > > Anything >=16 MB works with signed files for all packages in Fedora 
23.
> > > Let me know if you want me to resubmit the patch with a higher 
limit.
> > 
> > Yes, please. 256MB is probably the way to go. Let's hope we don't 
reach
> > that any time soon.
> > 
> > Florian
> > 
> > --
> > 
> > Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn,
> > Commercial register: Amtsgericht Muenchen, HRB 153243,
> > Managing Directors: Paul Argiry, Charles Cachera, Michael Cunningham,
> > Michael O'Neill
> > ___
> > Rpm-maint mailing list
> > Rpm-maint@lists.rpm.org
> > http://lists.rpm.org/mailman/listinfo/rpm-maint
> > 
> 


___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH v2 2/2] Fix handling of zero-length file digests

2016-04-29 Thread Stefan Berger

From: Stefan Berger <stef...@us.ibm.com>

Do not try to convert a zero-length file digest to a binary representation.
Zero-length file digests may stem from directory entries and symbolic links.
Return an empty signature in this case.

Returning an empty signature results in the ima.so plugin getting a sequence
of zeroes that it would write into security.ima xattr. Check for a signature
header consisting of only zeroes and do not write it into the filesystem.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 lib/rpmsignfiles.c |  4 
 plugins/ima.c  | 25 -
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/lib/rpmsignfiles.c b/lib/rpmsignfiles.c
index 97a5be4..3cd2b1a 100644
--- a/lib/rpmsignfiles.c
+++ b/lib/rpmsignfiles.c
@@ -82,6 +82,10 @@ const char *key, char *keypass)
 
 /* convert file digest hex to binary */
 memset(digest, 0, diglen);
+/* some entries don't have a digest - we return an empty signature */
+if (strlen(fdigest) != diglen * 2)
+return strdup("");
+
 for (int i = 0; i < diglen; ++i, fdigest += 2)
digest[i] = (rnibble(fdigest[0]) << 4) | rnibble(fdigest[1]);
 
diff --git a/plugins/ima.c b/plugins/ima.c
index 0dfdd8b..be15ecf 100644
--- a/plugins/ima.c
+++ b/plugins/ima.c
@@ -12,6 +12,29 @@
 
 #define XATTR_NAME_IMA "security.ima"
 
+/*
+ * check_zero_hdr: Check the signature for a zero header
+ *
+ * Check whether the given signature has a header with all zeros
+ *
+ * Returns -1 in case the signature is too short to compare
+ * (invalid signature), 0 in case the header is not only zeroes,
+ * and 1 if it is only zeroes.
+ */
+static int check_zero_hdr(const unsigned char *fsig, size_t siglen)
+{
+   /*
+* Every signature has a header signature_v2_hdr as defined in
+* Linux's (4.5) security/integrity/integtrity.h. The following
+* 9 bytes represent this header in front of the signature.
+*/
+   static const uint8_t zero_hdr[] = {0, 0, 0, 0, 0, 0, 0, 0, 0};
+
+   if (siglen < sizeof(zero_hdr))
+   return -1;
+   return (memcmp(fsig, _hdr, sizeof(zero_hdr)) == 0);
+}
+
 static rpmRC ima_psm_post(rpmPlugin plugin, rpmte te, int res)
 {
rpmfi fi = rpmteFI(te);
@@ -30,7 +53,7 @@ static rpmRC ima_psm_post(rpmPlugin plugin, rpmte te, int res)
if (!(rpmfiFFlags(fi) & RPMFILE_CONFIG)) {
fpath = rpmfiFN(fi);
fsig = rpmfiFSignature(fi, );
-   if (fsig) {
+   if (fsig && (check_zero_hdr(fsig, len) == 0)) {
lsetxattr(fpath, XATTR_NAME_IMA, fsig, len, 0);
}
}
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH v2 1/2] Extend header size to 256MB due to file signatures

2016-04-29 Thread Stefan Berger

From: Stefan Berger <stef...@us.ibm.com>

Extend the header size to 256MB in case an RPM has a lot of files
and the file signatures do not fit within the current limit of 16MB.

An example for an RPM with many files is kcbench-data-4.0. It contains
more than 52000 files. With each signature with a 2048 bit key requiring
256 bytes plus a preamble, its representation in text from, and other
overhead, the size of the header (index length and data length) exceeds
32Mb.

If this particular RPM's files have been signed using this patch, older
versions of the rpm tool will report the header being too large. So this
failure is expected then.

By setting the limit to 256MB we create a lot of room for the future.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 lib/header.c  | 2 +-
 lib/header_internal.h | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/header.c b/lib/header.c
index 81f2038..7f7c115 100644
--- a/lib/header.c
+++ b/lib/header.c
@@ -99,7 +99,7 @@ struct headerToken_s {
 /** \ingroup header
  * Maximum no. of bytes permitted in a header.
  */
-static const size_t headerMaxbytes = (32*1024*1024);
+static const size_t headerMaxbytes = (256*1024*1024);
 
 #defineINDEX_MALLOC_SIZE   8
 
diff --git a/lib/header_internal.h b/lib/header_internal.h
index bbe2097..aed3977 100644
--- a/lib/header_internal.h
+++ b/lib/header_internal.h
@@ -45,9 +45,10 @@ struct indexEntry_s {
 
 /**
  * Sanity check on data size and/or offset and/or count.
- * This check imposes a limit of 16 MB, more than enough.
+ * This check imposes a limit of 256 MB -- file signatures
+ * may require a lot of space in the header.
  */
-#define HEADER_DATA_MAX 0x00ff
+#define HEADER_DATA_MAX 0x0fff
 #define hdrchkData(_nbytes) ((_nbytes) & (~HEADER_DATA_MAX))
 
 /**
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Re: [Rpm-maint] [PATCH 5/5] Fix handling of zero-length file digests

2016-04-27 Thread Stefan Berger

"Rpm-maint"  wrote on 04/27/2016 05:45:56 
AM:

> 
> I get the following warning:
> 
> ima.c:23:1: warning: ‘PACKED’ attribute directive ignored [-Wattributes]
>  } __attribute__((PACKED));
> 
> May be there is an simpler way to check for the header being zeros only?

One way of doing it would be to create an array of 9 zero bytes (with a 
comment of what it represents) and compare against that.

I guess you wouldn't like gcc #pragma tricks to disable -Wattributes in 
this case.

Stefan

> 
> Florian
> 
> -- 
> 
> Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn,
> Commercial register: Amtsgericht Muenchen, HRB 153243,
> Managing Directors: Paul Argiry, Charles Cachera, Michael Cunningham,
> Michael O'Neill
> ___
> Rpm-maint mailing list
> Rpm-maint@lists.rpm.org
> http://lists.rpm.org/mailman/listinfo/rpm-maint
> 


___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 1/5] Fix indentation and formatting

2016-04-25 Thread Stefan Berger

From: Stefan Berger <stef...@us.ibm.com>

Fix the indentation and formatting in signature related files.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 lib/rpmsignfiles.c | 12 ++--
 sign/rpmgensig.c   |  3 ++-
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/rpmsignfiles.c b/lib/rpmsignfiles.c
index 61ea33e..95ac851 100644
--- a/lib/rpmsignfiles.c
+++ b/lib/rpmsignfiles.c
@@ -138,13 +138,13 @@ rpmRC rpmSignFiles(Header h, const char *key, char 
*keypass)
return RPMRC_FAIL;
 }
 
-   headerDel(h, RPMTAG_FILESIGNATURELENGTH);
-   headerDel(h, RPMTAG_FILESIGNATURES);
-   siglen = signatureLength(algoname, diglen, key, keypass);
-   headerPutUint32(h, RPMTAG_FILESIGNATURELENGTH, , 1);
+headerDel(h, RPMTAG_FILESIGNATURELENGTH);
+headerDel(h, RPMTAG_FILESIGNATURES);
+siglen = signatureLength(algoname, diglen, key, keypass);
+headerPutUint32(h, RPMTAG_FILESIGNATURELENGTH, , 1);
 
-   headerGet(h, RPMTAG_FILEDIGESTS, , HEADERGET_MINMEM);
-   while ((digest = rpmtdNextString())) {
+headerGet(h, RPMTAG_FILEDIGESTS, , HEADERGET_MINMEM);
+while ((digest = rpmtdNextString())) {
signature = signFile(algoname, digest, diglen, key, keypass);
if (!signature) {
rpmlog(RPMLOG_ERR, _("signFile failed\n"));
diff --git a/sign/rpmgensig.c b/sign/rpmgensig.c
index 9341fa8..77b6d5a 100644
--- a/sign/rpmgensig.c
+++ b/sign/rpmgensig.c
@@ -655,7 +655,8 @@ static rpmRC includeFileSignatures(FD_t fd, const char *rpm,
replaceSigDigests(fd, rpm, sigp, sigStart, sigTargetSize, SHA1, MD5);
 
 exit:
-if (ofd)(void) closeFile();
+if (ofd)
+   (void) closeFile();
 return rc;
 }
 
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 4/5] Extend header size to 64MB due to file signatures

2016-04-25 Thread Stefan Berger

From: Stefan Berger <stef...@us.ibm.com>

Extend the header size to 64MB in case an RPM has a lot of files
and the file signatures do not fit within the current limit of 16MB.

An example for an RPM with many files is kcbench-data-4.0. It contains
more than 52000 files. With each signature with a 2048 bit key requiring
256 bytes plus a preamble, its representation in text from, and other
overhead, the size of the header (index length and data length) exceeds
32Mb.

If this RPM's files have been signed using this patch, older versions
of the rpm tool will report the header being too large. So this
failure is expected then.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 lib/header.c  | 2 +-
 lib/header_internal.h | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/header.c b/lib/header.c
index 81f2038..ae292f9 100644
--- a/lib/header.c
+++ b/lib/header.c
@@ -99,7 +99,7 @@ struct headerToken_s {
 /** \ingroup header
  * Maximum no. of bytes permitted in a header.
  */
-static const size_t headerMaxbytes = (32*1024*1024);
+static const size_t headerMaxbytes = (64*1024*1024);
 
 #defineINDEX_MALLOC_SIZE   8
 
diff --git a/lib/header_internal.h b/lib/header_internal.h
index bbe2097..410ad58 100644
--- a/lib/header_internal.h
+++ b/lib/header_internal.h
@@ -45,9 +45,10 @@ struct indexEntry_s {
 
 /**
  * Sanity check on data size and/or offset and/or count.
- * This check imposes a limit of 16 MB, more than enough.
+ * This check imposes a limit of 64 MB -- file signatures
+ * may require a lot of space in the header.
  */
-#define HEADER_DATA_MAX 0x00ff
+#define HEADER_DATA_MAX 0x03ff
 #define hdrchkData(_nbytes) ((_nbytes) & (~HEADER_DATA_MAX))
 
 /**
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 2/5] Fix various memory leaks in file signature related functions.

2016-04-25 Thread Stefan Berger

From: Stefan Berger <stef...@us.ibm.com>

Fix various memory leaks in file signature related functions.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 lib/rpmsignfiles.c |  2 ++
 rpmsign.c  |  4 +++-
 sign/rpmgensig.c   | 24 +---
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/lib/rpmsignfiles.c b/lib/rpmsignfiles.c
index 95ac851..b7d9ccc 100644
--- a/lib/rpmsignfiles.c
+++ b/lib/rpmsignfiles.c
@@ -152,10 +152,12 @@ rpmRC rpmSignFiles(Header h, const char *key, char 
*keypass)
goto exit;
}
if (!headerPutString(h, RPMTAG_FILESIGNATURES, signature)) {
+   free(signature);
rpmlog(RPMLOG_ERR, _("headerPutString failed\n"));
rc = RPMRC_FAIL;
goto exit;
}
+   free(signature);
 }
 
 exit:
diff --git a/rpmsign.c b/rpmsign.c
index a61981a..ddbc5c5 100644
--- a/rpmsign.c
+++ b/rpmsign.c
@@ -60,6 +60,7 @@ static int doSign(poptContext optCon)
 char * passPhrase = NULL;
 char * name = rpmExpand("%{?_gpg_name}", NULL);
 struct rpmSignArgs sig = {NULL, 0, 0};
+char *key = NULL;
 
 if (rstreq(name, "")) {
fprintf(stderr, _("You must set \"%%_gpg_name\" in your macro file\n"));
@@ -71,7 +72,7 @@ static int doSign(poptContext optCon)
 }
 
 if (signfiles) {
-   const char *key = rpmExpand("%{?_file_signing_key}", NULL);
+   key = rpmExpand("%{?_file_signing_key}", NULL);
if (rstreq(key, "")) {
fprintf(stderr, _("You must set \"$$_file_signing_key\" in your 
macro file or on the command line with --fskpath\n"));
goto exit;
@@ -102,6 +103,7 @@ static int doSign(poptContext optCon)
 }
 
 exit:
+free(key);
 free(passPhrase);
 free(name);
 return rc;
diff --git a/sign/rpmgensig.c b/sign/rpmgensig.c
index 77b6d5a..0b23db3 100644
--- a/sign/rpmgensig.c
+++ b/sign/rpmgensig.c
@@ -576,8 +576,10 @@ static rpmRC includeFileSignatures(FD_t fd, const char 
*rpm,
 key = rpmExpand("%{?_file_signing_key}", NULL);
 
 keypass = rpmExpand("%{_file_signing_key_password}", NULL);
-if (rstreq(keypass, ""))
+if (rstreq(keypass, "")) {
+   free(keypass);
keypass = NULL;
+}
 
 rc = rpmSignFiles(*hdrp, key, keypass);
 if (rc != RPMRC_OK) {
@@ -641,11 +643,15 @@ static rpmRC includeFileSignatures(FD_t fd, const char 
*rpm,
 sigTargetSize = Ftell(fd) - headerStart;
 fdFiniDigest(fd, PGPHASHALGO_MD5, (void **), , 0);
 
-if (headerGet(*sigp, RPMSIGTAG_MD5, , HEADERGET_DEFAULT))
+if (headerGet(*sigp, RPMSIGTAG_MD5, , HEADERGET_DEFAULT)) {
memcpy(o_md5, osigtd.data, 16);
+   rpmtdFreeData();
+}
 
-if (headerGet(*sigp, RPMSIGTAG_SHA1, , HEADERGET_DEFAULT))
+if (headerGet(*sigp, RPMSIGTAG_SHA1, , HEADERGET_DEFAULT)) {
o_sha1 = xstrdup(osigtd.data);
+   rpmtdFreeData();
+}
 
 if (memcmp(MD5, o_md5, md5len) == 0 && strcmp(SHA1, o_sha1) == 0)
rpmlog(RPMLOG_WARNING,
@@ -655,6 +661,12 @@ static rpmRC includeFileSignatures(FD_t fd, const char 
*rpm,
replaceSigDigests(fd, rpm, sigp, sigStart, sigTargetSize, SHA1, MD5);
 
 exit:
+free(trpm);
+free(MD5);
+free(SHA1);
+free(o_sha1);
+free(keypass);
+free(key);
 if (ofd)
(void) closeFile();
 return rc;
@@ -675,7 +687,7 @@ static int rpmSign(const char *rpm, int deleting, int 
signfiles)
 char *trpm = NULL;
 Header sigh = NULL;
 Header h = NULL;
-char * msg = NULL;
+char *msg = NULL;
 int res = -1; /* assume failure */
 rpmRC rc;
 struct rpmtd_s utd;
@@ -693,7 +705,6 @@ static int rpmSign(const char *rpm, int deleting, int 
signfiles)
 
 if ((rc = rpmLeadRead(fd, , NULL, )) != RPMRC_OK) {
rpmlog(RPMLOG_ERR, "%s: %s\n", rpm, msg);
-   free(msg);
goto exit;
 }
 
@@ -702,14 +713,12 @@ static int rpmSign(const char *rpm, int deleting, int 
signfiles)
 if (rc != RPMRC_OK) {
rpmlog(RPMLOG_ERR, _("%s: rpmReadSignature failed: %s"), rpm,
(msg && *msg ? msg : "\n"));
-   msg = _free(msg);
goto exit;
 }
 
 headerStart = Ftell(fd);
 if (rpmReadHeader(NULL, fd, , ) != RPMRC_OK) {
rpmlog(RPMLOG_ERR, _("%s: headerRead failed: %s\n"), rpm, msg);
-   msg = _free(msg);
goto exit;
 }
 
@@ -845,6 +854,7 @@ exit:
 rpmFreeSignature(sigh);
 headerFree(h);
 rpmLeadFree(lead);
+free(msg);
 
 /* Clean up intermediate target */
 if (trpm) {
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 3/5] Check range of algo index parameter before accessing array with it

2016-04-25 Thread Stefan Berger

From: Stefan Berger <stef...@us.ibm.com>

Check the range of the algo index parameter before using it for
accessing an array.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 lib/rpmsignfiles.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/lib/rpmsignfiles.c b/lib/rpmsignfiles.c
index b7d9ccc..97a5be4 100644
--- a/lib/rpmsignfiles.c
+++ b/lib/rpmsignfiles.c
@@ -32,6 +32,8 @@ static const char *hash_algo_name[] = {
 [PGPHASHALGO_SHA224]   = "sha224",
 };
 
+#define ARRAY_SIZE(a)  (sizeof(a) / sizeof(a[0]))
+
 char *get_fskpass(void)
 {
 struct termios flags, tmp_flags;
@@ -130,6 +132,10 @@ rpmRC rpmSignFiles(Header h, const char *key, char 
*keypass)
rpmlog(RPMLOG_ERR, _("missing RPMTAG_FILEDIGESTALGO\n"));
return RPMRC_FAIL;
 }
+if (algo < 0 || algo >= ARRAY_SIZE(hash_algo_name)) {
+   rpmlog(RPMLOG_ERR, _("File digest algorithm id is invalid"));
+   return RPMRC_FAIL;
+}
 
 diglen = rpmDigestLength(algo);
 algoname = hash_algo_name[algo];
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 0/5] Fix issues related to signed files

2016-04-25 Thread Stefan Berger

This series of patches fixes several issues related to signed files
produced by rpmsign.

  Stefan

Stefan Berger (5):
  Fix indentation and formatting
  Fix various memory leaks in file signature related functions.
  Check range of algo index parameter before accessing array with it
  Extend header size to 64MB due to file signatures
  Fix handling of zero-length file digests

 lib/header.c  |  2 +-
 lib/header_internal.h |  5 +++--
 lib/rpmsignfiles.c| 24 ++--
 plugins/ima.c | 36 +++-
 rpmsign.c |  4 +++-
 sign/rpmgensig.c  | 27 +++
 6 files changed, 79 insertions(+), 19 deletions(-)

-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

[Rpm-maint] [PATCH 5/5] Fix handling of zero-length file digests

2016-04-25 Thread Stefan Berger

From: Stefan Berger <stef...@us.ibm.com>

Do not try to convert a zero-length file digest to a binary representation.
Zero-length file digests may stem from directory entries and symbolic links.
Return an empty signature in this case.

Returning an empty signature results in the ima.so plugin getting a sequence
of zeroes that it would write into security.ima xattr. Check for a signature
consisting of only zeroes and do not write it into the filesystem.

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
 lib/rpmsignfiles.c |  4 
 plugins/ima.c  | 36 +++-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/lib/rpmsignfiles.c b/lib/rpmsignfiles.c
index 97a5be4..3cd2b1a 100644
--- a/lib/rpmsignfiles.c
+++ b/lib/rpmsignfiles.c
@@ -82,6 +82,10 @@ const char *key, char *keypass)
 
 /* convert file digest hex to binary */
 memset(digest, 0, diglen);
+/* some entries don't have a digest - we return an empty signature */
+if (strlen(fdigest) != diglen * 2)
+return strdup("");
+
 for (int i = 0; i < diglen; ++i, fdigest += 2)
digest[i] = (rnibble(fdigest[0]) << 4) | rnibble(fdigest[1]);
 
diff --git a/plugins/ima.c b/plugins/ima.c
index 0dfdd8b..2b998d0 100644
--- a/plugins/ima.c
+++ b/plugins/ima.c
@@ -12,6 +12,40 @@
 
 #define XATTR_NAME_IMA "security.ima"
 
+/* security.ima signature header */
+struct signature_v2_hdr {
+   uint8_t type;
+   uint8_t version;
+   uint8_t hash_algo;
+   uint32_t keyid;
+   uint16_t sig_size;
+   uint8_t sig[0];
+} __attribute__((PACKED));
+
+static const struct signature_v2_hdr zero_hdr = {
+   .type = 0,
+   .version = 0,
+   .hash_algo = 0,
+   .keyid = 0,
+   .sig_size = 0,
+};
+
+/*
+ * check_zero_hdr: Check the signature for a zero header
+ *
+ * Check whether the given signature has a header with all zeros
+ *
+ * Returns -1 in case the signature is too short to compare
+ * (invalid signature), 0 in case the header is not only zeroes,
+ * and 1 if it is only zeroes.
+ */
+static int check_zero_hdr(const unsigned char *fsig, size_t siglen)
+{
+   if (siglen < sizeof(zero_hdr))
+   return -1;
+   return (memcmp(fsig, _hdr, sizeof(zero_hdr)) == 0);
+}
+
 static rpmRC ima_psm_post(rpmPlugin plugin, rpmte te, int res)
 {
rpmfi fi = rpmteFI(te);
@@ -30,7 +64,7 @@ static rpmRC ima_psm_post(rpmPlugin plugin, rpmte te, int res)
if (!(rpmfiFFlags(fi) & RPMFILE_CONFIG)) {
fpath = rpmfiFN(fi);
fsig = rpmfiFSignature(fi, );
-   if (fsig) {
+   if (fsig && (check_zero_hdr(fsig, len) == 0)) {
lsetxattr(fpath, XATTR_NAME_IMA, fsig, len, 0);
}
}
-- 
2.5.5

___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

58 matches

Mail list logo