Bug #1811139 “[MIR] kronosnet” : Bugs : kronosnet package : Ubuntu

Gianfranco Costamagna (costamagnagianfranco) on 2019-05-06

description:	updated
Changed in kronosnet (Ubuntu):
assignee:	nobody → MIR approval team (ubuntu-mir)

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2019-05-06:

#1

Taking the review on this

Changed in kronosnet (Ubuntu):
assignee:	MIR approval team (ubuntu-mir) → Christian Ehrhardt  (paelzer)

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2019-05-06:

#2

Download full text (3.3 KiB)

[Duplication]
There is no equivalent functionality in main.
Some of its crypto functionality referred to be simlar to libnss3 but that is no duplication either as it implements it by using libnss3 just providing a different level of abstraction and integration into the other kronosnet features.

This source also contains libnozzle* binary packages which are not pulled in by corosync.
MIR review is on source scope, but I'd nack that part of it and recommend to not pull it in.
It is described as:
This is an over-engineered commodity library to manage a pool of tap devices and provides the basic pre-up.d/up.d/down.d/post-down.d infrastructure.
I think we have enough fun with pre/post hooks in the systemd-networkd world already.
Not a nack for nozzle, but a request to think twice then.
Looking at references it might be added by corosync down the road:
https://trello.com/c/v6kp0xj3/60-add-support-for-libnozzle-in-corosync30

kronosnet itself seems safe and a unique use case to be worth promiting.

[Embedded sources and static linking]
- no embedded libraries or tools
- no static linking used
- no golang

[Security]
- no CVEs yet
- it does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not processes arbitrary web content
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)

Ok, but needing review by security:
- kronosnet parses data formats (its own transport)
- opens a port (indirectly by providing network to apps using the lib)
- it has no daemon at the moment, but kronosnetd exists (is not packaged at the moment)
- by the nature what functionality the package provides (network transport, encryption) it is security sensitive and needs a security review

[Common blockers]
- Does it FTBFS currently? => No (only on x32 which is not in Ubuntu)
- Test suite for kronosnet active and working
- Tests for nozzle active, but all tests skipped
- But subscriber will be the server-team for HA packages in general
- I have not found translations, but this is only for admins not for (unskilled) end-user usage
- no python code to consider for py2

[Packaging red flags]
- Ubuntu carries no delta
- symbols tracking in place
- d/watch in place
- Debian is very up to date and has regular uploads
- no Lintian warnings at all (wow)
- d/rules is rather clean
- not using Built-Using

[Upstream red flags]
- regular upstream releases
- no errors, a few warnings on undocumented internal data structures and functions
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH
- no Open bugs
  - none except the MIR in Ubuntu
  - only one about x32 FTBFS in Debian (no problem for us)
  - no major bugs open atm (crashers, etc) in Upstream
- No Dependency on webkit, qtwebkit, seed or libgoa-*
- no Embedded source copies
- not part of the Unity Dash for privacy settings?

[Summary]
I'd not really like libnozzle yet (e.g tests disabled) but even that looks ok.
The actual request is for libknet which seems good upstream and well packaged and maintained in Debian.
Ack from the MIR team.

But the nature of the package requires a securi...

[Duplication]
There is no equivalent functionality in main.
Some of its crypto functionality referred to be simlar to libnss3 but that is no duplication either as it implements it by using libnss3 just providing a different level of abstraction and integration into the other kronosnet features.

This source also contains libnozzle* binary packages which are not pulled in by corosync.
MIR review is on source scope, but I'd nack that part of it and recommend to not pull it in.
It is described as:
 This is an over-engineered commodity library to manage a pool of tap devices and provides the basic pre-up.d/up.d/down.d/post-down.d infrastructure.
I think we have enough fun with pre/post hooks in the systemd-networkd world already.
Not a nack for nozzle, but a request to think twice then.
Looking at references it might be added by corosync down the road:
  https://trello.com/c/v6kp0xj3/60-add-support-for-libnozzle-in-corosync30

kronosnet itself seems safe and a unique use case to be worth promiting.

