[Bug debuginfod/28204] New: extend webapi / verification with forthcoming signed-contents archives

[Bug debuginfod/28204] New: extend webapi / verification with forthcoming signed-contents archives

[fche at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

            Bug ID: 28204
           Summary: extend webapi / verification with forthcoming
                    signed-contents archives
           Product: elfutils
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: debuginfod
          Assignee: unassigned at sourceware dot org
          Reporter: fche at redhat dot com
                CC: elfutils-devel at sourceware dot org
  Target Milestone: ---

Efforts such as https://fedoraproject.org/wiki/Changes/Signed_RPM_Contents
might look stalled, but some distro builds are experimenting with this stuff
already.   We may soon avail ourselves of RPMs that carry per-file IMA
signatures.  If so, we should extract those signature bits and pass them back
to debuginfod clients.  They may be able to offline verify the integrity of the
download, so as to not trust the debuginfod server.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[fche at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #1 from Frank Ch. Eigler <fche at redhat dot com> ---
https://bugzilla.redhat.com/show_bug.cgi?id=1896046#c10 (private, sorry)
contains instructions on generating IMA-signed RPMs via

% rpm --addsign --signfiles --fskpath=FOO.pem foo.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mark at klomp dot org

--- Comment #2 from Mark Wielaard <mark at klomp dot org> ---
OpenSuse has some documentation IMA here:
https://en.opensuse.org/SDB:Ima_evm#Introduction

Gentoo:
https://wiki.gentoo.org/wiki/Integrity_Measurement_Architecture

How would this be used together with debuginfod? Where/how would the user get
the signatures and how would the client library check those signatures?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[fche at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #3 from Frank Ch. Eigler <fche at redhat dot com> ---
> How would this be used together with debuginfod?
> Where/how would the user get the signatures

From debuginfod, possibly via additional response headers,
when extracting files from IMA-signed archives.

> and how would the client library check those signatures?

If it has the file and the signature (and the appropriate
public key!) available, the debuginfod client can perform
this integrity check at download time.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[fche at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #4 from Frank Ch. Eigler <fche at redhat dot com> ---
some more references

https://fedoraproject.org/wiki/Changes/Signed_RPM_Contents,              

https://man.archlinux.org/man/rpmsign.8.en                                      

rpmfiFSignature(), lib/rpmfi.c

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[rgoldber at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

Ryan Goldberg <rgoldber at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rgoldber at redhat dot com

--- Comment #5 from Ryan Goldberg <rgoldber at redhat dot com> ---
Created attachment 14208
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14208&action=edit
Submit A Patch for 28204

At the moment there are no public x509 verification certificates available to
validate the signatures generated for RHEL 9. There is discussion at
https://issues.redhat.com/browse/SIGNSERVER-245 for the future location and in
fedora 37 the cert will be available on the fedora security site,
https://getfedora.org/security/

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[fche at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

Frank Ch. Eigler <fche at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at sourceware dot org   |rgoldber at redhat dot com

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #6 from Mark Wielaard <mark at klomp dot org> ---
Still reviewing patch, but we seem to also need a configure check to make sure
we have a new enough rpm to recognize the rhel9 rpm, otherwise some tests will
fail with:

+ rpm2cpio ../../R/debuginfod-rpms/rhel9/hello2-1.0-1.x86_64.rpm
+ cpio -ivd
argument is not an RPM package
cpio: premature end of archive

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[rgoldber at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

Ryan Goldberg <rgoldber at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #14208|0                           |1
        is obsolete|                            |

--- Comment #7 from Ryan Goldberg <rgoldber at redhat dot com> ---
Created attachment 14256
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14256&action=edit
Submit A Patch for 28204, corrections made

The following patch fixes the previously mentioned issues. It passes all the
tests across the try buildbots (users/rgoldber/try-bz28204)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #8 from Mark Wielaard <mark at klomp dot org> ---
Note that we'll need rpm-devel ima-evm-utils-devel openssl-devel rpm-sign to
test this (should go into elfutils.spec BuildRequires).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #9 from Mark Wielaard <mark at klomp dot org> ---
To make this slightly easier to review I applied the patch and rebased it to
current master. Pushed here:
https://code.wildebeest.org/git/user/mjw/elfutils/commit/?h=ima

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[rgoldber at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

Ryan Goldberg <rgoldber at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #10 from Ryan Goldberg <rgoldber at redhat dot com> ---
Hi, it has been quite the journey, but the latest draft of this patch is ready
for review

It is sitting on the try-branch users/rgoldber/try-bz28204c (figure'd it was
big enough that just looking at a patch might be hard to follow)

Since the last review the major changes are as follows
* I added a --koji-sigcache flag to the server which will enable koji specific
mappings of rpm paths to get IMA signatures. 
* DEBUGINFOD_IMA_CERT_PATH can now include paths to dirs containing PEM and DER
encoded certificates. And will be traversed looking for the first cert which
has a skid matching the signature which we need to validate.
* The verification certificates for RHEL and CentOS have been finalized and we
have a green light to distribute copies of them alongside our source (since
they have not been formally published to a known location yet). They are in
debuginfod/ima-certs and will be installed to
$(sysconfdir)/debuginfod/ima-certs. DEBUGINFOD_IMA_CERT_PATH will by default
include this path. This dir also has copies of the current fedora verification
certs (which are already public but not yet backported to f38 [fedora-repos
commit 93b2c8a])

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #11 from Mark Wielaard <mark at klomp dot org> ---
So as far as I understand this is now the three commits on top of
users/rgoldber/try-bz28204c:
https://sourceware.org/cgit/elfutils/log/?h=users/rgoldber/try-bz28204c

I find it slightly easier when patches are simply posted to the mailinglist
(using git send-email).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #12 from Mark Wielaard <mark at klomp dot org> ---
In config/profile.csh.in and config/profile.sh.in the prefix variable is
explicitly set and no longer unset. Is that deliberate?

In debuginfod_validate_imasig the file_data = malloc(data_len); depends on the
(externally) given file size. It is then read in one pread call. And the whole
buffer is then given to EVP_DigestUpdate. Note that this might create a giant
malloc buffer, which might trigger OOM. pread might succeed with fewer bytes
than given. It needs to be called in a loop. But it would be better if we could
read it and feed it to EVP_DigestUpdate in (small) chunks.

Is EACCESS the right error code to return when the signature couldn't be
checked/is invalid? That is the same as when we get CURLE_REMOTE_ACCESS_DENIED.
It might be good if it was an unique error code so users can know that the file
was not trusted.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #13 from Mark Wielaard <mark at klomp dot org> ---
The configure checks might need to check whether the rpm development headers
define the needed constants.

On an rhel8 system this gives:

checking for headerGet in -lrpm... yes
checking for imaevm_hash_algo_from_sig in -limaevm... yes
checking for EVP_MD_CTX_new in -lcrypto... yes

but...

Making all in debuginfod
  CXX      debuginfod.o
elfutils/debuginfod/debuginfod.cxx: In function ‘MHD_Response*
handle_buildid_r_match(bool, int64_t, const string&, const string&, const
string&, int*)’:
elfutils/debuginfod/debuginfod.cxx:2026:47: error: ‘RPMSIGTAG_FILESIGNATURES’
was not declared in this scope
       if (!sig_hdr || 1 != headerGet(sig_hdr, RPMSIGTAG_FILESIGNATURES,
&sig_tag_data, HEADERGET_ALLOC))
                                               ^~~~~~~~~~~~~~~~~~~~~~~~
elfutils/debuginfod/debuginfod.cxx:2026:47: note: suggested alternative:
‘RPMTAG_FILESIGNATURES’
       if (!sig_hdr || 1 != headerGet(sig_hdr, RPMSIGTAG_FILESIGNATURES,
&sig_tag_data, HEADERGET_ALLOC))
                                               ^~~~~~~~~~~~~~~~~~~~~~~~
                                               RPMTAG_FILESIGNATURES
make[2]: *** [Makefile:657: debuginfod.o] Error 1

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #14 from Mark Wielaard <mark at klomp dot org> ---
I think it is the user/distro packager who should decide which ima-certs to
ship. I don't think elfutils should come with ima-certs itself.

Why is there a "permissive" policy? What is the use case for this?

Should the policy be per debuginfod url? So you can point to an official distro
debuginfod which must be in enforcing mode, but can add a local one with an
"ignore" policy.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #15 from Mark Wielaard <mark at klomp dot org> ---
The basic idea having a collection of certs, and an signature for each file
that is transported with the fetch operation that will be checked against those
trusted certs is understandable.

But I must admit that I am a little lost in the rpm and koji mechanisms to
extract those signatures. How easy will it be to extend to other platforms that
might store such signatures in different ways?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[rgoldber at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #16 from Ryan Goldberg <rgoldber at redhat dot com> ---
(In reply to Mark Wielaard from comment #12)
> In config/profile.csh.in and config/profile.sh.in the prefix variable is
> explicitly set and no longer unset. Is that deliberate?
Taking a look at both files they seem to still contain `unset prefix` as their
last lines. Would that not do the trick?

> In debuginfod_validate_imasig the file_data = malloc(data_len); depends on
> the (externally) given file size. It is then read in one pread call. And the
> whole buffer is then given to EVP_DigestUpdate. Note that this might create
> a giant malloc buffer, which might trigger OOM. pread might succeed with
> fewer bytes than given. It needs to be called in a loop. But it would be
> better if we could read it and feed it to EVP_DigestUpdate in (small) chunks.
Sure, just looked at the docs for EVP_DigestUpdate and seems like this change
should be pretty straightforward.

> Is EACCESS the right error code to return when the signature couldn't be
> checked/is invalid? That is the same as when we get
> CURLE_REMOTE_ACCESS_DENIED. It might be good if it was an unique error code
> so users can know that the file was not trusted.
How do you feel about EPERM? Not exactly its usage but we've used up a good
number of the more applicable ones with the CURL errors (EACCESS, EINVAL)

(In reply to Mark Wielaard from comment #13)
> The configure checks might need to check whether the rpm development headers
> define the needed constants.
Sure

(In reply to Mark Wielaard from comment #14)
> I think it is the user/distro packager who should decide which ima-certs to
> ship. I don't think elfutils should come with ima-certs itself.
From what I've seen finding the correct certificates for IMA verification has
been pretty tricky, so distributing a copy of these (at least early
certificates) might be a good idea. Eventually the intention is for these certs
to be shipped in a known location which we can easily add to the search path,
but until then having a copy seems like the best bet imho.

> Why is there a "permissive" policy? What is the use case for this?
It might be that a user wants to be aware of what's going on but does not need
to be quite so strict on rejection. Permissive won't allow incorrect signatures
but will allow say an unsigned file whereas an enforcing client will reject
anything that does not come with a valid signature

> Should the policy be per debuginfod url? So you can point to an official
> distro debuginfod which must be in enforcing mode, but can add a local one
> with an "ignore" policy.
I was thinking of keeping things simple, but don't have anything against moving
to a per URL sort of approach. What kind of format would you want for such a
thing? For urls foo:bar:baz would we want something like ignore::enforcing
where blanks are the default? Don't love this structure since it seems a little
unwieldy. I'll think on a better format for it.

(In reply to Mark Wielaard from comment #15)
> But I must admit that I am a little lost in the rpm and koji mechanisms to
> extract those signatures. How easy will it be to extend to other platforms
> that might store such signatures in different ways?
The koji flag seems like a necessary evil in my opinion. At least the way it
works is that when the flag is enabled we try the koji storage method and then
fall back to the rpm itself, so to add another signature storage format we
could do the same method, so adding more shouldn't be too difficult. Might
require a little logic tweak but wouldn't think its too complex a change

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #17 from Mark Wielaard <mark at klomp dot org> ---
(In reply to Ryan Goldberg from comment #16)
> (In reply to Mark Wielaard from comment #12)
> > In config/profile.csh.in and config/profile.sh.in the prefix variable is
> > explicitly set and no longer unset. Is that deliberate?
> Taking a look at both files they seem to still contain `unset prefix` as
> their last lines. Would that not do the trick?

Yes it would. I was just confused why it was needed.
But I now see that @sysconfdir@ is used and that uses $prefix.
So that explains why you need it defined.

> > In debuginfod_validate_imasig the file_data = malloc(data_len); depends on
> > the (externally) given file size. It is then read in one pread call. And the
> > whole buffer is then given to EVP_DigestUpdate. Note that this might create
> > a giant malloc buffer, which might trigger OOM. pread might succeed with
> > fewer bytes than given. It needs to be called in a loop. But it would be
> > better if we could read it and feed it to EVP_DigestUpdate in (small) chunks.
> Sure, just looked at the docs for EVP_DigestUpdate and seems like this
> change should be pretty straightforward.
> 
> > Is EACCESS the right error code to return when the signature couldn't be
> > checked/is invalid? That is the same as when we get
> > CURLE_REMOTE_ACCESS_DENIED. It might be good if it was an unique error code
> > so users can know that the file was not trusted.
> How do you feel about EPERM? Not exactly its usage but we've used up a good
> number of the more applicable ones with the CURL errors (EACCESS, EINVAL)

Yes, EPERM sounds like a good pick. It is unlikely to be produced by any other
operation (unless there are file seals in play).

> (In reply to Mark Wielaard from comment #14)
> > I think it is the user/distro packager who should decide which ima-certs to
> > ship. I don't think elfutils should come with ima-certs itself.
> From what I've seen finding the correct certificates for IMA verification
> has been pretty tricky, so distributing a copy of these (at least early
> certificates) might be a good idea. Eventually the intention is for these
> certs to be shipped in a known location which we can easily add to the
> search path, but until then having a copy seems like the best bet imho.
> 
> > Why is there a "permissive" policy? What is the use case for this?
> It might be that a user wants to be aware of what's going on but does not
> need to be quite so strict on rejection. Permissive won't allow incorrect
> signatures but will allow say an unsigned file whereas an enforcing client
> will reject anything that does not come with a valid signature

Doesn't that give a false sense of "security"?
It still rejects some stuff, but doesn't really protect against "falsifying"
files, all a server has to do is not provide an IMA 

If it is just to see what would happen if enabling ima file checking, then it
probably shouldn't reject anything. In that case it should warn for both
missing and invalid signatures, but still accept them.

But in general I like less "modes".

> > Should the policy be per debuginfod url? So you can point to an official
> > distro debuginfod which must be in enforcing mode, but can add a local one
> > with an "ignore" policy.
> I was thinking of keeping things simple, but don't have anything against
> moving to a per URL sort of approach. What kind of format would you want for
> such a thing? For urls foo:bar:baz would we want something like
> ignore::enforcing where blanks are the default? Don't love this structure
> since it seems a little unwieldy. I'll think on a better format for it.

Maybe prefixing or postfixing URLS with + or adding the name of the cert?
DEBUGINFOD_URLS="https://debuginfod.fedoraproject.org/+FEDORACERTNAME" ?

Yeah, gets ugly quick :{
But I think a common use case will be having the main distro debuginfod server
in enforcing mode and your local/org debuginfod server in trusting mode.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[fche at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #18 from Frank Ch. Eigler <fche at redhat dot com> ---
> Doesn't that give a false sense of "security"?
> It still rejects some stuff, but doesn't really protect against "falsifying"
> files, all a server has to do is not provide an IMA 

Yes, but trusted servers won't just do that.

> If it is just to see what would happen if enabling ima file checking, then
> it probably shouldn't reject anything. In that case it should warn for both
> missing and invalid signatures, but still accept them.

The difference between missing and invalid is that the latter is KNOWN bad.
An invalid signature is evidence that the file has a problem.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[rgoldber at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #19 from Ryan Goldberg <rgoldber at redhat dot com> ---
(In reply to Mark Wielaard from comment #17)
> Maybe prefixing or postfixing URLS with + or adding the name of the cert?
I'm leaning towards a combination of this idea with my original. We can use
DEBUGINFOD_IMA_POLICY to set the default ima verification response policy
(possibly renaming to DEBUGINFOD_IMA_DEFAULT_POLICY) and then we can use
DEBUGINFOD_URLS="url1+enforcing url2 url3+ignore". This seems like a simple way
the user can define policy at a granularity of their choice. I wrote up a quick
test patch for this and it seems pretty straightforward.

(In reply to Frank Ch. Eigler from comment #18)
> > Doesn't that give a false sense of "security"?
> > It still rejects some stuff, but doesn't really protect against "falsifying"
> > files, all a server has to do is not provide an IMA 
> 
> Yes, but trusted servers won't just do that.
It's up to the user to choose when to allow a permissive policy. Maybe not
using it as the default will alleviate these concerns? A user would have to
explicitly choose to 'let their guard down' when for instance using a trusted
server

> The difference between missing and invalid is that the latter is KNOWN bad.
> An invalid signature is evidence that the file has a problem.
I agree that the distinction is enough that having a 3rd mode seems like a good
bet. When using https://debuginfod.fedoraproject.org/ it doesn't feel like an
issue to me to use a file that doesn't happen to have a signature, but otoh I
wouldn't want one that I know is incorrect

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #20 from Mark Wielaard <mark at klomp dot org> ---
(In reply to Frank Ch. Eigler from comment #18)
> > Doesn't that give a false sense of "security"?
> > It still rejects some stuff, but doesn't really protect against "falsifying"
> > files, all a server has to do is not provide an IMA 
> 
> Yes, but trusted servers won't just do that.

But isn't the idea of checking the IMA signatures that you don't have to trust
the server providing the debuginfo files as the distro intended them?

> > If it is just to see what would happen if enabling ima file checking, then
> > it probably shouldn't reject anything. In that case it should warn for both
> > missing and invalid signatures, but still accept them.
> 
> The difference between missing and invalid is that the latter is KNOWN bad.
> An invalid signature is evidence that the file has a problem.

And a missing signature is UNKNOWN bad?

So both are bad in some way. Which imho means that if we support some kind of
permissive mode, then it should explicitly warn for both kind of baddness.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[rgoldber at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #21 from Ryan Goldberg <rgoldber at redhat dot com> ---
(In reply to Mark Wielaard from comment #20)
> But isn't the idea of checking the IMA signatures that you don't have to
> trust the server providing the debuginfo files as the distro intended them?
But this will allow for the case of a trusted server which only has some of
it's RPMs per-file signed. Take for instance a server which has the RPMs for
f36,37,38. The f36 files don't have signatures so using enforcing here is too
strict since we are ok just letting a client know that these ones are
unverifiable, but we still want to be able to reject any of the invalid ones
for f38

> So both are bad in some way. Which imho means that if we support some kind
> of permissive mode, then it should explicitly warn for both kind of baddness.
In the permissive mode you'll get:
* "the signature is valid" for valid sigs
* "ALERT: this download is being rejected since the IMA signature could not be
verified" for invalid sigs
* "the signature could not be verified" otherwise

So we do warn for both kinds of bad, we just don't reject the 'unknown' bad

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #22 from Mark Wielaard <mark at klomp dot org> ---
(In reply to Ryan Goldberg from comment #21)
> (In reply to Mark Wielaard from comment #20)
> > But isn't the idea of checking the IMA signatures that you don't have to
> > trust the server providing the debuginfo files as the distro intended them?
> But this will allow for the case of a trusted server which only has some of
> it's RPMs per-file signed. Take for instance a server which has the RPMs for
> f36,37,38. The f36 files don't have signatures so using enforcing here is
> too strict since we are ok just letting a client know that these ones are
> unverifiable, but we still want to be able to reject any of the invalid ones
> for f38

This still feels odd. Since you cannot distinguish between non-sig f36 package
(okish?) and non-sig f38 packages (bad?). I think you have to either trust or
reject all non-sig packages from such a server.

> > So both are bad in some way. Which imho means that if we support some kind
> > of permissive mode, then it should explicitly warn for both kind of baddness.
> In the permissive mode you'll get:
> * "the signature is valid" for valid sigs
> * "ALERT: this download is being rejected since the IMA signature could not
> be verified" for invalid sigs
> * "the signature could not be verified" otherwise

I'll look at the code to see if I understand what this means in practice. Are
those the messages presented to the user (in verbose mode? always?). And does
this mean all three cases warn (or only the second and third)? And is it just a
message or does it also mean actual rejection in some cases?

> So we do warn for both kinds of bad, we just don't reject the 'unknown' bad

But how is 'unknown' bad different from invalid signature bad?
I think you should assume that if there is no signature, then the signature is
by definition invalid (assume it has a signature of
00000000000000000000000000000000).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[rgoldber at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #23 from Ryan Goldberg <rgoldber at redhat dot com> ---
(In reply to Mark Wielaard from comment #22)
> This still feels odd. Since you cannot distinguish between non-sig f36
> package (okish?) and non-sig f38 packages (bad?). I think you have to either
> trust or reject all non-sig packages from such a server.
I see where you're coming from but what policy would you use for
https://debuginfod.fedoraproject.org? We'd have to ignore then, which seems
like one of the primary users of such verification might just skip it all
together.

> I'll look at the code to see if I understand what this means in practice.
> Are those the messages presented to the user (in verbose mode? always?). And
> does this mean all three cases warn (or only the second and third)? And is
> it just a message or does it also mean actual rejection in some cases?
2008   if(NULL != url_ima_policies && ignore != url_ima_policies[committed_to])
2009   {
2010     int result = debuginfod_validate_imasig(c, target_cache_tmppath, fd);
2011     if(0 == result)
2012     {
2013       if (vfd >= 0) dprintf (vfd, "the signature is valid\n");
2014     }
2015     else if(EINVAL == result || enforcing ==
url_ima_policies[committed_to])
2016     {
2017       // All invalid signatures are rejected.
2018       // Additionally in enforcing mode any non-valid signature is
rejected, so by reaching
2019       // this case we do so since we know it is not valid. Note - this not
just invalid signatures
2020       // but also signatures that cannot be validated
2021       if (vfd >= 0) dprintf (vfd, "ALERT: this download is being rejected
since the IMA signature could not be verified\n");
2022       rc = -EPERM;
2023       goto out2;
2024     }
2025     else
2026     {
2027       // By default we are permissive, so since the signature isn't
invalid we
2028       // give it the benefit of the doubt
2029       if (vfd >= 0) dprintf (vfd, "the signature could not be
verified\n");
2030     }
2031   }
2032 

Here is the relevant snippet

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #24 from Mark Wielaard <mark at klomp dot org> ---
BTW. How does this interact with the "section" queries?

If the server doesn't support "section" then the debuginfod-client fallback to
fetching "debuginfo" or "executable" should do the signature checking.

But there doesn't seem to be a way to make (client side) signature checking
work when fetching "section". Does this mean that in enforcing mode we always
need to use the fallback mode?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #25 from Mark Wielaard <mark at klomp dot org> ---
So I am looking at users/rgoldber/try-bz28204d but it isn't clear you want to
merge that in separate commits or squashed together. I am comparing to
users/rgoldber/try-bz28204c which I believe is the previous version reviewed.

It really makes things a lot easier if the actual patches that are intended to
be merged are posted (with a description of what was changed since the last
review).

So as far as I can tell the new series has been rebased from commit
35e059b654224b1a01d05877b13582c74c692388 to
27a84961f7a061b83f10f7e02bf433c229d6537a. Good. That is just 3 commits.

- configure.ac checks updated.
- debuginfod/debuginfod-client.c introduces ima_policy_t
  Includes an "undefined" policy?
  debuginfod_validate_imasig updated to read/digest in chunks of DATA_SIZE.
  Is the k += DATA_SIZE correct? Can't pread return an n < DATA_SIZE?
  If the cert_paths = strdup (...) fails cert_paths gets assigned a static
string?
  Won't that crash the strtok calls or the free (cert_path) call?
  In debuginfod_query_server the server_urls are parsed to see te ima policy,
  as described debuginfod-client-config.7

Sorry, have to stop for a bit. Will try to look at the rest later.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #26 from Mark Wielaard <mark at klomp dot org> ---
(In reply to Ryan Goldberg from comment #23)
> (In reply to Mark Wielaard from comment #22)
> > This still feels odd. Since you cannot distinguish between non-sig f36
> > package (okish?) and non-sig f38 packages (bad?). I think you have to either
> > trust or reject all non-sig packages from such a server.
> I see where you're coming from but what policy would you use for
> https://debuginfod.fedoraproject.org?

I think that depends on which release I am using.
Normally I would be debugging/profiling local programs/libraries.
So for f38 I would like to a policy that checks the ima signatures.

If I am on f36 or if e.g. I expect to be inspecting a non-local core file based
on f36 then I would not use a policy that checks the ima signatures, because
they might not be there.

> > I'll look at the code to see if I understand what this means in practice.
> > Are those the messages presented to the user (in verbose mode? always?). And
> > does this mean all three cases warn (or only the second and third)? And is
> > it just a message or does it also mean actual rejection in some cases?
>
> 2008   if(NULL != url_ima_policies && ignore != url_ima_policies[committed_to])
> 2009   {
> 2010     int result = debuginfod_validate_imasig(c, target_cache_tmppath, fd);
> 2011     if(0 == result)
> 2012     {
> 2013       if (vfd >= 0) dprintf (vfd, "the signature is valid\n");
> 2014     }
> 2015     else if(EINVAL == result || enforcing == url_ima_policies[committed_to])
> 2016     {
> 2017       // All invalid signatures are rejected.
> 2018       // Additionally in enforcing mode any non-valid signature is rejected, so by reaching
> 2019       // this case we do so since we know it is not valid. Note - this not just invalid signatures
> 2020       // but also signatures that cannot be validated
> 2021       if (vfd >= 0) dprintf (vfd, "ALERT: this download is being rejected since the IMA signature could not be verified\n");
> 2022       rc = -EPERM;
> 2023       goto out2;
> 2024     }
> 2025     else
> 2026     {
> 2027       // By default we are permissive, so since the signature isn't invalid we
> 2028       // give it the benefit of the doubt
> 2029       if (vfd >= 0) dprintf (vfd, "the signature could not be verified\n");
> 2030     }
> 2031   }
>
> Here is the relevant snippet

Thanks.

So if the policy is "ignore" nothing is checked.
Otherwise the ima signature is checked.
If the policy is enforcing we return EPERM.
Otherwise we return the target/OK, independent of the result of the ima sig
check.
All notices are printed if we are in verbose mode.

BTW. The last notice isn't really accurate.
The signature might have been verified. In which case it was invalid.

In practice "permissive" (or default) mode is no different from "ignore".
Which imho makes permissive mode not very useful.

So I think we shouldn't have a permissive mode, just use ignore.

If we have an permissive mode then I think it should work like the selinux
permissive mode.
That is, it should always check the signature, but instead of failing with
EPERM, it should
always produce a warning (whether or not we are in verbose mode or not).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[rgoldber at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #27 from Ryan Goldberg <rgoldber at redhat dot com> ---
(In reply to Mark Wielaard from comment #24)
> BTW. How does this interact with the "section" queries?
Since these aren't ima verifiable anyways wdyt of just skipping verification
all together (i.e treat that query in the same way as the ignore policy)

Tweaking the above to something like:
2008    if(NULL != url_ima_policies && ignore != url_ima_policies[committed_to]
&& NULL == section) { ... }

(In reply to Mark Wielaard from comment #25)
>   Includes an "undefined" policy?
Just used internally when parsing DEBUGINFOD_URLS
>   Is the k += DATA_SIZE correct? Can't pread return an n < DATA_SIZE?
Fixed
>   If the cert_paths = strdup (...) fails cert_paths gets assigned a static string?
Fixed

(In reply to Mark Wielaard from comment #26)
> If we have an permissive mode then I think it should work like the selinux
> permissive mode.
> That is, it should always check the signature, but instead of failing with
> EPERM, it should
> always produce a warning (whether or not we are in verbose mode or not).
I would be ok with this kind of permissive mode

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives

[mark at klomp dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28204

--- Comment #28 from Mark Wielaard <mark at klomp dot org> ---
(In reply to Ryan Goldberg from comment #27)
> (In reply to Mark Wielaard from comment #24)
> > BTW. How does this interact with the "section" queries?
> Since these aren't ima verifiable anyways wdyt of just skipping verification
> all together (i.e treat that query in the same way as the ignore policy)

Assuming ima verification is so one can be sure bits are what a distro really
distributes I don't think a default to ignore policy is useful. I think in
enforcing mode there is no good way to fetch just the section data and the
client has to fall back onto requesting the full executable or debuginfo file,
verify the ima signature, and then locally extract the section data (of course
this is somewhat inefficient).

> Tweaking the above to something like:
> 2008    if(NULL != url_ima_policies && ignore !=
> url_ima_policies[committed_to] && NULL == section) { ... }
> 
> (In reply to Mark Wielaard from comment #25)
> >   Includes an "undefined" policy?
> Just used internally when parsing DEBUGINFOD_URLS
> >   Is the k += DATA_SIZE correct? Can't pread return an n < DATA_SIZE?
> Fixed
> >   If the cert_paths = strdup (...) fails cert_paths gets assigned a static string?
> Fixed

Thanks. It would be good to post a patch with these fixes (maybe rebased to
current git trunk). So it is more clear what exact change you intent to commit.

> (In reply to Mark Wielaard from comment #26)
> > If we have an permissive mode then I think it should work like the selinux
> > permissive mode.
> > That is, it should always check the signature, but instead of failing with
> > EPERM, it should
> > always produce a warning (whether or not we are in verbose mode or not).
> I would be ok with this kind of permissive mode

This would have my preference too. Or just simply have either an ignore or
enforcing mode.

But from our discussion on irc I think Frank believes it is better to see
permissive mode as a kind of possible checksum mode. I am not sure about that
use case.

Given that we have reliable transports a checksum mode might be something for
the server to do (assuming the server doesn't trust its own storage). The
server could do an ima check before sending any file and simply don't sent it
if the ima check(sum) fails. This then would be independent of the client ima
check (enforced or not).

-- 
You are receiving this mail because:
You are on the CC list for the bug.