[MIR] duktape

[Jeremy Bícha]

[Availability]
The package duktape is already in Ubuntu universe.
The package duktape build for the architectures it is designed to work on.
It currently builds and works for architetcures: amd64 arm64 armhf ppc64el riscv64 s390x
Link to package https://launchpad.net/ubuntu/+source/duktape

[Rationale]
- The package duktape is required in Ubuntu main for updating polkit.
  Upstream polkit landed an alternative option to use duktape instead
  of mozjs. duktape is a much smaller JavaScript implementation and
  simpler code-base to maintain than mozjs, and Debian is going to use
  it.
  - Support for JavaScript-based rules in polkit have also been
    requested in enterprise desktop use-cases.

- The package duktape is required in Ubuntu main no later than Feb 23
  due to feature freeze.

[Security]
- Had 1 security issue in the past
  - https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-46322
  - https://ubuntu.com/security/CVE-2021-46322

  - no `suid` or `sgid` binaries
  - no executables in `/sbin` and `/usr/sbin`
  - Package does not install services, timers or recurring jobs
  - Packages does not open privileged ports (ports < 1024)
  - Packages does not contain extensions to security-sensitive software
    (filters, scanners, plugins, UI skins, ...)

[Quality assurance - function/usage]
- The package works well right after install

[Quality assurance - maintenance]
- The package is maintained well in Debian/Ubuntu, and currently has
  only one normal and one wishlist bug open
  and long term critical bugs open
  - Ubuntu https://bugs.launchpad.net/ubuntu/+source/duktape/+bugs
  - Debian https://bugs.debian.org/cgi-bin/pkgreport.cgi?src=duktape
    https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=951902
    https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=906201
- The package does not deal with exotic hardware we cannot support

[Quality assurance - testing]
- The package does not run a test at build time because it currently
  does not have a test suite (duktape itself does, but not its Debian
  package).

- The package does not run an autopkgtest because it doesn't have one.

- The package does not have failing autopkgtests right now.

- The package can not be tested at build time because upstream does
  not include tests in their release tarballs, which is what the
  Debian packaging uses -- though I've opened an upstream issue for
  this, requesting inclusion of their test suite in release tarballs:
  https://github.com/svaarala/duktape/issues/2523
  For now, in lieu of that, there is a test plan and example test
  logs, as well as a proposed autopkgtest, included in a separate
  comment below.

[Quality assurance - packaging]
- debian/watch is present and works

- debian/control defines a correct Maintainer field

- This package does not yield massive lintian Warnings, Errors
- Link to a recent build log of the package
  https://launchpad.net/ubuntu/+source/duktape/2.7.0-1/+build/23197556/+files/buildlog_ubuntu-jammy-amd64.duktape_2.7.0-1_BUILDING.txt.gz
- Full output from `lintian --pedantic` attached as an extra post.
- Lintian overrides are not present

- This package does not rely on obsolete or about to be demoted packages.
- This package has no python2 or GTK2 dependencies

- The package will be installed by default, but does not ask debconf
  questions higher than medium

- Packaging and build is easy, link to d/rules
  https://salsa.debian.org/debian-iot-team/duktape/-/blob/master/debian/rules

[UI standards]
- Application is not end-user facing (does not need translation)

[Dependencies]
- No further depends or recommends dependencies that are not yet in main

[Standards compliance]
- This package correctly follows FHS and Debian Policy

[Maintenance/Owner]
- Owning Team will be desktop-packages
- Team is not yet, but will subscribe to the package before promotion

- This does not use static builds

- This does not use vendored code

- This package is not rust based

- The package has been built in the archive more recently than the last
  test rebuild

[Background information]
The Package description explains the package well
Upstream Name is Duktape
Link to upstream project https://duktape.org

[Amin Bandali (bandali)]

Test plan for duktape

