Ubuntu
apt package

apt fails to verify keys when Dir has space, and set via cmdline

Bug #1672710 reported by Dimitri John Ledkov on 2017-03-14

This bug affects 1 person

	Status	Importance	Assigned to
apt (Ubuntu)	Fix Released	High	Unassigned
Xenial	Fix Released	Low	Unassigned
Yakkety	Won't Fix	High	Julian Andres Klode

Bug Description

When Dir has a space, and it is set via APT_CONFIG file, keys are found and validated correctly.
When Dir is set without a space via cmdline, keys are found and validated correctly.
When Dir is set with a space via cmdline, keys are not found and repositories are not verified.

[Test case]
Please see attached reproducer, which works on xenial system (gpg1) but not on zesty system (gpg2)

$ bash reproducer.sh
++ mktemp -d
+ tmpdir=/tmp/tmp.sFipy6h5yL
+ pushd /tmp/tmp.sFipy6h5yL
/tmp/tmp.sFipy6h5yL ~
+ mkdir 'Sub Dir'
+ pushd 'Sub Dir'
/tmp/tmp.sFipy6h5yL/Sub Dir /tmp/tmp.sFipy6h5yL ~
+ mkdir -p etc/apt/apt.conf.d
+ mkdir -p etc/apt/trusted.gpg.d
+ mkdir -p etc/apt/preferences.d
+ mkdir -p var/lib/apt/lists/partial
+ mkdir -p var/lib/dpkg
+ touch var/lib/dpkg/status
+ cp /etc/apt/trusted.gpg.d/ubuntu-keyring-2012-archive.gpg etc/apt/trusted.gpg.d/
+ echo 'deb http://archive.ubuntu.com/ubuntu/ trusty main'
+ echo 'Dir "/tmp/tmp.sFipy6h5yL/Sub Dir";'
+ export APT_CONFIG=/tmp/tmp.sFipy6h5yL/apt.conf
+ APT_CONFIG=/tmp/tmp.sFipy6h5yL/apt.conf
+ cat /tmp/tmp.sFipy6h5yL/apt.conf
Dir "/tmp/tmp.sFipy6h5yL/Sub Dir";
+ :
+ : == list available keys ==
+ apt-key list
/tmp/tmp.sFipy6h5yL/Sub Dir/etc/apt/trusted.gpg.d/ubuntu-keyring-2012-archive.gpg
---------------------------------------------------------------------------------
pub rsa4096 2012-05-11 [SC]
790B C727 7767 219C 42C8 6F93 3B4F E6AC C0B2 1F32
uid [ unknown] Ubuntu Archive Automatic Signing Key (2012) <email address hidden>

+ :
+ : == update with environ APT_CONFIG setting the Dir variable ==
+ apt update
Ign:1 http://archive.ubuntu.com/ubuntu trusty InRelease
Get:2 http://archive.ubuntu.com/ubuntu trusty Release [58.5 kB]
Get:3 http://archive.ubuntu.com/ubuntu trusty Release.gpg [933 B]
Get:4 http://archive.ubuntu.com/ubuntu trusty/main amd64 Packages [1,350 kB]
Fetched 1,410 kB in 0s (1,959 kB/s)
Reading package lists... Done
Building dependency tree... Done
All packages are up to date.
+ unset APT_CONFIG
+ :
+ : == update with cmdline Dir option setting Dir to relative pwd ==
+ apt -o Dir=./ update
Ign:1 http://archive.ubuntu.com/ubuntu trusty InRelease
Hit:2 http://archive.ubuntu.com/ubuntu trusty Release
Reading package lists... Done
Building dependency tree... Done
All packages are up to date.
+ :
+ : == update with cmdline Dir option setting Dir to absolute pwd with space ==
+ apt -o 'Dir=/tmp/tmp.sFipy6h5yL/Sub Dir' update
Ign:1 http://archive.ubuntu.com/ubuntu trusty InRelease
Hit:2 http://archive.ubuntu.com/ubuntu trusty Release
Err:3 http://archive.ubuntu.com/ubuntu trusty Release.gpg
The following signatures couldn't be verified because the public key is not available: NO_PUBKEY 40976EAF437D05B5 NO_PUBKEY 3B4FE6ACC0B21F32
Reading package lists... Done
Building dependency tree... Done
All packages are up to date.
W: An error occurred during the signature verification. The repository is not updated and the previous index files will be used. GPG error: http://archive.ubuntu.com/ubuntu trusty Release: The following signatures couldn't be verified because the public key is not available: NO_PUBKEY 40976EAF437D05B5 NO_PUBKEY 3B4FE6ACC0B21F32
W: Failed to fetch http://archive.ubuntu.com/ubuntu/dists/trusty/Release.gpg The following signatures couldn't be verified because the public key is not available: NO_PUBKEY 40976EAF437D05B5 NO_PUBKEY 3B4FE6ACC0B21F32
W: Some index files failed to download. They have been ignored, or old ones used instead.

