[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#980849: apt fails to reject repositories with invalid InRelease file



Hi David,

On Sun, Jan 24, 2021 at 05:17:14PM +0100, David Kalnischkies wrote:
> On Sat, Jan 23, 2021 at 09:35:12AM +0200, Wouter Verhelst wrote:
> > Currently, there are two merge requests open[3] for repositories on
> > which my script fails while trying to verify the InRelease file.
> > 
> > It turns out that these repositories return data for the InRelease file
> > -- i.e., a file that has checksums and is signed by some tool -- but the
> > signature is invalid. The repository also has a Release/Release.gpg
> > pair, where the signature *is* valid.
> 
> Merge request 66 repository results in this 'apt update' output for me:
> | […]
> | Err:3 https://adoptopenjdk.jfrog.io/adoptopenjdk/deb buster InRelease
> |   The following signatures were invalid: ERRSIG 8AC3B29174885C03
> | Reading package lists...
> | W: GPG error: https://adoptopenjdk.jfrog.io/adoptopenjdk/deb buster InRelease: The following signatures were invalid: ERRSIG 8AC3B29174885C03
> | E: The repository 'https://adoptopenjdk.jfrog.io/adoptopenjdk/deb buster InRelease' is not signed.
> | N: Updating from such a repository can't be done securely, and is therefore disabled by default.
> | N: See apt-secure(8) manpage for repository creation and user configuration details.
> 
> So please give us the actual configuration and output apt produces for
> you if you suspect a bug (reportbug should help you with that).

Since I didn't actually enable that repository on my laptop, that
wouldn't work :) But yeah, I suppose I should indeed have tried things
out first. Sorry about that.

Also, it looks like the repository has changed since; when I tried it
before filing this bug, it would validate correctly if I skipped the
InRelease file and only checked the Release/Release.gpg files. Can't
help it if people break their stuff, of course ;-)

(but then perhaps that might have been another case of the same issue as
MR 66)

> Merge request 65 repository is a fun one as apt actually accepts
> the InRelease as valid, even though throwing it directly at gpg does
> indeed indicate a bad signature – gpg is not telling what is bad about
> this signature through: The class of this signature. The signature is
> of the binary class, which clearsigned text is not by definition.
> 
> apt doesn't feed the InRelease file to gpg directly though: It first
> splits the file in two – creating files akin to Release and Release.gpg
> – and passes those to gpg. A signature for a binary file is valid in
> that context – and successfully passes verification by gpg.

Oh, I wasn't aware of that. Thanks for informing me. I guess that means
I need to update my validation tool then :)

> We do this as we work with the Release internally and we want to be sure
> that the data we see is what was signed as we run the risk of either us
> or gpg being tricked into parsing the InRelease differently and hence
> seeing and acting on [unsigned] different data (it happened before).

Makes sense.

[...]
> > Apt should probably produce a warning (if not an error) on such
> > repositories; it currently does not seem to do that.
> 
> We could implement a check for that I presume, but I am not particular
> keen on parsing the different signature packet formats out of the base64
> encoding just so we can complain about something that is technically no
> problem for us even if that is of course not a supported interface but
> a reliance on an implementation detail which could change any minute
> (in theory at least) and is likely a problem for clients with
> a different implementation. That seems more like the job of a linter…
> 
> There are btw reasons a seemingly invalid InRelease file is ignored and
> we continue on to use a Release + Release.gpg pair without producing an
> error or a warning (except an Ign line in update output): Some servers
> e.g. send file not found pages with "200 OK".

In that case, gpgv will report

gpgv: no valid OpenPGP data found
gpgv: the signature could not be verified

which is different from

[GNUPG:] NEWSIG
[GNUPG:] KEY_CONSIDERED ...
[GNUPG:] BADSIG ...

For clarity, what I mean to say is that if the signature exists but is
invalid on the InRelease file (while the Release.gpg file contains a
valid signature) then you have a repository that is signed both
correctly and incorrectly. This is not the same thing as a repository
that is signed correctly and that returns junk instead of some other
possible signature.

If the repository is signed both correctly and incorrectly, then which
of the two is correct? You may have had an attacker who replaced
software in your repository with an older, vulnerable, version of the
same software, but was thrown out before they could complete their
attack (or they messed up).

I'm not saying that apt should always check that both versions of the
Release data exist, but if it downloads something and fails to verify
the signature, it should then not discard that information just because
it so happens that there is also a valid signature on some other form of
the data available.

However, your explanation above as to why the validation fails in my
tool while it works with apt makes sense, and if that is the reason
(rather than apt falling back on Release/Release.gpg after seeing that
the InRelease signature is invalid, which is what I thought was the
case) then there isn't a problem.

> Also, your indication that apt would drop support for the twin files is
> a bit aggressive. I guess I shouldn't say it too loud as one will appear
> then, but so far there exists no plan to do this.

Oh? I vaguely remember some form of announcement from jak@ who mentioned
that you guys were planning this. I guess that means I either
misunderstood or dreamed it ;-)

> I kinda doubt all existing clients support it or that all repositories
> have one – after all, its a rather new addition in terms of Debian
> stable releases, too, as producing an InRelease file can be hard work
> if you e.g. want two independent signatures on the data and the
> benefits of InRelease only really appear if you have mirrors of some
> sort (and then there are those who want apt to deal with arbitrarily
> historic repos so I don't see such a plan appear without at least some
> opposition regardless of the timeline).
> 
> 
> Anyway, I don't think this bug is actionable in this state: Either add
> moreinfo for the 66 case or move it to your more linter-like tool
> in case of 65. Otherwise I might close it in the future.

If the behavior that I thought was happening (i.e., apt falls back on
Release.gpg if InRelease has a signature but it doesn't validate) is
indeed not happening, then I agree that this bug should be closed.

-- 
To the thief who stole my anti-depressants: I hope you're happy

  -- seen somewhere on the Internet on a photo of a billboard


Reply to: