kiwix / kiwix-build

Kiwix & openZIM build engine
GNU General Public License v3.0
86 stars 42 forks source link

libssl is embedded in the kiwix-desktop appimage #694

Closed veloman-yunkan closed 3 months ago

veloman-yunkan commented 4 months ago

Fixes #871

libssl.so and libcrypto.so are embedded in the appimage so that it can work on newer systems having a deceptively backward-incompatible version of OpenSSL.

The fix includes patching the libQt5Network.so library so that aria2's CA certificate bundle (being included in the appimage before this change) is used as a fallback if no certificates can be found in various locations used for certificate stores on different major Linux distributions. To this end, the AppRun entry point of the AppImage is changed from a symlink to the kiwix-desktop binary to a small shell script that creates a temporary symlink at a hardcoded path (/tmp/cert_bundle_provided_by_kiwix.crt) pointing to the said certificate bundle within the AppImage filesystem.

veloman-yunkan commented 4 months ago

@kelson42 @mgautierfr Will you please advise if it make sense to pursue this approach?

mgautierfr commented 4 months ago

I don't see any strong counter-argument. But I'm really disturbed by the idea of binary patch on security binaries (mainly because I am not a security guy and I don't know). I would be more confident if we could have an input from some openssl people. Can you ask the question upstream to see if something can goes wrong (I'm the feeling it will, it is not for nothing that certificate paths are hard coded)

veloman-yunkan commented 4 months ago

@mgautierfr

it is not for nothing that certificate paths are hard coded

Definitely. The reason is that trusted certificates should be loaded only from trusted locations where unauthorized entities cannot tamper with the content.

The appimage filesystem is mounted as readonly, so - assuming that we build the appimage correctly - we do not introduce additional security risks. If the appimage binary itself is writable, any malware having that privilege can not just replace certificates in it but also patch libQt5Network.so and do any other harmful things. So it doesn't make sense to consider potential vulnerabilities of that kind.

The weakest point in the proposed solution is the need to create a symlink at a fixed path that will serve as a hardcoded location for the certificate store used by the kiwix-desktop appimage. Then malware running in the same user's name will be able to replace that symlink with another one pointing to a location containing fraudulent certificates.

