apt-get source only checks md5 hashes in Sources files

[Marc Deslauriers]

'apt-get source' only validates the md5 hash in the Sources file. Ideally, it should check the sha hashes also.

[David Kalnischkies (donkult)]

(I should have read all mails before answering some)

Setting to incomplete as I have no idea where you get that idea from. Can you please elaborate?

For history proposes, copy from https://bugs.launchpad.net/launchpad/+bug/1078697/comments/15:
"And of course @mdeslaur, apt-get source does more than just checking MD5. It does what it does for all other downloads as well: Take the "best" checksum it knows and is available for checking if it isn't forced to use another (Acquire::ForceHash). What it does do with MD5 only is checking if the file on the disc matches the file we would download and if it does skipping the download as already done, which should be fixed (so that we can drop MD5 at some point) but has no real security implications as someone with write access to your local disk in that directory has better things to do …"

[Marc Deslauriers (mdeslaur)]

Steps to reproduce in a newly-installed Quantal VM:

1- apt-get update
2- Modify /var/lib/apt/lists/*Sources file to break sha1 and sha256 sums of 'hello' package
3- apt-get source hello

I would expect this to fail, but it doesn't.

If you then modify *Sources again to break the md5 sum of the 'hello' package, apt-get source hello then fails as expected.

In apt-get.cc, DoSource() seems to do:

  new pkgAcqFile(&Fetcher,Last->Index().ArchiveURI(I->Path),
   I->MD5Hash,I->Size,
   Last->Index().SourceInfo(*Last,*I),Src);

[Michael Vogt (mvo)]

On Sat, Jan 12, 2013 at 02:24:09PM -0000, Marc Deslauriers wrote:
> Steps to reproduce in a newly-installed Quantal VM:
>
> 1- apt-get update
> 2- Modify /var/lib/apt/lists/*Sources file to break sha1 and sha256 sums of 'hello' package
> 3- apt-get source hello
>
> I would expect this to fail, but it doesn't.
>
> If you then modify *Sources again to break the md5 sum of the 'hello'
> package, apt-get source hello then fails as expected.
>
> In apt-get.cc, DoSource() seems to do:
>
> new pkgAcqFile(&Fetcher,Last->Index().ArchiveURI(I->Path),
> I->MD5Hash,I->Size,
> Last->Index().SourceInfo(*Last,*I),Src);

Thanks for your bugreport. Indeed, the debsrcrecords.cc parser is just
looking that the "Files" section in the source package record AFAICT
not the new Checksums-{sha1,sha256}.

For a long time the server had no sha{1,256} information in the Source
records. But now that it has there seems to be some issues here too,
e.g. the quantal partner archive has:
"""
Files:
 54ed74fd2fc267d562d35c219c5c33f5 14131200 adobe-flashplugin_11.2.202.261.orig.tar.gz
 f486992e58f025bdbd4cbeb57c7751da 5213 adobe-flashplugin_11.2.202.261-0quantal1.diff.gz
 273e471dbdc98bd28f3d2693ad41dea8 1739 adobe-flashplugin_11.2.202.261-0quantal1.dsc
Checksums-Sha1: 3daa3f68d64940489c04077c5db3123102218fdd 14131200
adobe-flashplugin_11.2.202.261.orig.tar.gz
 69b3cce479651bafe11282ee8efd86a103f90baf 5213 adobe-flashplugin_11.2.202.261-0quantal1.diff.gz
Checksums-Sha256:
c70cfd0df681b3d686c7d1a0bc3f911dd16a5f0710af9fbc3dabdb7be26851db 14131200 adobe-flashplugin_11.2.202.261.orig.tar.gz
 bc339b8e637ae0b99ac009528c1a9f7401322b9a7323d3988af9dfbdac39b944 5213 adobe-flashplugin_11.2.202.261-0quantal1.diff.gz
"""

I.e. for the .dsc file there is just a md5, not a sha available would
be good to figure out why this is the case. Interesstingly the main
archive seems to be ok.

I attached a first iteration of a fix, it needs a test and it also
needs some tweaking, see the FIXMEs. The other issue is that
technically at this form it breaks API as there is the rename MD5Hash
-> Hash. We could leave the misleading name I guess.

Cheers,
 Michael

[Daniel Hartwig (wigs)]

Integration test.

# pkg-sha256-bad has a bad SHA sum, but good MD5 sum. If apt is
# checking the best available hash (as it should), this will trigger
# a hash mismatch.

-- before patch:
Test for hash ok of apt-get source -d pkg-md5-ok … PASS
Test for hash ok of apt-get source -d pkg-sha256-ok … PASS
Test for hash mismatch of apt-get source -d pkg-sha256-bad … FAIL

-- after patch:
Test for hash ok of apt-get source -d pkg-md5-ok … PASS
Test for hash ok of apt-get source -d pkg-sha256-ok … PASS
Test for hash mismatch of apt-get source -d pkg-sha256-bad … PASS

[Michael Vogt (mvo)]

On Thu, Jan 31, 2013 at 02:30:47AM -0000, Daniel Hartwig wrote:
> Integration test.
>
> # pkg-sha256-bad has a bad SHA sum, but good MD5 sum. If apt is
> # checking the best available hash (as it should), this will trigger
> # a hash mismatch.
>
> -- before patch:
> Test for hash ok of apt-get source -d pkg-md5-ok … PASS
> Test for hash ok of apt-get source -d pkg-sha256-ok … PASS
> Test for hash mismatch of apt-get source -d pkg-sha256-bad … FAIL
>
> -- after patch:
> Test for hash ok of apt-get source -d pkg-md5-ok … PASS
> Test for hash ok of apt-get source -d pkg-sha256-ok … PASS
> Test for hash mismatch of apt-get source -d pkg-sha256-bad … PASS

Thanks Daniel! That looks great :)

I added it to my bzr branch for this bug and will merge it to the
debian and ubuntu branches soon. I also cleaned up the FIXMEs and
pushed to lp:~mvo/apt/source-hashes/

Cheers,
 Michael

[Daniel Hartwig (wigs)]

> For a long time the server had no sha{1,256} information in the Source
> records. But now that it has there seems to be some issues here too,
> e.g. the quantal partner archive has:
> …
> I.e. for the .dsc file there is just a md5, not a sha available

A problem for the current patch which would miss those files.

With an API break, the parser could scan through all fields and store a list of available hashes on each file. Similar to how pkgRecords::Parser provides access to each available hash. Subtle problems like this can be avoided.

> The other issue is that
> technically at this form it breaks API as there is the rename MD5Hash
> -> Hash. We could leave the misleading name I guess.

Keeping the name is not an option. The existing field contains “HASH”, where as the proposed patch will use it for “TYPE:HASH”. An API break may be unavoidable to /neatly/ resolve this. Perhaps the additional type–hash pairs could be stored in an auxillery structure, to avoid an immediate break. Existing code can still use the MD5Hash field unchanged.

Other than these issues, it looks ok to me and works, obviously.

[Daniel Hartwig (wigs)]

> Perhaps the additional type–hash pairs
> could be stored in an auxillery structure, to avoid an immediate break.

Though I'm not sure the issue of ignoring SHA sums is serious enough to justify such a temporary hack.

[Christoph Anton Mitterer (calestyo)]

Is this still open?

Sounds rather critical (MD5 is really severly broken)...

IMHO APT's behaviour with respect to verifying signatures should generall be the follwoing:

Secure APT should always verify _all_ of the present sums and fail if _any_ of them doesn't match.... and it should _always_ expect at least one hash some type to be present (i.e. a secure one like SHA3, or SHA512)... and fail it that one is not present.

[Julian Andres Klode (juliank)]

This should be fixed in recent versions.

[Christoph Anton Mitterer (calestyo)]

Hey Julian...

How has this been fixed eventually?

As I proposed, that all available hash algos are verified and if any fails consider it completely failed,... plus a minimum hash algo (that is not md5 or sha1 ^^)?

Cheers,
Chris.