[Regression Potential]
The fix changes the code to generate quotes in the Commmandline:AsString option, which dumps the command line for logging purposes. This was not parseable before, causing the file to be rejected, so what can happen is that something now works that did not before.

See original description

Tags:

Revision history for this message

Dimitri John Ledkov (xnox) wrote on 2017-03-14:

reproducer.sh Edit (873 bytes, text/x-sh)

Changed in apt (Ubuntu):
status:	New → Confirmed
importance:	Undecided → High

Revision history for this message

Dimitri John Ledkov (xnox) wrote on 2017-03-14:

I made the apt-key dump the passed config file...

The difference between the generated files in the first and second cases are:

-CommandLine::AsString "apt update";
+CommandLine::AsString "apt -o Dir="/tmp/tmp.26IBhxIxAw/Sub Dir" update"";

When executing apt-config with the second file as the APT_CONFIG to get the trusted keys I get:

$ APT_CONFIG=/tmp/apt-key-cheat-1489496770 apt-config shell TRUSTEDPARTS Dir::Etc::TrustedParts/d
E: Syntax error /tmp/apt-key-cheat-1489496770:213: Extra junk at end of file

Revision history for this message

Dimitri John Ledkov (xnox) wrote on 2017-03-14:

I guess the correct generated config should have:
CommandLine::AsString "apt -o Dir=""/tmp/tmp.26IBhxIxAw/Sub Dir"" update";

instead of the currently broken:
CommandLine::AsString "apt -o Dir="/tmp/tmp.26IBhxIxAw/Sub Dir" update"";

Note the wrong double double quote escaping.

Revision history for this message

Julian Andres Klode (juliank) wrote on 2017-03-14:

apt config does not support double quote escaping. "" just ends the first string and starts another. and \" does not work either.

Specifying Dir on the commandline does not really work in a sane way anyway, as it loads the configuration files from /etc, but the rest from within the specified directory. You likely always want to use APT_CONFIG if you want to specify Dir.

Revision history for this message

Launchpad Janitor (janitor) wrote on 2017-04-05:

This bug was fixed in the package apt - 1.4

---------------
apt (1.4) unstable; urgency=medium

