Bug #1971738 “[MIR] ell” : Bugs : ell package : Ubuntu

Sebastien Bacher (seb128) on 2022-05-06

description:	updated
Changed in ell (Ubuntu):
importance:	Undecided → High

Lukas Märdian (slyon) on 2022-05-10

Changed in ell (Ubuntu):
assignee:	nobody → Christian Ehrhardt  (paelzer)

Christian Ehrhardt  (paelzer) on 2022-05-17

Changed in ell (Ubuntu):
milestone:	none → ubuntu-22.08

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2022-05-17:

#1

lintian artifact Edit (5.2 KiB, text/plain)

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2022-05-17:

#2

Local build log (including post build lintian) Edit (239.0 KiB, text/plain)

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2022-05-17:

#3

Download full text (4.1 KiB)

Review for Package: ell

[Summary]
It replicates functionality that exists otherwise, but in a scope that
we do not have in an alternate form. Other than that it seems to be
nice and trimmed to just the function it is advertising.
=> MIR team ACK

This does need a security review, so I'll assign ubuntu-security

List of specific binary packages to be promoted to main: ell
Specific binary packages built, but NOT to be promoted to main: <none>

Required TODOs:
- none
Recommended TODOs:
- none

[Duplication]
Well - the whole base system system is what is duplicated in this library.
dhcp requests, icmp handling, tls, netlink, ... many more things that a base
system would do are essentially re-implemented as a library (instead of
independent tools/services) here. That is some duplication, but it is done
in a way to provide those as a lib/binding interface as used by IWD.
And in that use-case/context there is no duplication in Ubuntu main yet.

Problems: None

[Dependencies]
OK:
- no other Dependencies to MIR due to this
- -dev package will be auto-promoted but also has sane dependencies
- No dependencies in main that are only superficially tested requiring
more tests now.

Problems: None

[Embedded sources and static linking]
OK:
- no embedded source present (just a kernel header for gpio)
- no static linking
One common way to use ell is static linking, but nowadays it does allow
to be a dynamic lib and that is how it e.g. is used from iwd
- does not have odd Built-Using entries
- not a go package, no extra constraints to consider in that regard

Problems: None

[Security]
OK:
- history of CVEs does not look concerning (unless we consider having none
as concerning since the functionality it covers usually has CVEs)
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not process 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)
- does not deal with security attestation (secure boot, tpm, signatures)

Problems:
- does parse data formats (most of the functions interact with some remote
  component)
- does open a port/socket (for some functions)
- While not having CVEs yet, in general this covers a lot of things like dhcp,
  dbus or many others that are known to be epxloited in other places. So the
  assumption should be that there is a reasonable attack surface here as well.

[Common blockers]
OK:
- does not FTBFS currently
- does have a test suite that runs at build time
- test suite fails will fail the build upon error.
- No need for special HW to test
- no new python2 dependency

Problems:
- does not have a non-trivial test suite that runs as autopkgtest
  - a non-trivial test on this level does not make sense (the lib alone
    is only doing rather simple things), but there is an autopkgtest in iwd
    which is the context that pulls this into main.
    I think there is no strong need to require or recommend to add an
    autopkgtest on this level as well.

