* cmdline/apt-key:

  - fix apt-key net-update, thanks to Marc Deslauriers and 
    Adam Conrad for the code review (LP: #857472)

4 files changed, 96 insertions, 9 deletions

diff --git a/cmdline/apt-key b/cmdline/apt-key
index 4d2b7c49f..7bfe47fca 100755
--- a/cmdline/apt-key
+++ b/cmdline/apt-key
@@ -22,7 +22,7 @@ MASTER_KEYRING=/usr/share/keyrings/ubuntu-master-keyring.gpg
 ARCHIVE_KEYRING=/usr/share/keyrings/ubuntu-archive-keyring.gpg
 REMOVED_KEYS=/usr/share/keyrings/ubuntu-archive-removed-keys.gpg
 ARCHIVE_KEYRING_URI=http://archive.ubuntu.com/ubuntu/project/ubuntu-archive-keyring.gpg
-
+TMP_KEYRING=/var/lib/apt/keyrings/maybe-import-keyring.gpg
 
 requires_root() {
 	if [ "$(id -u)" -ne 0 ]; then
@@ -34,7 +34,7 @@ requires_root() {
 add_keys_with_verify_against_master_keyring() {
     ADD_KEYRING=$1
     MASTER=$2
-    
+
     if [ ! -f "$ADD_KEYRING" ]; then
 	echo "ERROR: '$ADD_KEYRING' not found"
 	return
@@ -50,11 +50,25 @@ add_keys_with_verify_against_master_keyring() {
     #   from a key in the $distro-master-keyring
     add_keys=`$GPG_CMD --keyring $ADD_KEYRING --with-colons --list-keys | grep ^pub | cut -d: -f5`
     master_keys=`$GPG_CMD --keyring $MASTER --with-colons --list-keys | grep ^pub | cut -d: -f5`
+    
     for add_key in $add_keys; do
-	ADDED=0
+
+        # ensure there are no colisions LP: #857472
+	for master_key in $master_keys; do
+            if [ "$add_key" = "$master_key" ]; then
+                echo >&2 "Keyid collision for '$add_key' detected, operation aborted"
+                return 1
+            fi
+        done
+
+        # export the add keyring one-by-one
+        rm -f $TMP_KEYRING
+        $GPG_CMD --keyring $ADD_KEYRING --output $TMP_KEYRING --export $add_key 
+        # check if signed with the master key and only add in this case
+        ADDED=0
 	for master_key in $master_keys; do
-	    if $GPG_CMD --keyring $ADD_KEYRING --list-sigs --with-colons $add_key | grep ^sig | cut -d: -f5 | grep -q $master_key; then
-		$GPG_CMD --quiet --batch --keyring $ADD_KEYRING --export $add_key | $GPG --import
+	    if $GPG_CMD --keyring $MASTER --keyring $TMP_KEYRING --check-sigs --with-colons $add_key | grep '^sig:!:' | cut -d: -f5 | grep -q $master_key; then
+		$GPG --import $TMP_KEYRING
 		ADDED=1
 	    fi
 	done
@@ -62,14 +76,13 @@ add_keys_with_verify_against_master_keyring() {
 	    echo >&2 "Key '$add_key' not added. It is not signed with a master key"
 	fi
     done
+    rm -f $TMP_KEYRING
 }
 
 # update the current archive signing keyring from a network URI
 # the archive-keyring keys needs to be signed with the master key
 # (otherwise it does not make sense from a security POV)
 net_update() {
-    # Disabled for now as code is insecure
-    exit 1
 
     if [ -z "$ARCHIVE_KEYRING_URI" ]; then
 	echo >&2 "ERROR: Your distribution is not supported in net-update as no uri for the archive-keyring is set"
diff --git a/debian/changelog b/debian/changelog
index e3944d769..a0f59e019 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,10 +1,16 @@
-apt (0.8.16~exp5ubuntu13) UNRELEASED; urgency=low
+apt (0.8.16~exp5ubuntu13) UNRELEASEDoneiric; urgency=low
 
+  [ Adam Conrad ]
   * On armel, call update-apt-xapian-index with '-u' to keep the CPU
     and I/O usage low.  We would do this on all arches, but there's a
     regression risk here, but that's better than killing slow systems.
+  
+  [ Michael Vogt ]
+  * cmdline/apt-key:
+    - fix apt-key net-update, thanks to Marc Deslauriers and 
+      Adam Conrad for the code review (LP: #857472)
 
- -- Adam Conrad <adconrad@ubuntu.com>  Tue, 04 Oct 2011 01:24:01 -0600
+ -- Michael Vogt <michael.vogt@ubuntu.com>  Thu, 06 Oct 2011 16:13:00 +0200
 
 apt (0.8.16~exp5ubuntu12) oneiric; urgency=low
 
diff --git a/test/integration/exploid-keyring-with-dupe-keys.pub b/test/integration/exploid-keyring-with-dupe-keys.pub
new file mode 100644
index 000000000..642952a40
--- /dev/null
+++ b/test/integration/exploid-keyring-with-dupe-keys.pub
diff --git a/test/integration/test-apt-key-net-update b/test/integration/test-apt-key-net-update
new file mode 100755
index 000000000..bc4e6029f
--- /dev/null
+++ b/test/integration/test-apt-key-net-update
@@ -0,0 +1,68 @@
+#!/bin/sh
+set -e
+
+TESTDIR=$(readlink -f $(dirname $0))
+. $TESTDIR/framework
+
+setupenvironment
+configarchitecture "i386"
+
+# mock
+requires_root() {
+    return 0
+}
+
+# extract net_update() and import it
+func=$( sed -n -e '/^add_keys_with_verify_against_master_keyring/,/^}/p' ${BUILDDIRECTORY}/apt-key )
+eval "$func"
+
+mkdir -p ./etc/apt
+TRUSTEDFILE=./etc/apt/trusted.gpg
+mkdir -p ./var/lib/apt/keyrings
+TMP_KEYRING=./var/lib/apt/keyrings/maybe-import-keyring.gpg
+GPG_CMD="gpg --ignore-time-conflict --no-options --no-default-keyring"
+GPG="$GPG_CMD --keyring $TRUSTEDFILE"
+MASTER_KEYRING=/usr/share/keyrings/ubuntu-master-keyring.gpg
+
+
+msgtest "add_keys_with_verify_against_master_keyring"
+if [ ! -e $MASTER_KEYRING ]; then
+    echo -n "No $MASTER_KEYRING found"
+    msgskip 
+    exit 0
+fi
+
+# test bad keyring and ensure its not added (LP: #857472)
+ADD_KEYRING=./keys/exploid-keyring-with-dupe-keys.pub
+if add_keys_with_verify_against_master_keyring $ADD_KEYRING $MASTER_KEYRING; then
+    msgfail
+else
+    msgpass
+fi
+
+# ensure the keyring is still empty
+gpg_out=$($GPG --list-keys)
+msgtest "Test if keyring is empty"
+if [ -n "" ]; then
+    msgfail
+else
+    msgpass
+fi
+
+# test good keyring and ensure we get no errors
+ADD_KEYRING=/usr/share/keyrings/ubuntu-archive-keyring.gpg
+if add_keys_with_verify_against_master_keyring $ADD_KEYRING $MASTER_KEYRING; then
+    msgpass
+else
+    msgfail
+fi
+
+testequal './etc/apt/trusted.gpg
+---------------------
+pub   1024D/437D05B5 2004-09-12
+uid                  Ubuntu Archive Automatic Signing Key <ftpmaster@ubuntu.com>
+sub   2048g/79164387 2004-09-12
+
+pub   1024D/FBB75451 2004-12-30
+uid                  Ubuntu CD Image Automatic Signing Key <cdimage@ubuntu.com>
+' $GPG --list-keys