While studying some details of
https://salsa.debian.org/ftp-team/code-signing/-/blob/master/secure-boot-code-sign.py,
I noticed that the "Check that files.json doesn't use absolute paths" in
check_template doesn't seem quite sufficient. The current code is as
follows:
# Check that files.json doesn't use absolute paths
for pkg, metadata in job.bin_pkgs.items():
assert(re.match(r'[a-z0-9][a-z0-9.+-]+\Z', pkg))
for file in metadata['files']:
assert(any(file["file"].startswith(prefix) for prefix in ("boot/", "lib/", "usr/")))
assert(".." not in file["file"])
assert(not os.path.isabs(file["file"]))
Consider packages that look like this:
foo:
./usr/lib -> ../../../../../etc
foo-signed-template:
./usr/share/code-signing/foo-signed-template/files.json:
{"foo": {"files": [{"sig_type": "efi", "file": "usr/lib/passwd"}]}}
This will pass, because the file name starts with "usr/", doesn't
contain "..", and isn't absolute. sign_and_log_files will then go on to
resolve those file names relative to the extracted binary package being
signed, following symlinks, and might be able to read any data
accessible to the user running secure-boot-code-sign.py.
(Incidentally, the check for ".." there is implemented weirdly. It
should check that none of the individual segments are "..", not that the
substring ".." appears nowhere in the path.)
Obviously this would never pass any kind of attentive manual check, and
there are probably limited ways in which you could actually use this to
extract information, so I can't really justify RC severity here; but
since you have these checks in place I'm assuming that you think it's
worth them being effective.
The following is a mostly untested patch (though I've tested some of the
individual bits of code in isolation). I generally find it easier to
get this stuff right using pathlib nowadays, so I converted the relevant
bits of code to that, on the assumption that you have at least Python
3.9 for Path.is_relative_to.
I considered filing this as a merge request, but since nobody's replied
to either of my fairly straightforward MRs in
https://salsa.debian.org/ftp-team/code-signing/-/merge_requests, I
suspect that the right people might not be subscribed to those.
diff --git a/secure-boot-code-sign.py b/secure-boot-code-sign.py
index 97f86c1..81ed844 100755
--- a/secure-boot-code-sign.py
+++ b/secure-boot-code-sign.py
@@ -24,6 +24,7 @@ import fcntl
import hashlib
import json
import os
+import pathlib
import re
import subprocess
import sys
@@ -160,7 +161,7 @@ def hash_file(f):
return m.hexdigest()