[Embedded sources and static linking]
- no embedded libraries or tools
- no static linking used
- no golang

[Security]
- no CVEs yet
- it does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not processes arbitrary web content
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)

Ok, but needing review by security:
- kronosnet parses data formats (its own transport)
- opens a port (indirectly by providing network to apps using the lib)
- it has no daemon at the moment, but kronosnetd exists (is not packaged at the moment)
- by the nature what functionality the package provides (network transport, encryption) it is security sensitive and needs a security review

[Common blockers]
- Does it FTBFS currently? => No (only on x32 which is not in Ubuntu)
- Test suite for kronosnet active and working
  - Tests for nozzle active, but all tests skipped
- But subscriber will be the server-team for HA packages in general
- I have not found translations, but this is only for admins not for (unskilled) end-user usage
- no python code to consider for py2

[Packaging red flags]
- Ubuntu carries no delta
- symbols tracking in place
- d/watch in place
- Debian is very up to date and has regular uploads
- no Lintian warnings at all (wow)
- d/rules is rather clean
- not using Built-Using

[Upstream red flags]
- regular upstream releases
- no errors, a few warnings on undocumented internal data structures and functions
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH
- no Open bugs
  - none except the MIR in Ubuntu
  - only one about x32 FTBFS in Debian (no problem for us)
  - no major bugs open atm (crashers, etc) in Upstream
- No Dependency on webkit, qtwebkit, seed or libgoa-*
- no Embedded source copies
- not part of the Unity Dash for privacy settings?

[Summary]
I'd not really like libnozzle yet (e.g tests disabled) but even that looks ok.
The actual request is for libknet which seems good upstream and well packaged and maintained in Debian.
Ack from the MIR team.

But the nature of the package requires a security review, hence assigning to security.

Finally the package has no team bug subscriber yet, please resolve that as well before it can be promoted.

Changed in kronosnet (Ubuntu):
assignee:	Christian Ehrhardt  (paelzer) → Ubuntu Security Team (ubuntu-security)

Rafael David Tinoco (rafaeldtinoco) on 2019-07-02

tags:

added: ubuntu-ha

Revision history for this message

Alex Murray (alexmurray) wrote on 2019-07-03:

#3

Download full text (5.1 KiB)

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 generatio...

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.

Changed in kronosnet (Ubuntu):
assignee:	Ubuntu Security Team (ubuntu-security) → nobody

Christian Ehrhardt  (paelzer) on 2019-07-03

Changed in kronosnet (Ubuntu):
status:	New → In Progress

Rafael David Tinoco (rafaeldtinoco) on 2019-07-08

Changed in kronosnet (Ubuntu):
importance:	Undecided → High

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2019-07-15:

#4

Since the change that triggers the mismatch is already in the archive the correct state for now is actually "Fix Committed" - waiting for an AA to resolve the mismatch.

Changed in kronosnet (Ubuntu):
status:	In Progress → Fix Committed

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2019-07-15:

#5

This is "almost" complete, I pinged powerj to subscribe the server Team to kronosnet as initially discussed in comment #2.

Revision history for this message

Joshua Powers (powersj) wrote on 2019-07-15:

#6

ubuntu-server is now subscribed

Revision history for this message

Steve Langasek (vorlon) wrote on 2019-07-16:

#7