But the question then is why do we have to care about those kind of security issues. Since kiwix-desktop can't be used as a web browser to read articles on the Internet (can it?), it only connects (via the Qt network stack) to library.kiwix.org in order to fetch the OPDS and the book thumbnails1. I see little justification why we couldn't do it over plain HTTP, but given the now-widespread suppression of non-secure HTTP in favour of the secure HTTPS (when you visit http://library.kiwix.org the connection is automatically upgraded to HTTPS) we need to be able to handle the latter. Thus we need a trusted certificate, we provide one and the worst a malware can do is remove that certificate whereupon the connection to library.kiwix.org will fail to be established.

In a more elaborate attack the attacker can spoof the DNS server but then they will also need a valid SSL certificate for the fake library.kiwix.org server under their control and this is where the ability to present arbitrary certificates to the kiwix-desktop appimage can be abused. But currently the same effect can be achieved more easily through the debugging functionality allowing to override the remote library server.


1 downloading of books is performed via aria2c. BTW the latter is put into the appimage along with its own set of certificates, however I doubt that it uses those - since the executable is not patched it should instead load the certificates from a fixed path /etc/ssl/certs/ca-certificates.crt, i.e. from the host system.

veloman-yunkan commented 4 months ago

BTW, libQt5Network.so tries several paths as potential locations of the certificate store:

$ strings /usr/lib/x86_64-linux-gnu/libQt5Network.so|grep /certs
/etc/ssl/certs/
/usr/lib/ssl/certs/
/var/ssl/certs/
/usr/local/ssl/certs/
/etc/openssl/certs/
/opt/openssl/certs/
/etc/pki/tls/certs/ca-bundle.crt
/usr/local/share/certs/ca-root-nss.crt

$ # /tmp/ssl and /tmp/lib/ssl are absent
$ strace ./Kiwix-x86_64.AppImage|&grep /cert
openat(AT_FDCWD, "/tmp/ssl/certs/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/tmp/lib/ssl/certs/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/var/ssl/certs/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/local/ssl/certs/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/etc/openssl/certs/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/opt/openssl/certs/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
statx(AT_FDCWD, "/etc/pki/tls/certs/ca-bundle.crt", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7fff4e8119c0) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/etc/pki/tls/certs", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
statx(AT_FDCWD, "/usr/local/share/certs/ca-root-nss.crt", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7fff4e8119c0) = -1 ENOENT (No such file or directory)
readlink("/usr/local/share/certs", 0x7fff4e810fe0, 1023) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/local/share/certs", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)

We can replace only the last one, so that if the appimage appears to be running on a system missing all of the previous certificate store candidates then we can present our own backup version of the certificates.

veloman-yunkan commented 3 months ago

This PR is now final. The description has been updated.

I tested the appimage built on Ubuntu 20.04 focal by running it on Ubuntu 22.04 jammy in two modes:

  1. default config (/etc/ssl present)
  2. missing /etc/ssl

The test script (see it below) demonstrates how in the first case the system certificate store is used, while in the second scenario a number of paths are tried before the fallback certificate bundle embedded in the appimage and made accessible via a temorary symlink is discovered and used).

Test script (test_appimage)

#!/usr/bin/env bash

set -x

# Run a command via sudo so that the rest of the script
# runs smoothly without asking for a password
if ! sudo strace --version
then
    echo >&2 "ERROR: strace is not available. Exiting"
fi

if [ "$1" == "--rename-etc-ssl" ]
then
    sudo mv /etc/ssl{,.bak}
    trap 'sudo mv /etc/ssl{.bak,}' EXIT
    shift
fi

appimage=${1:-./Kiwix-x86_64.AppImage}

"$appimage" &

until pgrep kiwix-desktop
do
    sleep 1
done

sudo strace -p $(pgrep kiwix-desktop) |& grep 'ssl\|crt'

Scenario 1 (No tampering with /etc/ssl)

$ ./test_appimage 
+ sudo strace --version
strace -- version 5.16
Copyright (c) 1991-2022 The strace developers <https://strace.io>.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Optional features enabled: stack-trace=libunwind stack-demangle m32-mpers mx32-mpers
+ '[' '' == --rename-etc-ssl ']'
+ appimage=./Kiwix-x86_64.AppImage
+ pgrep kiwix-desktop
+ ./Kiwix-x86_64.AppImage
+ sleep 1
Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
+ pgrep kiwix-desktop
46998
+ grep 'ssl\|crt'
++ pgrep kiwix-desktop
+ sudo strace -p 46998
add widget
Downloading "https://library.kiwix.org:443/catalog/search?lang=eng&count=-1&category=wiktionary"
openat(AT_FDCWD, "/tmp/.mount_Kiwix-Tpnmp6/usr/bin/../lib/libssl.so.1.1", O_RDONLY|O_CLOEXEC) = 65
openat(AT_FDCWD, "/usr/lib/ssl/openssl.cnf", O_RDONLY) = 65
read(65, "ertout # insta.cert.pem\n\n[ssl_se"..., 4096) = 131
openat(AT_FDCWD, "/etc/ssl/certs/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = 65
statx(AT_FDCWD, "/etc/ssl/certs/b1159c4c.0", AT_STATX_SYNC_AS_STAT, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=1350, ...}) = 0
Downloading "https://library.kiwix.org:443/catalog/v2/categories"
Downloading "https://library.kiwix.org:443/catalog/v2/languages"
session saved

Scenario 2 (Missing /etc/ssl)

$ ./test_appimage --rename-etc-ssl
+ sudo strace --version
strace -- version 5.16
Copyright (c) 1991-2022 The strace developers <https://strace.io>.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Optional features enabled: stack-trace=libunwind stack-demangle m32-mpers mx32-mpers
+ '[' --rename-etc-ssl == --rename-etc-ssl ']'
+ sudo mv /etc/ssl /etc/ssl.bak
+ trap 'sudo mv /etc/ssl{.bak,}' EXIT
+ shift
+ appimage=./Kiwix-x86_64.AppImage
+ pgrep kiwix-desktop
+ ./Kiwix-x86_64.AppImage
+ sleep 1
Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
+ pgrep kiwix-desktop
47084
+ grep 'ssl\|crt'
++ pgrep kiwix-desktop
+ sudo strace -p 47084
add widget
Downloading "https://library.kiwix.org:443/catalog/search?lang=eng&count=-1&category=wiktionary"
openat(AT_FDCWD, "/tmp/.mount_Kiwix-wLZ0aP/usr/bin/../lib/libssl.so.1.1", O_RDONLY|O_CLOEXEC) = 65
openat(AT_FDCWD, "/usr/lib/ssl/openssl.cnf", O_RDONLY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/etc/ssl/certs/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/lib/ssl/certs/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/share/ssl/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/local/ssl/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/var/ssl/certs/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/local/ssl/certs/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/etc/openssl/certs/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/opt/openssl/certs/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/etc/ssl/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/etc/ssl/certs", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/lib/ssl/certs", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/share/ssl", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/local/ssl", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/var/ssl/certs", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/local/ssl/certs", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/etc/openssl/certs", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/opt/openssl/certs", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/etc/ssl", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
statx(AT_FDCWD, "/etc/pki/tls/certs/ca-bundle.crt", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffd769f1ad0) = -1 ENOENT (No such file or directory)
statx(AT_FDCWD, "/tmp/cert_bundle_provided_by_kiwix.crt", AT_STATX_SYNC_AS_STAT, STATX_ALL, {stx_mask=STATX_BASIC_STATS|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=197624, ...}) = 0
openat(AT_FDCWD, "/tmp/cert_bundle_provided_by_kiwix.crt", O_RDONLY|O_CLOEXEC) = 65
Downloading "https://library.kiwix.org:443/catalog/v2/categories"
Downloading "https://library.kiwix.org:443/catalog/v2/languages"
session saved
+ sudo mv /etc/ssl.bak /etc/ssl
veloman-yunkan commented 3 months ago

It seems that the date must be embedded in the appimage filename (at least that's what the CI failure message suggests).

veloman-yunkan commented 3 months ago

Please rebase-fixup and we can merge.

Pleased, rebased, fixuped, anded, weed, & canned. The only thing left is merging :)