1. Install the required packages, clone the upstream source repo,
   checkout the git tag corresponding to the release version you'd
   like to test, and run `make check'. For example:

       sudo apt install devscripts git npm python2 python-yaml
       git clone https://github.com/svaarala/duktape.git
       cd duktape
       git checkout v2.7.0
       make test

   Note that this is currently a bit tricky since the test suite for
   the latest upstream duktape release uses Python 2, which is no
   longer packaged in upcoming Debian or Ubuntu releases. The scripts
   have been ported to Python 3 on upstream's `master' branch, but the
   change is not included in any existing release.

2. Install build-essential and pkg-config, and build and install
   duktape itself. Then, compile and run the simple test.c from
   https://duktape.org and verify it produces the expected output.

   Alternatively, this can be done automatically as an autopkgtest
   per the attached tarball.

[Amin Bandali (bandali)]

Sample logs from running `make check' per part (1) of the above test
plan is attached.

[Amin Bandali (bandali)]

Output from running `lintian --pedantic duktape_2.7.0-1_amd64.changes'
is attached. I believe the lintian warnings are not problems that
should block the MIR, as they are pedantic warnings about minor things
such as man page and copyright year (can send a quick patch to the
Debian maintainer to bump the copyright year).

[Seth Arnold (seth-arnold)]

Have Debian's lawyers said that the copyright years need to be updated?

https://hynek.me/til/copyright-years/

Thanks

[Christian Ehrhardt  (paelzer)]

Review for Package: duktape

[Summary]

My answer was first a NACK:
"""
An alternative is in main (more details in the section [Duplication]. Instead just add a delta to continue to use mozjs please. If you want to challenge that decision, then please make a clear argument why, as long as we have to maintain mozjs anyway for other things, staying on mozjs isn't sufficient.
"""

But on discussion and later comments it became clear that this isn't a blocker despite that.
So instead the verdict is now:
MIR team ACK under the constraint to resolve the below listed required TODOs and as much as
possible having a look at the recommended TODOs.

This does need a security review, but since - for now - this is a NACK anyway
I'll not yet assign ubuntu-security.

List of specific binary packages to be promoted to main: duktape, libduktape207, duktape-dev