* The April Fools' Release

  [ Julian Andres Klode ]
  * Ignore \.ucf-[a-z]+$ like we do for \.dpkg-[a-z]+$
  * Fix mistake in CHANGEPATH comment example

  [ Chris Lamb ]
  * auto-removal: Ignore running kernel if attempting a reproducible build
    (Closes: #857632)

[ Joe Dalton ]
* Danish program translation update (Closes: #856723)

  [ David Kalnischkies ]
  * Fix and avoid quoting in CommandLine::AsString (LP: #1672710)
  * Ignore AutomaticRemove conffile option in upgrade (Closes: #855891)

-- Julian Andres Klode <email address hidden> Sat, 01 Apr 2017 21:39:37 +0200

Changed in apt (Ubuntu):
status:	Confirmed → Fix Released

Julian Andres Klode (juliank) on 2017-04-21

description:	updated
Changed in apt (Ubuntu Yakkety):
importance:	Undecided → High
status:	New → Triaged
assignee:	nobody → Julian Andres Klode (juliank)

Revision history for this message

Julian Andres Klode (juliank) wrote on 2017-04-25:

xenial has the same problem, but does not actually parse the file to apt-key. If we SRU there, I suggest:

apt-config dump -o foo=" bar " | sed s#CommandLine::AsString#CommandLineAsString#g | apt-config shell FOO CommandLineAsString -c /dev/stdin

as the test case.

xenial:
> E: Syntax error /dev/stdin:211: Extra junk after value
current:
> FOO='apt-config dump -o foo='\'' bar '\'''

Julian Andres Klode (juliank) on 2017-04-25

Changed in apt (Ubuntu Xenial):
status:	New → Triaged
importance:	Undecided → Low

Julian Andres Klode (juliank) on 2017-04-25

Changed in apt (Ubuntu Xenial):
status:	Triaged → Fix Committed
Changed in apt (Ubuntu Yakkety):
status:	Triaged → Fix Committed

Revision history for this message

Chris J Arges (arges) wrote on 2017-04-26: Please test proposed package

Hello Dimitri, or anyone else affected,

Accepted apt into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/apt/1.2.21 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

tags:

added: verification-needed

Revision history for this message

Chris J Arges (arges) wrote on 2017-04-26:

Hello Dimitri, or anyone else affected,

Accepted apt into yakkety-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/apt/1.3.6 in a few hours, and then in the -proposed repository.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Revision history for this message

Adam Conrad (adconrad) wrote on 2017-07-27:

Hello Dimitri, or anyone else affected,

Accepted apt into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/apt/1.2.24 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-xenial to verification-done-xenial. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-xenial. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

tags:	added: verification-needed-xenial
Changed in apt (Ubuntu Yakkety):
status:	Fix Committed → Won't Fix

Revision history for this message

Dimitri John Ledkov (xnox) wrote on 2017-07-28:

#10

Started with:
ii apt 1.2.20 amd64 commandline package manager

Run modified test case to check that quoted strings are escaped right:

root@apt-verify:~# apt-config dump -o foo=" bar " | sed s#CommandLine::AsString#CommandLineAsString#g | apt-config shell FOO CommandLineAsString -c /dev/stdin
E: Syntax error /dev/stdin:233: Extra junk after value

if failed.

Upgraded to:
ii apt 1.2.24 amd64 commandline package manager

And it works now:

# apt-config dump -o foo=" bar " | sed s#CommandLine::AsString#CommandLineAsString#g | apt-config shell FOO CommandLineAsString -c /dev/stdin
FOO='apt-config dump -o foo='\'' bar '\'''

tags:

added: verification-done verification-done-xenial
removed: verification-needed verification-needed-xenial

Revision history for this message

Launchpad Janitor (janitor) wrote on 2017-07-31:

#11

This bug was fixed in the package apt - 1.2.24

---------------
apt (1.2.24) xenial; urgency=medium

  * Microrelease covering fixes of 1.4.6
  * Fix parsing of or groups in build-deps with ignored packages (LP: #1694697)
  * apt.systemd.daily: Use unattended-ugrade --download-only if available.
    Instead of passing -d, which enables a debugging mode; check if
    unattended-upgrade supports an option --download-only (which is yet
    to be implemented) and use that (Closes: #863859)

apt (1.2.23) xenial; urgency=medium

* Microrelease covering fixes of 1.4.4

[ Alan Jenkins ]
* apt.systemd.daily: fix error from locking code (Closes: #862567)

apt (1.2.22) xenial; urgency=medium

  [ Julian Andres Klode ]
  * Run unattended-upgrade -d in download part
  * apt.systemd.daily: Add locking
  * Split apt-daily timer into two (LP: #1686470)

[ Matt Kraai ]
* bash-completion: Fix spelling of autoclean (Closes: #861846)

apt (1.2.21) xenial; urgency=medium

* Microrelease covering fixes of 1.4 and 1.4.1

  [ Julian Andres Klode ]
  * Ignore \.ucf-[a-z]+$ like we do for \.dpkg-[a-z]+$
  * systemd: Rework timing and add After=network-online (was LP #1615482)

[ David Kalnischkies ]
* Fix and avoid quoting in CommandLine::AsString (LP: #1672710)

[ Unit 193 ]
* apt-ftparchive: Support '.ddeb' dbgsym packages

-- Julian Andres Klode <email address hidden> Mon, 19 Jun 2017 13:58:04 +0200

Changed in apt (Ubuntu Xenial):
status:	Fix Committed → Fix Released

Revision history for this message

Adam Conrad (adconrad) wrote on 2017-07-31: Update Released

#12

The verification of the Stable Release Update for apt has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.