Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upDebian Template: build script security - deal with 'apt-get update' unreliable exit codes #1107
Comments
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
adrelanos
Aug 6, 2015
Member
For your convenience, copying in the relevant issue description here.
prerequisite knowledge, apt-get exit codes are unreliable:
- apt: Provide meaningful exit codes for gpg failures
- provide meaningful exit codes for network failures
- audit 'apt-get update' exit codes
The issue is, that apt-get does not provide meaningful exit codes for gpg and network failures. "Not meaningful" here means, that apt-get update exits 0, even if it failed. This makes it hard to figure out in scripts if apt-get update actually succeeded loading repository metadata or failed.
The threat model here is using multiple repositories. Such as Debian's main repository as well as Debian's security repository. For consistent and secure builds, both have to be enabled. Now, an adversary could make fetching the Debian main repository succeed, but simulate a gpg or network failure for the Debian security repository. Then the build would go on only with the Debian main repository, without the Debian security repository.
In other words... Here is a practical example...
Consider the following sources.list which is fine.
deb http://security.debian.org wheezy/updates main contrib non-free
deb http://ftp.us.debian.org/debian wheezy main contrib non-free
Consider the following sources.list which is fine which contains an intentional typo for demonstration purposes.
deb http://not-security.debian.org wheezy/updates main contrib non-free
deb http://ftp.us.debian.org/debian wheezy main contrib non-free
Now, when you run...
sudo apt-get update ; echo $?
The output shows.
Err http://not-security.debian.org wheezy/updates Release.gpg
Could not resolve 'not-security.debian.org'
Hit http://ftp.us.debian.org wheezy Release.gpg
...
W: Failed to fetch http://not-security.debian.org/dists/wheezy/updates/Release.gpg Could not resolve 'not-security.debian.org'
W: Some index files failed to download. They have been ignored, or old ones used instead.
0
Security repository was excluded. That's an attack an adversary can mount or it can happen for other reasons also. The bad thing here is, apt-get exited 0. Therefore hard to detect these situations in scripts.
The way this was solved in Whonix 10 is by using a wrapper. /usr/lib/apt-get-wrapper, that parses apt-get update's output. Awful hack, but the real fix would require fixing apt-get upstream at Debian. I think the only way to make qubes-builder-debian equally secure would be using that or a similar script/way.
|
For your convenience, copying in the relevant issue description here. prerequisite knowledge, apt-get exit codes are unreliable:
The issue is, that apt-get does not provide meaningful exit codes for gpg and network failures. "Not meaningful" here means, that The threat model here is using multiple repositories. Such as Debian's main repository as well as Debian's security repository. For consistent and secure builds, both have to be enabled. Now, an adversary could make fetching the Debian main repository succeed, but simulate a gpg or network failure for the Debian security repository. Then the build would go on only with the Debian main repository, without the Debian security repository. In other words... Here is a practical example... Consider the following sources.list which is fine.
Consider the following sources.list which is fine which contains an intentional typo for demonstration purposes.
Now, when you run...
The output shows.
Security repository was excluded. That's an attack an adversary can mount or it can happen for other reasons also. The bad thing here is, apt-get exited The way this was solved in Whonix 10 is by using a wrapper. |
marmarek
added
bug
C: builder
P: major
labels
Aug 6, 2015
marmarek
added this to the Release 3.0 milestone
Aug 6, 2015
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
marmarek
Sep 1, 2015
Member
Since this have great potential for regressions, moving after final R3.0.
|
Since this have great potential for regressions, moving after final R3.0. |
marmarek
modified the milestones:
Release 3.1,
Release 3.0
Sep 1, 2015
adrelanos
referenced this issue
Sep 5, 2015
Closed
qubes-download-dom0-updates.sh gets confused by dns resolution issues #1168
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
adrelanos
Oct 16, 2015
Member
The place to implement this is function aptUpdate:
https://github.com/marmarek/qubes-builder-debian/blob/9ca9b4676d227e61344a56a33f55b78d8c143966/template_debian/distribution.sh#L161-L165
Improved that command a bit.
apt-get update 2>&1 | gawk 'BEGIN{IGNORECASE=1} /^W:|^E:/{ret_code=125;}/^/ ; END{exit ret_code}'
Done:
- should match for both,
W:andE: - match
W:andE:case-insensitive, i.e. also forw:ande:(You'll never know if apt-get changes the output at some point.)
My current issue, unclear how to solve:
- there is no need to always
exit 125just because aW:orE:was found in the output, for many cases apt-get properly exits non-zero in case ofW:orE: - (aside: full list)
Does anyone know how to return $? just after apt-get update 2>&1 in case apt-get update 2>&1 already exited non-zero?
We don't want to copy a script into the chroot and execute that. But does this necessarily have to become just a single long command? Would an exported bash function (export -f) be okay? I think that would be much more readable.
|
The place to implement this is function Improved that command a bit.
Done:
My current issue, unclear how to solve:
Does anyone know how to We don't want to copy a script into the chroot and execute that. But does this necessarily have to become just a single long command? Would an exported bash function ( |
marmarek
modified the milestones:
Release 3.1,
Release 3.1 updates
Feb 8, 2016
andrewdavidwong
added
the
C: Debian
label
Jul 1, 2016
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
unman
Apr 16, 2017
Member
@andrewdavidwong I believe this issue still arises in 3.2 milestone - please update label
|
@andrewdavidwong I believe this issue still arises in 3.2 milestone - please update label |
adrelanos commentedAug 6, 2015
Full description of the security issue:
https://phabricator.whonix.org/T170#5357
Issue was confirmed by @nrgaway:
https://phabricator.whonix.org/T170#5792
Suggested solution by @nrgaway:
My comments on that solution:
https://phabricator.whonix.org/T170#5793
apt upstream issues and upstream bug report linked here:
https://phabricator.whonix.org/T170