Ubuntu

APT does not properly handle expired or revoked key signatures

Reported by Michael Casadevall on 2009-04-06
262
This bug affects 1 person
Affects Status Importance Assigned to Milestone
apt (Debian)
Fix Released
Unknown
apt (Ubuntu)
Medium
Michael Casadevall
Dapper
Medium
Jamie Strandboge
Gutsy
Medium
Jamie Strandboge
Hardy
Medium
Jamie Strandboge
Intrepid
Medium
Jamie Strandboge
Jaunty
Medium
Michael Casadevall

Bug Description

apt-get does not properly handle revoked or expired key signatures since it internally uses gpgv vs gpg to check signatures, and does not properly check for the error codes. It uses VALIDSIG to determine if a signature is valid, but this code can be given if the signature itself has expired, the signing key has expired, or the key has been revoked.

Steps to Reproduce:
1. Add a source with expired or revoked key to sources.list (or set the system clock far enough that a key appears to be expired)
2. Run apt-get update
3. No warning message is printed from apt-get.

I'm working on a bazaar branch to resolve this now by properly using gpg vs gpgv and checking the status messages from GPG.

The Debian bug linked does not include that revoked signatures are a problem.

Changed in apt (Ubuntu):
assignee: nobody → mcasadevall
importance: Undecided → Medium
milestone: none → ubuntu-9.04
description: updated
tags: added: apt gpg security
Changed in apt (Ubuntu):
status: New → Confirmed
summary: - [SECURITY] APT does not properly hand expired or revoked key signatures
+ [SECURITY] APT does not properly handle expired or revoked key
+ signatures

I looked at the branch that Michael posted and this is a no-go for a security update for stable because:

a) it adds new strings
b) it changes config option names:
- string pubringpath = _config->Find("APT::GPGV::TrustedKeyring", "/etc/apt/trusted.gpg");
+ string pubringpath = _config->Find("APT::gpg::TrustedKeyring", "/etc/apt/trusted.gpg");

Currently I believe this is a problem with gpgv and should adressed there (also there is some argument
about this given that the man page for gpgv states that it will trust any key).

Michael Vogt (mvo) wrote :

So it seems like the check for VALIDSIG is incorrect (also the doc/DETAILS says:
   VALIDSIG <fingerprint in hex> <sig_creation_date> <sig-timestamp>
                <expire-timestamp> <sig-version> <reserved> <pubkey-algo>
                <hash-algo> <sig-class> <primary-key-fpr>

        The signature with the keyid is good. This is the same as
        GOODSIG but has the fingerprint as the argument.
)

The doc for GOOGSIG says:
   GOODSIG <long keyid> <username>
        The signature with the keyid is good. For each signature only
        one of the three codes GOODSIG, BADSIG or ERRSIG will be
        emitted and they may be used as a marker for a new signature.

Michael Vogt (mvo) wrote :
Download full text (4.7 KiB)

So it seems validsig is written on the status-fd but no goodsig for revoked/expired keys, here is what I see (I ran it with both gpgv and gpg to be able to compare the output):

Here is what gpgv/gpg outputs (on the status fd):

[valid]
$ gpgv --status-fd 1 --keyring /etc/apt/trusted.gpg /var/lib/apt/lists/archive.canonical.com_ubuntu_dists_jaunty_Release.gpg /var/lib/apt/lists/archive.canonical.com_ubuntu_dists_jaunty_Release
gpgv: Signature made 2009-01-21T20:40:58 CET using DSA key ID 437D05B5
[GNUPG:] SIG_ID yBuoChDxaZ0UKcDy7tLCPl5qQ2M 2009-01-21 1232566858
[GNUPG:] GOODSIG 40976EAF437D05B5 Ubuntu Archive Automatic Signing Key <email address hidden>
gpgv: Good signature from "Ubuntu Archive Automatic Signing Key <email address hidden>"
[GNUPG:] VALIDSIG 630239CC130E1A7FD81A27B140976EAF437D05B5 2009-01-21 1232566858 0 3 0 17 2 00 630239CC130E1A7FD81A27B140976EAF437D05B5

