Possible bug in ucert?

For Developers
1

I'm building my own firmware for my devices, nothing non standard.

I encountered problems during sysupgrade which led me to conclusion that there may ne bugs in ucert.

Process

  1. from sysupgrade commandline i got error "Image check failed"
  2. I sourced it to the lack of 'valid=true' in /usr/libexec/validate_firmware_image
  3. it was because /lib/upgrade/fwtool.sh fwtool_check_signature failed on command:
    fwtool -q -T -s /dev/null "$1" | ucert -V -m - -c "/tmp/sysupgrade.ucert" -P /etc/opkg/keys
  4. i then checked that keys and certs are present, fwtool works on device, and firmware has signature attached (fwtool -s filetocheck firmware.bin)
  5. I then checked, learned and then manually recreated adding signature to the firmware file on the build machine (ucert, usign, fwtool) but it resulted in the same firmware (same signature added)
  6. I narrowed it down to the key-build.ucert file (which looks more/less ok(*)), so I recreated it from scratch but got the same problem
  7. I then search how cert verification is done, patched ucert.c to check where exactly the problem is
  8. Looks like the problem starts in cert_load function around blob_parse_untrusted function which reports 'no attributes found'

Here I stopped my learning journey, asking for help or guidance for next steps :slight_smile:

(*) the only thing which makes me wonder is why key-build.ucert contains binary data, while other files (pub, priv, sig) are text only. Maybe thats a problem? However I used default key-builds in the process, and they were generated by standard openwrt build process (package/base-files/Makefile is the place of key-build generation)

2

Yes, ucert is likely broken since the recent libubox API changes.

3

Thanks,
can you confirm that it's good/bad to have binary bytes in line 1 and 3 (or 4) of key-build.ucert ?

5

same issue here - json is unable to parse the cert (most likely to do with the binary bytes)

strace output

open("sysupgrade.ucert", O_RDONLY|O_LARGEFILE) = 3
readv(3, [{iov_base="\0\0\1d\0\0\0\233untrusted comment: signe"..., iov_len=4094}, {iov_base="", iov_len=1024}], 2) = 516
readv(3, [{iov_base="", iov_len=3578}, {iov_base="", iov_len=1024}], 2) = 0
close(3)                                = 0
writev(2, [{iov_base="cert_verify(600): cannot parse c"..., iov_len=36}, {iov_base=NULL, iov_len=0}], 2cert_verify(600): cannot parse cert
) = 36
6

I encounter the same problem.
ucert can ONLY parse '*.ucert' file without sig(nature) appended.

admin@OpenWrt:~# ucert -D -c /tmp/sysupgrade.ucert 
cert_dump(406): cannot parse cert
admin@OpenWrt:~# fwtool -q -T -s /dev/null /cache/sysupgrade.bin | ucert -V -m -
 -c /tmp/sysupgrade.ucert -P /etc/opkg/keys/
cert_verify(600): cannot parse cert
7

As suggested by @jow , I check the commit log in package/system/cert, the issued HEAD version is:

commit 25e1afb9e1884b262404e4f2c6d3ecc149052fe1
Author: Petr Štetiar <ynezz@true.cz>
Date:   Thu Dec 19 13:25:03 2019 +0100

    ucert: update to version 2019-12-19
    
    14a279411cff fix certificate blob parsing vulnerability by using blob_parse_untrusted
    19a7225ac018 fix leaking memory in cert_dump_blob
    9dba44ddd4f5 fix possibly garbage value returned in cert_process_revoker
    4462ff9dedfa add cram based unit tests
    5fe64b5606aa cmake: split usign bits into static library
    5d7626a2b6d8 cmake: reindent the file
    e284ed941972 cmake: enable hardening compiler flags and fix the reported issues
    7e5390666347 add initial GitLab CI support
    fa0bf4ef45b1 cmake: add proper include and library dependencies
    
    Signed-off-by: Petr Štetiar <ynezz@true.cz>
    (cherry picked from commit 2544cb1ba377149a8663c7ac4a625d5399993e33)

diff --git a/package/system/ucert/Makefile b/package/system/ucert/Makefile
index 3e8935c9a0..bb095c0f51 100644
--- a/package/system/ucert/Makefile
+++ b/package/system/ucert/Makefile
@@ -5,9 +5,9 @@ PKG_RELEASE:=1
 
 PKG_SOURCE_PROTO:=git
 PKG_SOURCE_URL=$(PROJECT_GIT)/project/ucert.git
-PKG_SOURCE_DATE:=2019-11-29
-PKG_SOURCE_VERSION:=e4bd927cc7c756de5f3005824b63a7a6d827e1ee
-PKG_MIRROR_HASH:=cc6853e861110dd89cc8bd909c5f02150c2c3f9a77de66017393515644a7ac36
+PKG_SOURCE_DATE:=2019-12-19
+PKG_SOURCE_VERSION:=14a279411cff06f9b1363711df4ec3b5db73f042
+PKG_MIRROR_HASH:=797f35138c6e1099a839e768d9e01db022223884d9e0a6d09965e625caf0ae79
 
 CMAKE_INSTALL:=1
 PKG_CHECK_FORMAT_SECURITY:=1

After I revert ucert from 2019-12-19 back to 2019-11-29(previous revision), the ucert_load seems to work fine.

admin@OpenWrt:~# ucert -D -c /tmp/sysupgrade.ucert 
=== CHAIN ELEMENT 01 ===
signature:
---
untrusted comment: signed by key 779bc910508f9d9e
RWR3m8kQUI+dnuj8ypvbERZ/cKD4ppnip2dJO1KDeObOHmJf3OPGfxL0GpxQf25mpgqyANI3LVh5SEFJri9dXIJni4Y0MMeaaQw=
---
payload:
---
"ucert": {
        "certtype": 1,
        "validfrom": 1621484753,
        "expiresat": 1653020753,
        "pubkey": "untrusted comment: Local build key\nRWR3m8kQUI+dnia0n3qN40SO2ps8iIlwsqCxEka3xV0VNd70990B9mGo\n"
}
---
=== CHAIN ELEMENT 02 ===
signature:
---
untrusted comment: signed by key 779bc910508f9d9e
RWR3m8kQUI+dnt8jWHChwc6Ik9NvJcYDYnk8UWFACJ9avgGY8pZy3jXNCv7zIuDfpHz3nwA8fQKU8vAzAReNWUuRGg4bg7X7AQI=
---
admin@OpenWrt:~# fwtool -q -T -s /dev/null "/cache/sysupgrade.bin" | ucert -V -m
 - -c "/tmp/sysupgrade.ucert" -P /etc/opkg/keys 
OK
OK
admin@OpenWrt:~#