Required TODOs:
- Until upstream resolved 2523 auto-generating the matching test sources and
  add it to the package - might be in d/t/* or in the original place out of
  a multi-tarball upload. Either way then please - it is ok if build time tests
  might still be impossible, but for something as complex as a JS engine they
  need to be added as autopkgtest. Otherwise I'm afraid we might too often
  break them without noticing it for quite some time.

Recommended TODOs:
- could you please have a retry if adding back the creation of building
  debug symbols might work nowadays? It will help supporting the package.
  If it does still not work, fine, but at least leave comment on the bug what
  the remaining issue is so it can be revisited in the future.
- Could we have a timeline and maybe a bug/ML reference for "duktape is a much
  smaller JavaScript implementation and simpler code-base to maintain than
  mozjs, and Debian is going to use it." ?

[Duplication]
Duktape is explicitly picked for smaller size and better maintainability.
There are various others, depending on the language of choice like
libjavascriptcoregtk-4.0-bin, libmujs1, mujs and
golang-github-robertkrimen-otto-dev, but none is in main either.
And more like quickjs and elk which are meant to be small (like duktape),
but are not packaged. So ok, of all the alternatives duktape is fine.

But then - as outlined in the report - there is mozjs. I can follow the argument
of "simpler code-base to maintain", but unless we get rid of mozjs it means
we end up with a big and a small codebase. So this makes the overall effort
worse right?
And mozjs is already in main since a long time (search for spidermonkey,
that works better). AFAICS [1] polikit still works with both and [2] many other
things hold mozjs in main. So for Ubuntu it would be better to fully support
one (=mozjs) and carry a delta to use that instead of duktape.

[1]: https://gitlab.freedesktop.org/polkit/polkit/-/blob/master/src/polkitbackend/meson.build#L40
[2]: https://people.canonical.com/~ubuntu-archive/germinate-output/ubuntu.lunar/rdepends/mozjs102/libmozjs-102-0

=> There is annother package in main providing the same functionality.

[Dependencies]
OK:
- no other Dependencies to MIR due to this (just depends on libc)
- no -dev/-debug/-doc packages that need exclusion
  duktap...

[Marc Deslauriers (mdeslaur)]

It is the security team that has requested that Polkit be built with duktape specifically because we are unable to maintain mozjs in any reasonable state of security, and do not wish for Polkit to be built with an insecure javascript engine.

[Christian Ehrhardt  (paelzer)]

Thank you Marc.
In a discussion we have further clarified that this is even true with mozjs staying in main.
"we prefer maintaining duktape than have polkit, which is security sensitive, use mozjs"
So it is in particular about getting it out of a stack that authenticates.

Hence this goes into my prepared secondary answer, which is Ack under constraints.
Here Seb already said: "and we are working on the testing story" which is great.
And I'm assigning to security for review.

[Sebastien Bacher (seb128)]

Adding to that there is no plan from GNOME upstream to move gnome-shell away from mozjs and that's not something we could do as a distro patch. That's an unfortunate code duplication but we feel like we don't really have a choice (out of sticking to a 10 years old unmaintained version of polkit which is also not really desirable, especially that the rules syntax is limited in features compared to what newer versions allow)

[Seth Arnold (seth-arnold)]

I reviewed duktape 2.7.0-1 as checked into kinetic. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.
This is an exceedingly complex package with very intricate inner workings,
we will be highly reliant upon upstream to prepare fixes.

duktape is an embeddable javascript interpreter; it's intended for uses
more like Lua than like v8.

- CVE History:
  - CVE-2021-46322 -- the upstream author communicated results along the
    way, came up with a small fix, and included a test case with the pull
    request. Nice.
- Build-Depends?
  - debhelper-compat, dh-exec
- pre/post inst/rm scripts?
  - None
- init scripts?
  - None
- systemd units?
  - None
- dbus services?
  - None
- setuid binaries?
  - None
- binaries in PATH?
  - /usr/bin/duk
- sudo fragments?
  - None
- polkit files?
  - none
- udev rules?
  - None
- unit tests / autopkgtests?
  - The upstream tests have been removed
  - No autopkgtests
  - This is very unfortunate
- cron jobs?
  - None
- Build logs:
  - clean

- Processes spawned?
  - None
- Memory management?
  - Extremely complicated. I've got concerns.
- File IO?
  - None
- Logging?
  - Spot checks looked fine
- Environment variable usage?
  - None
- Use of privileged functions?
  - None
- Use of cryptography / random number sources etc?
  - None
- Use of temp files?
  - None
- Use of networking?
  - None
- Use of WebKit?
  - None
- Use of PolicyKit?
  - Funny enough... this *is* polkit, now :)

- Any significant cppcheck results?
  - pointless memory leaks in example
- Any significant Coverity results?
  - findings in debugger/ examples/ and polyfills/ -- I didn't inspect
- Any significant shellcheck results?
  - None
- Any significant bandit results?
  - Unsafe yaml load() function in tools/

This is very complex software. The memory management is very intricate,
and almost certainly has flaws. One defining feature of the program is
that it 100% trusts the input code and any stored bytecode. This is a fine
policy decision to make, and the upstream author has articulated it very
well, but it does drastically limit where duktape makes sense to be used.

As a general sketch, polkit files are fine. They are supplied by package
maintainers (who have root) or system administrators (who have root). They
MUST be perfect anyway.

Gnome extensions are iffy. The code quality of these are all over the
place, they're often run by users without review, but even if duktape were
perfect software, that'd still be a very dangerous thing to do. So it's
probably fine but not perfect.

JavaScript content loaded over the web is not appropriate. v8 has no end
of flaws in this threat model and I suspect duktape would be similar.

When I raised my concerns about memory allocations and integer overflows
very early in my review, the upstream author wrote back a very excellent
reply that addressed my concerns completely. We'd be happy to work with
the upstream author again but I suspect the threat model means that very
few bugs will actually be security bugs.

Security team ACK for promoting duktape to main.

[Christian Ehrhardt  (paelzer)]

This still waits on some of the required todos, but otherwise is ready.
Thanks to jbicha we know that Desktop is working on it.

[Sebastien Bacher (seb128)]

The package got updated in mantic now to include the upstream tests and run them as autopkgtests. Sending back to the MIR team for reconsideration. We repacked the tarball and some tidying up is required but that should be good enough for now, things should get better with the next upstream release. We also plan to work on the dbg symbols this cycle, which was a recommended todo in the review.

[Christian Ehrhardt  (paelzer)]

Thank you Seb and Team, that should indeed be good, but we'd like to see it pass on infra once.

The new version isn't there yet:
 duktape | 2.7.0-2 | mantic/universe | source, amd64, arm64, armhf, i386, ppc64el, riscv64, s390x

And due to that https://autopkgtest.ubuntu.com/packages/d/duktape/mantic/amd64 is missing.

I see it hit proposed 28 minutes ago (when you made the update).
Let us at least see that it works and then yes, I think you are good to go.

Would you mind pinging back when tests ran and worked, then we can give it the official "ready" status?

P.S. Thanks for giving symbols another look later on, as always this isn't meant to be busy-work. If it works => great, if not => explain why and we are usually ok (case by case decision).

[Sebastien Bacher (seb128)]

The tests have been running with success now
https://autopkgtest.ubuntu.com/packages/duktape

i386 is failing but that's expected and due to non installable depends on the partial arch and not due to issues in the tests themselve

[Sebastien Bacher (seb128)]

The desktop-packages team has been also subscribed to the package now

[Christian Ehrhardt  (paelzer)]

Thanks, that looks quite good now!

All requirements are fulfilled and more will be looked at later => MIR Ack.
Security Ack is already present.

Overall, ready to go.
Since it is not yet in component mismatches I'll set "in progress" for now.

[Sebastien Bacher (seb128)]

In fact it's on https://ubuntu-archive-team.ubuntu.com/component-mismatches-proposed.html

duktape: duktape-dev libduktape207
MIR: #1997417 (In Progress)
[Reverse-Depends: Rescued from duktape (Uploader: seb128), polkitd (MAIN)]

I'm doing the promotion now

Override component to main
duktape 2.7.0+tests-0ubuntu2 in mantic: universe/misc -> main
duktape-dev 2.7.0+tests-0ubuntu2 in mantic amd64: universe/interpreters/optional/100% -> main
duktape-dev 2.7.0+tests-0ubuntu2 in mantic arm64: universe/interpreters/optional/100% -> main
duktape-dev 2.7.0+tests-0ubuntu2 in mantic armhf: universe/interpreters/optional/100% -> main
duktape-dev 2.7.0+tests-0ubuntu2 in mantic i386: universe/interpreters/optional/100% -> main
duktape-dev 2.7.0+tests-0ubuntu2 in mantic ppc64el: universe/interpreters/optional/100% -> main
duktape-dev 2.7.0+tests-0ubuntu2 in mantic riscv64: universe/interpreters/optional/100% -> main
duktape-dev 2.7.0+tests-0ubuntu2 in mantic s390x: universe/interpreters/optional/100% -> main
libduktape207 2.7.0+tests-0ubuntu2 in mantic amd64: universe/libs/optional/100% -> main
libduktape207 2.7.0+tests-0ubuntu2 in mantic arm64: universe/libs/optional/100% -> main
libduktape207 2.7.0+tests-0ubuntu2 in mantic armhf: universe/libs/optional/100% -> main
libduktape207 2.7.0+tests-0ubuntu2 in mantic i386: universe/libs/optional/100% -> main
libduktape207 2.7.0+tests-0ubuntu2 in mantic ppc64el: universe/libs/optional/100% -> main
libduktape207 2.7.0+tests-0ubuntu2 in mantic riscv64: universe/libs/optional/100% -> main
libduktape207 2.7.0+tests-0ubuntu2 in mantic s390x: universe/libs/optional/100% -> main
Override [y|N]? y