$ gpg --verify --status-fd 1 --keyring /etc/apt/trusted.gpg /var/lib/apt/lists/archive.canonical.com_ubuntu_dists_jaunty_Release.gpg /var/lib/apt/lists/archive.canonical.com_ubuntu_dists_jaunty_Release
gpg: WARNING: unsafe ownership on configuration file `/home/egon/.gnupg/gpg.conf'
gpg: Signature made 2009-01-21T20:40:58 CET using DSA key ID 437D05B5
[GNUPG:] SIG_ID yBuoChDxaZ0UKcDy7tLCPl5qQ2M 2009-01-21 1232566858
[GNUPG:] GOODSIG 40976EAF437D05B5 Ubuntu Archive Automatic Signing Key <email address hidden>
gpg: Good signature from "Ubuntu Archive Automatic Signing Key <email address hidden>"
[GNUPG:] VALIDSIG 630239CC130E1A7FD81A27B140976EAF437D05B5 2009-01-21 1232566858 0 3 0 17 2 00 630239CC130E1A7FD81A27B140976EAF437D05B5
[GNUPG:] TRUST_UNDEFINED
gpg: WARNING: This key is not certified with a trusted signature!
gpg: There is no indication that the signature belongs to the owner.
Primary key fingerprint: 6302 39CC 130E 1A7F D81A 27B1 4097 6EAF 437D 05B5

[expired]
$ gpgv --status-fd 1 expired/lala.sig expired/lala
gpgv: Signature made Wed Apr 1 19:40:00 2009 UTC using DSA key ID 89F9D59A
[GNUPG:] KEYEXPIRED 1238701001
[GNUPG:] SIGEXPIRED deprecated-use-keyexpired-instead
[GNUPG:] KEYEXPIRED 1238701001
[GNUPG:] SIGEXPIRED deprecated-use-keyexpired-instead
[GNUPG:] SIG_ID Kzi0hzrQmTz3f+1M7m1VKTMAyeA 2009-04-01 1238614800
[GNUPG:] KEYEXPIRED 1238701001
[GNUPG:] SIGEXPIRED deprecated-use-keyexpired-instead
[GNUPG:] EXPKEYSIG 17427FB989F9D59A foo bar baz <email address hidden>
gpgv: Good signature from "foo bar baz <email address hidden>"
[GNUPG:] VALIDSIG 526A66FC781E11945CC532A817427FB989F9D59A 2009-04-01 1238614800 0 4 0 17 2 00 526A66FC781E11945CC532A817427FB989F9D59A

$ gpg --status-fd 1 --verify expired/lala.sig expired/lala
gpg: Signature made Wed Apr 1 19:40:00 2009 UTC using DSA key ID 89F9D59A
[GNUPG:] KEYEXPIRED 1238701001
[GNUPG:] SIGEXPIRED deprecated-use-keyexpired-instead
[GNUPG:] KEYEXPIRED 1238701001
[GNUPG:] SIGEXPIRED deprecated-use-keyexpired-instead
[GNUPG:] SIG_ID Kzi0hzrQmTz3f+1M7m1VKTMAyeA 2009-04-01 1238614800
[GNUPG:] KEYEXPIRED 1238701001
[GNUPG:] SIGEXPIRED deprecated-use-keyexpired-instead
[GNUPG:] EXPKEYSIG 17427FB989F9D59A foo bar baz <email address hidden>
gpg: Good signature from "foo bar baz <email address hidden>"
[GNUPG:] VALIDSIG 526A66FC781E11945CC532A81742...

Read more...

Kees Cook (kees) wrote :

Does this affect all releases?

summary: - [SECURITY] APT does not properly handle expired or revoked key
- signatures
+ APT does not properly handle expired or revoked key signatures
Michael Vogt (mvo) wrote :

My reading of the above is that the gpg method should check for GOODSIG and only accept signatures that have that, VALIDSIG is not enough. This looks like it might be trick to do without adding new strings :/

It also needs to be verified that all versions of gpgv (back down to dapper) behave consistently in the way that is shown above. The test above was done using jaunty.

Michael Vogt (mvo) wrote :
Michael Vogt (mvo) wrote :

I just checked if the behavior is the same on dapper and my tests indicate it is. Additional testing/clarification is very welcome.

Michael Vogt (mvo) wrote :

(home in the crude test environement is HOME=/home/egon - sorry for that).

Michael Vogt (mvo) wrote :

I looked at the code in gpg/gpgv at mainproc.c:check_sig_and_print() and GOODSIG is only set if its neither bad/expired/revoked:
...
 if(rc)
   statno=STATUS_BADSIG;
 else if(sig->flags.expired)
   statno=STATUS_EXPSIG;
 else if(is_expkey)
   statno=STATUS_EXPKEYSIG;
 else if(is_revkey)
   statno=STATUS_REVKEYSIG;
 else
   statno=STATUS_GOODSIG;
...

Validsig OTOH is returned ignoring the above statuses.

Michael Vogt (mvo) wrote :

Here is a proposed fix. It does the following:

 * recognize KEYEXPIRED and KEYREVOKED messages from gpgv and put them into a new "WorthlessSignatures " vector
 * only listen for GOODSIG message from gpgv and ignore VALIDSIG (as GOODSIG is only send when the signature is not with a expired or revoked key)
 * if there is no good signature, show a message that displayes the worthless signatures to the user (including the KEYEXPIRED or KEYREVSIG bits to ensure there is a way to know what is going on)
 * if there is one (or more) good signature and worthless signatures, just ignore the worthless ones

That should hopefully cover the problem without breaking strings and compatibility. Feedback/review/testing is very welcome. I tested it in a etch chroot with various expired settings and it works as it should, but I need to make a test-suit for it too. I will also pass it for review to debian.

Michael Vogt (mvo) wrote :

 I did a simple "test-suite" to test problem and
fix at:
$ bzr get lp:~mvo/+junk/apt-gpgv-tests
$ cd apt-gpgv-tests
$ sudo ./test.sh

It does test "good","expired","revoked","revoked+good","expired+good".

It should fail for expired and revoked (they are no good) but should
pass for revoked+good and expired+good. Basicly it will ignore expired
or revoked signatures if there is anohter valid signature on the
Release file.

Again, feedback/review is very welcome.

Cheers,
 Michael

Jamie Strandboge (jdstrand) wrote :

Michael, I'll go through this first thing tomorrow.

Jamie Strandboge (jdstrand) wrote :

I've confirmed that Dapper - Jaunty are all affected by the bug and have consistent GOODSIG behavior.

Changed in apt (Ubuntu Dapper):
status: New → Confirmed
importance: Undecided → Medium
Changed in apt (Ubuntu Gutsy):
status: New → Confirmed
importance: Undecided → Medium
Changed in apt (Ubuntu Hardy):
status: New → Confirmed
importance: Undecided → Medium
Changed in apt (Ubuntu Intrepid):
status: New → Confirmed
importance: Undecided → Medium
Jamie Strandboge (jdstrand) wrote :

I spent quite a bit of time reviewing this and feel the the patch is correct. It applies cleanly to intrepid and hardy, has some fuzz on gutsy and needed small adjustments for dapper. I also verified the strings for dapper-intrepid, and there are no new string changes.

I went through the logic of the VerifyGetSigners by hand using desk-checking (see attached for review) for GoodSigners, BadSigners, WorthlessSigners, and NoPubKeySigners being either empty or not empty, and believe the patch to be correct.

I also added several tests to Michael's test suite to ensure apt is working properly after patching and did not introduce regressions. I'll make these available shortly.

Changed in apt (Ubuntu Dapper):
assignee: nobody → Jamie Strandboge (jdstrand)
Changed in apt (Ubuntu Gutsy):
assignee: nobody → Jamie Strandboge (jdstrand)
Changed in apt (Ubuntu Hardy):
assignee: nobody → Jamie Strandboge (jdstrand)
Changed in apt (Ubuntu Intrepid):
assignee: nobody → Jamie Strandboge (jdstrand)
Jamie Strandboge (jdstrand) wrote :

I added some extra tests to https://code.launchpad.net/~jdstrand/+junk/apt-gpgv-tests. To use:

$ bzr get lp:~jdstrand/+junk/apt-gpgv-tests
$ cd apt-gpgv-tests
$ sudo ./test.sh

Michael, I plan to add these to https://launchpad.net/~ubuntu-bugcontrol/qa-regression-testing/master/ which is why there is copyright information in my branch. Please let me know if I should change anything or you don't want it included.

Jamie Strandboge (jdstrand) wrote :
Michael Vogt (mvo) wrote :

Thanks a lot Jamie for the additional tests and the desk_check.txt. That is great.

The copyright is fine, I was just to lazy^Wbusy to add it myself. Please add it to the tracker, I will do the jaunty upload in parallel (including your fix for the date problem).

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apt - 0.7.20.2ubuntu6

---------------
apt (0.7.20.2ubuntu6) jaunty; urgency=low

  [ Jamie Strandboge ]
  * apt.cron.daily: catch invalid dates due to DST time changes
    in the stamp files (LP: #354793)

  [ Michael Vogt ]
  * methods/gpgv.cc:
    - properly check for expired and revoked keys (closes: #433091)
      LP: #356012

 -- Michael Vogt <email address hidden> Wed, 08 Apr 2009 22:39:50 +0200

Changed in apt (Ubuntu Jaunty):
status: Confirmed → Fix Released
Jamie Strandboge (jdstrand) wrote :

I'm won't have an upload for Gutsy, because I won't be publishing this until next week, and Gutsy will be EOL.

Changed in apt (Ubuntu Gutsy):
status: Confirmed → Won't Fix
Changed in apt (Ubuntu Dapper):
status: Confirmed → Fix Committed
Changed in apt (Ubuntu Hardy):
status: Confirmed → Fix Committed
Changed in apt (Ubuntu Intrepid):
status: Confirmed → Fix Committed
visibility: private → public
Jamie Strandboge (jdstrand) wrote :

FYI-- the added apt tests have been added to qa-regression-testing in http://bazaar.launchpad.net/~ubuntu-bugcontrol/qa-regression-testing/master/files/head%3A/scripts/apt/ and the previous 'junk' url removed.

Jamie Strandboge (jdstrand) wrote :
Changed in apt (Ubuntu Dapper):
status: Fix Committed → Fix Released
Changed in apt (Ubuntu Hardy):
status: Fix Committed → Fix Released
Changed in apt (Ubuntu Intrepid):
status: Fix Committed → Fix Released
Changed in apt (Debian):
status: Unknown → Fix Released
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.