[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- symbols tracking is in place
- d/watch is presen...

Review for Package: ell

[Summary]
It replicates functionality that exists otherwise, but in a scope that
we do not have in an alternate form. Other than that it seems to be
nice and trimmed to just the function it is advertising.
=> MIR team ACK

This does need a security review, so I'll assign ubuntu-security

List of specific binary packages to be promoted to main: ell
Specific binary packages built, but NOT to be promoted to main: <none>

Required TODOs:
- none
Recommended TODOs:
- none

[Duplication]
Well - the whole base system system is what is duplicated in this library.
dhcp requests, icmp handling, tls, netlink, ... many more things that a base
system would do are essentially re-implemented as a library (instead of
independent tools/services) here. That is some duplication, but it is done
in a way to provide those as a lib/binding interface as used by IWD.
And in that use-case/context there is no duplication in Ubuntu main yet.

Problems: None

[Dependencies]
OK:
- no other Dependencies to MIR due to this
- -dev package will be auto-promoted but also has sane dependencies
- No dependencies in main that are only superficially tested requiring
  more tests now.

Problems: None

[Embedded sources and static linking]
OK:
- no embedded source present (just a kernel header for gpio)
- no static linking
  One common way to use ell is static linking, but nowadays it does allow
  to be a dynamic lib and that is how it e.g. is used from iwd
- does not have odd Built-Using entries
- not a go package, no extra constraints to consider in that regard

Problems: None

[Security]
OK:
- history of CVEs does not look concerning (unless we consider having none
  as concerning since the functionality it covers usually has CVEs)
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not process 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)
- does not deal with security attestation (secure boot, tpm, signatures)

Problems:
- does parse data formats (most of the functions interact with some remote
  component)
- does open a port/socket (for some functions)
- While not having CVEs yet, in general this covers a lot of things like dhcp,
  dbus or many others that are known to be epxloited in other places. So the
  assumption should be that there is a reasonable attack surface here as well.

[Common blockers]
OK:
- does not FTBFS currently
- does have a test suite that runs at build time
  - test suite fails will fail the build upon error.
- No need for special HW to test
- no new python2 dependency

Problems:
- does not have a non-trivial test suite that runs as autopkgtest
  - a non-trivial test on this level does not make sense (the lib alone
    is only doing rather simple things), but there is an autopkgtest in iwd
    which is the context that pulls this into main.
    I think there is no strong need to require or recommend to add an
    autopkgtest on this level as well.

