Comment 3 for bug 1811139

I reviewed kronosnet 1.8-2 as checked into eoan. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

kronosnet is a networking abstraction layer, designed to be used by
corosync (group communication engine) to provide a better, more reliable,
secure, fault tolerant and fast fail-over networking abstraction than just
UDP. It uses openssl or libnss to provide symmetrically encrypted and
authenticated communications.

- CVE History:
  - None
- Build-Depends
  - Packaging depends:
    - debhelper, pkg-config
  - Networking depends:
    - libnl-3-dev, libnl-route-3-dev, libsctp-dev
  - Compression depends:
    - libbz2-dev, liblz4-dev, liblzma-dev, liblzo2-dev, zlib1g-dev
  - Encryption depends:
    - libnss3-dev, libnspr4-dev, libssl-dev
- pre/post inst/rm scripts
  - none
- init scripts
  - none
- systemd units
  - none
- dbus services
  - none
- setuid binaries
  - none
- binaries in PATH
  - none
- sudo fragments
  - none
- udev rules
  - none
- unit tests / autopkgtests
  - unit tests run as part of build and also defined to run as autopkgtests
  - Has tests to ensure all API functions are tested and these appear to be
    quite comprehensive, testing various preconditions and error conditions
    as well as normal behaviours
- cron jobs
  - none
- Build logs:
  - Clean build log - no errors, only warnings are for undocumented parts
    in the doxygen documentation
  - Lintian fails at build time with "bad-distribution-in-changes-file
    unstable" since this is a direct import from Debian with no Ubuntu
    changes

- Processes spawned
  - None in libknet
  - libnozzle runs ifupdown scripts via execlp("/bin/sh") and appears to do
    so safely to avoid command-injection etc
- Memory management
  - Given the large code-base, there is only a small amount of dynamic
    memory allocation, which appears to be done carefully (allocations are
    checked for NULL, strings are NUL terminated etc)
  - Uses memmove() almost exclusively to memcpy() - this is safer since can
    handle overlapping memory areas but will be slower in general - so this
    is also a good sign of defensive (if perhaps unnecessary) programming
- File IO
  - libnozzle:
    - Directly opens /dev/net/tun for ioctl() etc
  - libknet:
    - Plugins are dlopen()'d, otherwise doesn't use files
- Logging
  - Is done carefully in libknet - when opening a knet handle, caller
    provides a file descriptor which libknet then uses for logging during
    it's normal operation. Logs using vsnprintf() underneath and doesn't
    appear to provide any chance for format injection attacks
- Environment variable usage
  - None used
- Use of privileged functions
  - libnozzle
    - uses ioctl() to read MTU, MAC addreses, interface clags etc.
- Use of cryptography / random number sources etc
  - Doesn't do any PKI etc - instead does symmetric encryption via a
    provided encryption key (this is expected to be provided to libknet by
    the user of the API so key distribution is clearly outside of scope for
    libknet)
  - Uses openssl / libnss to perform the crypto (this is configurable and
    our builds support both backends) and do random bytes generation etc
    for salts
  - Includes HMAC for encrypted packets, and does proper HMAC validation
    before decryption etc
- Use of temp files
  - No temp files used
- Use of networking
  - See notes above re cryptography since networking will use this when
    configured
  - Performs good validation of input data
- Use of WebKit
  - None
- Use of PolicyKit
  - None
- Significant cppcheck results
  - None
- Significant Coverity results
  - Coverity identified a large number of possible defects - ranging from
    buffer overruns (on read) which could be triggered on various error
    conditions (such as when failing to read from a socket), failure to NUL
    terminate strings, double frees - most of which occur on error paths. I
    have forwarded these upstream but I do not consider these should block
    kronosnet being promoted to main since they are not expected to occur
    under normal operation and most likely would result in a denial of
    service (and since then they are occurring during error conditions,
    this is not expected to have a high impact).

In general, the code quality of kronosnet is quite high. In particular,
libknet appears to be of slightly higher quality (less Coverity defects
identifed than in libnozzle) and so I feel more comfortable in ACKing this
MIR from a security point-of-view assuming this is only related to the
libknet binary packages and not the libnozzle package.

The defects identified by Coverity do indicate some possible memory errors
and corruption could occur during various error conditions but this needs
further investigation by upstream and should not block the MIR at this
stage. These have been reported upstream so hopefully will be addressed in
future releases of kronosnet -
https://github.com/kronosnet/kronosnet/issues/237

Security team ACK for promoting kronosnet to main for the libknet
packages. If libnozzle is required in a future corosync release, libnozzle
should be re-reviewed at that time.