Override component to main
kronosnet 1.8-2 in eoan: universe/misc -> main
libknet-dev 1.8-2 in eoan amd64: universe/libdevel/optional/100% -> main
libknet-dev 1.8-2 in eoan arm64: universe/libdevel/optional/100% -> main
libknet-dev 1.8-2 in eoan armhf: universe/libdevel/optional/100% -> main
libknet-dev 1.8-2 in eoan i386: universe/libdevel/optional/100% -> main
libknet-dev 1.8-2 in eoan ppc64el: universe/libdevel/optional/100% -> main
libknet-dev 1.8-2 in eoan s390x: universe/libdevel/optional/100% -> main
libknet-doc 1.8-2 in eoan amd64: universe/doc/optional/100% -> main
libknet-doc 1.8-2 in eoan arm64: universe/doc/optional/100% -> main
libknet-doc 1.8-2 in eoan armhf: universe/doc/optional/100% -> main
libknet-doc 1.8-2 in eoan i386: universe/doc/optional/100% -> main
libknet-doc 1.8-2 in eoan ppc64el: universe/doc/optional/100% -> main
libknet-doc 1.8-2 in eoan s390x: universe/doc/optional/100% -> main
libknet1 1.8-2 in eoan amd64: universe/libs/optional/100% -> main
libknet1 1.8-2 in eoan arm64: universe/libs/optional/100% -> main
libknet1 1.8-2 in eoan armhf: universe/libs/optional/100% -> main
libknet1 1.8-2 in eoan i386: universe/libs/optional/100% -> main
libknet1 1.8-2 in eoan ppc64el: universe/libs/optional/100% -> main
libknet1 1.8-2 in eoan s390x: universe/libs/optional/100% -> main
libnozzle-dev 1.8-2 in eoan amd64: universe/libdevel/optional/100% -> main
libnozzle-dev 1.8-2 in eoan arm64: universe/libdevel/optional/100% -> main
libnozzle-dev 1.8-2 in eoan armhf: universe/libdevel/optional/100% -> main
libnozzle-dev 1.8-2 in eoan i386: universe/libdevel/optional/100% -> main
libnozzle-dev 1.8-2 in eoan ppc64el: universe/libdevel/optional/100% -> main
libnozzle-dev 1.8-2 in eoan s390x: universe/libdevel/optional/100% -> main
libnozzle1 1.8-2 in eoan amd64: universe/libs/optional/100% -> main
libnozzle1 1.8-2 in eoan arm64: universe/libs/optional/100% -> main
libnozzle1 1.8-2 in eoan armhf: universe/libs/optional/100% -> main
libnozzle1 1.8-2 in eoan i386: universe/libs/optional/100% -> main
libnozzle1 1.8-2 in eoan ppc64el: universe/libs/optional/100% -> main
libnozzle1 1.8-2 in eoan s390x: universe/libs/optional/100% -> main
31 publications overridden.

Changed in kronosnet (Ubuntu):
status:	Fix Committed → Fix Released

Revision history for this message

Fabio Massimo Di Nitto (fabbione) wrote on 2019-07-17:

#8

Hi guys,

upstream maintainer here :)

I saw a few comments above related to libnozzle test suite not being executed. Due to the nature of libnozzle, the test suite has to be executed as root unfortunately. The ioctl required to manage tap devices are privileged only.

The test suite detects if running as root or not and act consequently.

Also, we are working to address the issues found by coverity scan (thanks for the report btw) and integrating coverity into upstream CI as well.

Cheers
Fabio

Revision history for this message

Seth Arnold (seth-arnold) wrote on 2019-07-17: Re: [Bug 1811139] Re: [MIR] kronosnet

#9

On Wed, Jul 17, 2019 at 06:39:06AM -0000, Fabio Massimo Di Nitto wrote:
> upstream maintainer here :)

Hello Fabio, thanks for saying hello :)

> I saw a few comments above related to libnozzle test suite not being
> executed. Due to the nature of libnozzle, the test suite has to be
> executed as root unfortunately. The ioctl required to manage tap devices
> are privileged only.
>
> The test suite detects if running as root or not and act consequently.

Ah, this makes good sense. It'd probably make more sense for the debian
packaging to use autopkgtests to run this, rather than build time tests.
(That might not be the easiest thing to do, coordinating tests built in
the package but not shipped in the package is a fair chunk of work.)

> Also, we are working to address the issues found by coverity scan
> (thanks for the report btw) and integrating coverity into upstream CI as
> well.

Very good news! Coverity can spot things that are just otherwise
impossible to even find.

Thanks

Ubuntu
kronosnet package

[MIR] kronosnet

Bug Description

Other bug subscribers

Remote bug watches

Ubuntukronosnet package

[MIR] kronosnet

Bug Description

Other bug subscribers

Remote bug watches

Ubuntu
kronosnet package