[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- symbols tracking is in place
- d/watch is present and looks ok
- Upstream update history is good
- Debian/Ubuntu update history is good
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs that so far
  maintained the package
- no massive Lintian warnings
  - a few very minor ones like override_dh_auto_test-does-not-check-DEB_BUILD_OPTIONS
- d/rules is rather clean
- It is not on the lto-disabled list

Problems: None

[Upstream red flags]
OK:
- no Errors/warnings during the build
- no incautious use of malloc/sprintf (as far as we can check it)
  has its own malloc wrapper to be used by lib-users (makes alloc
  fails fatal)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (usage is OK inside
  tests)
- no use of user nobody
- no use of setuid
- no important open bugs (crashers, etc) in Debian or Ubuntu
- no dependency on webkit, qtwebkit, seed or libgoa-*
- not part of the UI for extra checks
- no translation present, but none needed for this case (not user visible)

Problems: None

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

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2022-05-17:

#4

This really LGTM, but it needs security review as well.

FYI: I've also set the milestone to match was mentioned in the initial report.

Revision history for this message

Seth Arnold (seth-arnold) wrote on 2022-05-17:

#5

Hmm this thing seems to have its own TLS stack https://git.kernel.org/pub/scm/libs/ell/ell.git/tree/ell/tls.c

as well as rc4 and rc2 implementations https://git.kernel.org/pub/scm/libs/ell/ell.git/tree/ell/cipher.c

Does Intel have any guidance on this thing being certified FIPS / CC / STIG / etc?

Thanks

Revision history for this message

Sebastien Bacher (seb128) wrote on 2022-05-19:

#6

One 'detail' I noticed today while checking for other things, bluez which is in main includes a copy of ell which it is using. It has an option to use an external ell instead.

Revision history for this message

Christian Ehrhardt  (paelzer) wrote on 2022-05-19: Re: [Bug 1971738] Re: [MIR] ell

#7

On Wed, May 18, 2022 at 12:51 AM Seth Arnold <email address hidden> wrote:
>
> Hmm this thing seems to have its own TLS stack
> https://git.kernel.org/pub/scm/libs/ell/ell.git/tree/ell/tls.c

Yeah, as I've written it reimplements all base functions of a small
system in the lib.

Revision history for this message

Sebastien Bacher (seb128) wrote on 2022-05-20:

#8

We have added a simple autopkgtest to test a build with the lib to validate the basic and depends

https://autopkgtest.ubuntu.com/packages/e/ell

description:

updated

Seth Arnold (seth-arnold) on 2022-05-25

tags:

added: sec-1041

Camila Camargo de Matos (ccdm94) on 2022-06-03

Changed in ell (Ubuntu):
status:	New → In Progress
assignee:	Ubuntu Security Team (ubuntu-security) → Camila Camargo de Matos (ccdm94)

Revision history for this message

Camila Camargo de Matos (ccdm94) wrote on 2022-07-12:

#9

Download full text (14.6 KiB)

I reviewed ell 0.50-1ubuntu2 as checked into kinetic. This shouldn't be considered a full audit but rather a quick gauge of maintainability.

ell is the Embedded Linux Library. It is a library similar to GNOME Glib which allows for application development in C, providing, for example, core low-level functionalities for Linux system daemons. It is lightweight and therefore, a better fit for usage in Embedded Linux applications.

- CVE History:
  - It was previously mentioned in this bug that this package is vulnerable to CVE-2021-36081. This CVE, however, seems to instead be associated to the software tesseract, and not to ell. There are no patches provided by ell's upstream, only tesseract's upstream, and the patches do not relate at all to the implementation of ell. The only thing that seems to be ell-related in this CVE is the presence of the term 'one_ell_conflict' in the CVE description. Therefore, it will be considered that this software has no CVEs associated with it.
- Dependencies:
  - According to debian/control, package build depends on dbus-daemon, debhelper-compat (= 13) and libglib2.0-dev. The software also depends on the GNU C library.
  - There are no encryption libraries listed in the dependencies, however, ell does use the Kernel's Crypto API in order to perform a lot of its encryption operations. A lot of certificate management is implemented by ell directly. Given this information, it is important to mention that the 'About' section in ell's git mentions the need for a Linux kernel with support for epoll and timerfd, as well as if_alg for checksum usage and enablement of the keyctl() syscall for usage of Diffie-Hellman. As per https://git.kernel.org/pub/scm/libs/ell/ell.git/about/:

Kernel configuration checklist:

CONFIG_TIMERFD=y
CONFIG_EVENTFD=y

                        CONFIG_CRYPTO_USER_API=y
                        CONFIG_CRYPTO_USER_API_HASH=y
                        CONFIG_CRYPTO_MD5=y
                        CONFIG_CRYPTO_SHA1=y

CONFIG_KEY_DH_OPERATIONS=y

    These seem to be enabled by default in the Linux Kernel for Kinetic, except for CONFIG_CRYPTO_USER_API and CONFIG_CRYPTO_USER_API_HASH, which are set as 'm'.
  - DBus is needed to run the tests, which are also run during build.
- No pre/post inst/rm scripts.
- No init scripts.
- No systemd units (this is a library package).
- No dbus services (this is a library package).
- No setuid binaries (source package includes a 'tools' directory, but these do not seem to be installed).
- No sudo fragments.
- No polkit files.
- No udev rules.
- No cron jobs (this is a library package).
- Library installation path:
    /usr/lib/x86_64-linux-gnu/libell.a
    /usr/lib/x86_64-linux-gnu/libell.so.0.0.2
    /usr/lib/x86_64-linux-gnu/libell.so
    /usr/lib/x86_64-linux-gnu/libell.so.0
- The package does include unit tests. The tests run smoothly in kinetic and generate visible and understandable results, which is good when performing regression tests after application of a patch. The process to run these tests is easy (running a 'make check' command), with no external tools or configurat...

I reviewed ell 0.50-1ubuntu2 as checked into kinetic.  This shouldn't be considered a full audit but rather a quick gauge of maintainability.

ell is the Embedded Linux Library. It is a library similar to GNOME Glib which allows for application development in C, providing, for example, core low-level functionalities for Linux system daemons. It is lightweight and therefore, a better fit for usage in Embedded Linux applications.

- CVE History:
  - It was previously mentioned in this bug that this package is vulnerable to CVE-2021-36081. This CVE, however, seems to instead be associated to the software tesseract, and not to ell. There are no patches provided by ell's upstream, only tesseract's upstream, and the patches do not relate at all to the implementation of ell. The only thing that seems to be ell-related in this CVE is the presence of the term 'one_ell_conflict' in the CVE description. Therefore, it will be considered that this software has no CVEs associated with it.
- Dependencies:
  - According to debian/control, package build depends on dbus-daemon, debhelper-compat (= 13) and libglib2.0-dev. The software also depends on the GNU C library.
  - There are no encryption libraries listed in the dependencies, however, ell does use the Kernel's Crypto API in order to perform a lot of its encryption operations. A lot of certificate management is implemented by ell directly. Given this information, it is important to mention that the 'About' section in ell's git  mentions the need for a Linux kernel with support for epoll and timerfd, as well as if_alg for checksum usage and enablement of the keyctl() syscall for usage of Diffie-Hellman. As per https://git.kernel.org/pub/scm/libs/ell/ell.git/about/:

Kernel configuration checklist:

CONFIG_TIMERFD=y
                        CONFIG_EVENTFD=y

CONFIG_CRYPTO_USER_API=y
                        CONFIG_CRYPTO_USER_API_HASH=y
                        CONFIG_CRYPTO_MD5=y
                        CONFIG_CRYPTO_SHA1=y

CONFIG_KEY_DH_OPERATIONS=y

These seem to be enabled by default in the Linux Kernel for Kinetic, except for CONFIG_CRYPTO_USER_API and CONFIG_CRYPTO_USER_API_HASH, which are set as 'm'.
  - DBus is needed to run the tests, which are also run during build.
- No pre/post inst/rm scripts.
- No init scripts.
- No systemd units (this is a library package).
- No dbus services (this is a library package).
- No setuid binaries (source package includes a 'tools' directory, but these do not seem to be installed).
- No sudo fragments.
- No polkit files.
- No udev rules.
- No cron jobs (this is a library package).
- Library installation path:
    /usr/lib/x86_64-linux-gnu/libell.a
    /usr/lib/x86_64-linux-gnu/libell.so.0.0.2
    /usr/lib/x86_64-linux-gnu/libell.so
    /usr/lib/x86_64-linux-gnu/libell.so.0
- The package does include unit tests. The tests run smoothly in kinetic and generate visible and understandable results, which is good when performing regression tests after application of a patch. The process to run these tests is easy (running a 'make check' command), with no external tools or configuration needed.
- The build logs do not present any significant errors/warnings. There are, however, a few lintian error messages for this package for both deb files (libell0 and libell-dev).
- There are processes spawned in executables from the test directory. All calls that are made involve function execvpe. Since this usage is only present in test code, and not in the library itself, this seems to not be a worrisome security issue.
- The code, when performing memory management, uses a lot of functions that are considered dangerous and could result in security vulnerabilities, such as strcpy and memcpy. Checks when using these functions and when accessing allocated memory seem to be done consistently in most cases, with a few cases where it was verified that these checks are not done. This, however, is something expected of C code, and in general there seems to be care from the ell development team to avoid memory related issues. An honorable mention is the fact that ell seems to implement its own family of 'alloc's, where failure to allocate results in immediate program termination and allocations of size 0 return the NULL value. This avoids situations where a pointer is returned to an allocation of size 0, which can happen and cause many issues. It was noticed that there are also instances where a lack of a check is done purposefully, and in a few of the places where this happened, it was explicitly mentioned in comment form (in the function description) that checks were expected to be performed by the user of the ell library, as the ell library would not do this itself. An example of this is the following: function l_utf8_to_utf16 is associated with the comment "Converts a UTF8 formatted string to UTF16. It is assumed that the string is valid UTF8 and no sanity checking is performed". A lot of the checks that are performed were noticed to be "implicit" checks, an example being: a variable is not checked for a negative value because it has been set as an unsigned integer; or, a string being copied to a buffer does not have it's size verified because all calls to the function that perform such copy use hardcoded string values that are guaranteed to not exceed the destination buffer's size; or, a check that guarantees that a pointer is not null is made in a function that is called. All functions called within this initial one do not perform additional checks as they are not called by any other functions except the said initial one which does the check. These could all be seen as issues, given that it is easy to change code and forget to add essential checks that would avoid improper memory access vulnerabilities, however, it seems to be this way because ell is a library focused on performance. Given all other evidence on how the library is structured and how development is done for the library, it is a risk, but there is care to avoid such things, and when issues do show up, they seem to be fixed appropriately. There also is concern with memory initialization: ell's functions very commonly perform initialization of allocated arrays, removing any previous random values that might have been in the newly accessible memory area, and is a positive thing. Therefore, the final conclusion is that memory management is performed sufficiently well enough, considering that this is a C code. Should this library be used directly, knowledge of how exported functions work is necessary to avoid memory and security issues due to the fact that it is expected that certain checks will be performed by the developer using the library and not the library itself (use library with caution). 
- File I/O operations mainly access devices, buses and open sockets instead of dealing with actual files. Data used to allow this type of access is mostly sanitized, and data being recovered is also mostly sanitized, in a lot of cases, according to RFC rules, since the device is a socket being used for some type of network communication. There are functions, however, that seem to not validate data received from external sources due to the function being simply a low-level means to allow a library user to use that data for other types of processing. It was detected that the ell library uses the Linux Kernel GPIO interface, which has its use recommended for embedded systems. Actual files that are created by the library are created with 0600 permissions, which is a good set of permissions. The library seems to miss path verification on a few instances, trusting user input blindly. This however, not being a common issue in the code, does not mean the library is poorly implemented. It might be that it is expected once again that the caller of ell functions which deal with file handling will be performing these checks instead of ell itself, since, once again, it seems like the library's focus is performance and quick execution/results.
- The library allows for logging in both systemd and journald (has functions to connect to both unix sockets related to these logging daemons). It also allows debugging to happen in stderr. Ell seems to provide debug functions so that a developer using the library can set debug messages. The way that they work indicates that their usage aims to give a developer control over how to handle debug data generated by ell. In general, ell error messages seem to be carefully crafted and not prone to format string vulnerabilities (functions in the printf family being called appropriately).
- Environment variable usage happens in a few functions which are exported by the library. The values in these environment variables are minimally sanitized, however, there could be vulnerabilities associated with these variables depending on permissions associated with who runs the library code and who sets the variable value. However, since ell is just a library, it wouldn't be its responsibility to enforce this, but instead of the user who runs the ell code. Environment variables are also set in test code, however, their usage there is less concerning.
- Ell seems to use the Kernel Crypto API (through creation of AF_ALG sockets) for encryption, meaning, it uses a recognized framework to perform encryption operations instead of implementing things from scratch, which is a positive thing from a security point of view. However, the AF_ALG feature is known to have some security related issues associated with it, which could cause ell to become a means for an attacker to exploit the kernel. However, given that it is also something used by software such as openssl, and considering that it might be used as a means to achieve speed, this choice could be seen as a best effort solution. Ell also uses the Kernel's ASN.1 certificate parser, and it seems like the kernel is the go-to place for ell to run/implement cryptography and certificate functions, probably because, once again, it allows for speed of execution. Functions that implement certificate management in ell follow RFCs properly, with various comments included in the code explaining what the RFC instructs, referencing in which RFC and where in the RFC the information can be found at, and explaining how the code is following the RFC. This is a good indicator that things are not poorly implemented regarding certificates and networking in general. The code for certificate management is very well organized.
- Tests seem to create a lot of temporary files with predictable names. Created directories and opened files in these tests are properly closed and removed after test execution, and are created with strong enough permissions (0600 and 0700). No user input is used in said tests. That being said, tests are not a part of the final build and usage of the library does not imply usage of tests. Therefore, concerns regarding tests should exist mostly if the package is to be built from source.
- Regarding networking, the code uses the SET_CLOEXEC flags when creating sockets, which is good to avoid race conditions and leakage of file descriptors to child processes. Sockets are properly closed when operations that attempt to open them and bind them fail, and error messages are returned to the user. Missing networking functionalities in the dhcp.c file (address conflict detector) are mentioned in a TODO comment. The code uses BPF to filter data received from sockets in certain situations. Checks are properly performed in data used to create sockets. Uses AF_UNSPEC (layer-3 agnostic), which could be of concern were the system be receiving messages, since it exposes itself to getting larger responses than necessary (which could result in a stack buffer overflow), however, it seems like functions that do so are not receiving messages, only sending them. Most of the networking operations performed by ell are operations which involve low level networking tasks, since it is a library. Therefore, functions which check untrusted inputs and such are not really present here. General checks on header formats and sizes are performed, and more thorough checks mainly are performed in certificate management functions. The code seems to properly add prefixes/masks by converting addresses to binary format and performing & or | operations to get bits to flip as necessary. This is safer than directly managing the string. DHCP functions are well organized, with functions being divided into different files, one of DHCPv4 and one for DHCPv6. Functions that are "the same '' have similar names, the 6 added to DHCPV6 cases. This allows for organization and better programming on part of who is using the library. In general, the code follows RFCs and cites them a lot in comments, therefore, use cases should be well known to those that know the RFCs.
- No WebKit.
- No PolicyKit.
- Is not compiled with PIE enabled.
- Coverity presented a lot of false positives, however, there were a few results which indicated memory leaks in the code, unchecked return values and possible improper memory access. Most of these errors, however, are to be expected to exist in C code, and they are few.

Ell does not seem to have much documentation nor does it have an easily accessible way to track bugs/issues and their resolutions, however, there have not been any security bugs reported in order to better understand how security issues presented by the community are handled. That being said, they do have documentation on how to submit patches, and a clear set of rules on how patches should be formatted. They do seem to have a mailing list. Ell developers also seem to care about security issues and fix them when they are found, with https://git.kernel.org/pub/scm/libs/ell/ell.git/commit/?id=a501e4dccf9323044469e48475762f876478ab527 and https://git.kernel.org/pub/scm/libs/ell/ell.git/commit/?id=fd6ae8d5e6a54ce700064d113acd1da171bbb452 being two examples of commits that were made to the code as a result of a security scan done by Coverity. The software seems to not evolve too quickly, with constant updates happening and constant fixes being made, but not an absurd amount which makes the software extremely volatile. This means that it would be a good candidate for patching and for security maintenance, because backporting might be easier. No bug reports were found for ell in Debian nor in Ubuntu. The code seems to include useful comments which reference a few choices made, and there are commits that indicate that the software is actively being developed and improved, with items in their TODO list being implemented. Commits are well explained (the commit message details what is happening).

Therefore, even with a few possible bugs in the code, Ell seems to be well maintained enough and well coded enough to be supported by the Security Team as a main package.

Security team ACK for promoting ell to main.

Changed in ell (Ubuntu):
assignee:	Camila Camargo de Matos (ccdm94) → nobody

Revision history for this message

Didier Roche-Tolomelli (didrocks) wrote on 2022-08-24:

#10

only desktop-packages need to be subscribed, I will let that to seb when he promotes it.

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

Revision history for this message

Sebastien Bacher (seb128) wrote on 2022-09-19:

#11

The source had already been promoted it seems, I subscribed the desktop-packages team and promoted libell0 now

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

Ubuntu
ell package

[MIR] ell

Bug Description

CVE References

Other bug subscribers

Bug attachments

Remote bug watches

Ubuntuell package

[MIR] ell

Bug Description

CVE References

Other bug subscribers

Bug attachments

Remote bug watches

Ubuntu
ell package