[MIR] libfido2, libcbor (dependencies of openssh)

[Matthias Klose]

[MIR] libfido2, libcbor, mathjax (dependencies of openssh)

[Matthias Klose (doko)]

added libcbor-doc to extra-excludes, depending on mathjax-js.

[Matthias Klose (doko)]

The packaging looks ok, both packages provide symbols files.

libcbor didn't have a release for about two years, and upstream seems to have accumulated a few bug reports.

Both packages need a security review, and maybe the security team subscribing to the packages (as they did for openssh itself).

[Matthias Klose (doko)]

and subscribed foundations team

[Colin Watson (cjwatson)]

For background, OpenSSH 8.2 uses these for U2F hardware authenticator support, as described under "Changes since OpenSSH 8.1" in https://www.openssh.com/txt/release-8.2. This is a headline feature in 8.2 and I think it would be a significant net benefit to get it into 20.04 so that we can start encouraging its widespread use.

[Seth Arnold (seth-arnold)]

On Mon, Feb 24, 2020 at 11:45:12AM -0000, Launchpad Bug Tracker wrote:
> [MIR] libfido2, libcbor, mathjax (dependencies of openssh)

Is it known that mathjax is strictly needed? The mathjax webpage says:
"Beautiful math in all browsers"
which lines up with my expectation that it's a documentation tool.

How is openssh using mathjax? Perhaps we can leave this in universe on the
"it doesn't affect runtime" rule.

Thanks

[Dmitry Shachnev (mitya57)]

According to https://bugs.launchpad.net/ubuntu/+source/libfido2/+bug/1864439/comments/1, mathjax should not be MIRed anymore.

[Seth Arnold (seth-arnold)]

Version 0.5.0+dfsg-2 of libcbor as packaged in focal appears to have significant unaligned data access problems.

The version in github appears to be fixed: see eg https://github.com/PJK/libcbor/commit/c745f6b88a739e700c6ea2baa96bcfef1d51cc0f

We need to update libcbor before we can use this library.

Thanks

[Andreas Hasenack (ahasenack)]

I'll take on libcbor

[Seth Arnold (seth-arnold)]

Thanks Andreas; while you're there, could you give a look to running the tests in test/ during the build or as autopkgtests?

Thanks

[Andreas Hasenack (ahasenack)]

Took note of it, let's see what I can do.

[Andreas Hasenack (ahasenack)]

I got the tests running at build time. I'm not doing some other improvements to the package and will have an MP up early next week (shooting for Monday).

Current branch, for the curious, is https://code.launchpad.net/~ahasenack/ubuntu/+source/libcbor/+git/libcbor/+ref/focal-libcbor-mir-effort

Note that this update will require a no-change rebuild of libfido.

[Andreas Hasenack (ahasenack)]

PPA with test packages: https://launchpad.net/~ahasenack/+archive/ubuntu/openssh-fido
(using focal-proposed)

[Andreas Hasenack (ahasenack)]

The soname changed significantly, I'm not yet sure how to handle it:
libcbor0: /usr/lib/x86_64-linux-gnu/libcbor.so.0
libcbor0: /usr/lib/x86_64-linux-gnu/libcbor.so.0.0.0

vs

/usr/lib/x86_64-linux-gnu/libcbor.so.0.6.0 (no .so.0 symlink).

$ objdump -x /usr/lib/x86_64-linux-gnu/libcbor.so.0.6.0 |grep SONAME
  SONAME libcbor.so.0.6.0

Technically we would have to rename the binary package to libcbor0.6.0. Upstream changed this in https://github.com/PJK/libcbor/issues/52, with this warning in the release notes:
"""
Correctly set .so version [Fixes #52].

    Warning: All previous releases will be identified as 0.0 by the linker.
"""

The commit is https://github.com/PJK/libcbor/commit/076b491e70cdf6557299727be69f5c44eaa4d7c6

I'm thinking:

"0.0.0" SOVERSION 0

should have been replaced by:

${CBOR_VERSION} SOVERSION 0

instead of just

${CBOR_VERSION}

Or some other number instead of "0". But his change was just ${CBOR_VERSION}, which made that the soname.

I added a comment/question at the end of https://github.com/PJK/libcbor/issues/52

[Andreas Hasenack (ahasenack)]

I guessed wrong how sonaming works in cmake files (ugh), but I amended my comment a bit.

[Andreas Hasenack (ahasenack)]

Ok, I tried a different diff[1], and got:
obj-x86_64-linux-gnu/src/libcbor.so
obj-x86_64-linux-gnu/src/libcbor.so.0.6.0
obj-x86_64-linux-gnu/src/libcbor.so.6

With:
$ objdump -x obj-x86_64-linux-gnu/src/libcbor.so.0.6.0 | grep SONAME
  SONAME libcbor.so.6

("6" was made up by me).

So question for the MIR team:
a) stick to upstream, with SONAME being libcbor.so.0.6.0 (full version, meaning a minor version update will require rdepends to be rebuilt)
b) like (a), but also change the binary package name from libcbor0 to libcbor0.6.0 (meaning NEW package every time this updates)
c) patch like above, making up a soname of our own (RISKY). Would still require a package rename, since we wouldn't be using libcbor.so.0 which was used in 0.5.0 which is in focal currently. Although dpkg-gensymbols would take care of adjusting the dependency I guess.

I'm inclined towards (b) as more correct than (a), and (c) only if upstream agrees to it and makes a new release with that change.

1. https://github.com/PJK/libcbor/issues/52#issuecomment-601913358

[Alex Murray (alexmurray)]

I reviewed libfido2 1.3.1-1build1 as checked into focal. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

libfido2 is a library used for communicating with FIDO U2F and FIDO 2.0
devices over USB, and for verifying associated attestation and assertion
signatures.

- CVE History:
  - No history of CVEs
- Build-Depends
  - libcbor-dev, libssl-dev, libudev-dev
- No pre/post inst/rm scripts
- No init scripts
- No systemd units
- No dbus services
- No setuid binaries
- binaries in PATH
  - /usr/bin/fido2-assert
  - /usr/bin/fido2-cred
  - /usr/bin/fido2-token
- No sudo fragments
- No polkit files
- No udev rules
- unit tests / autopkgtests
  - No autopkgtests
  - It looks like some tests are present in the source package in the
    regress/ directory but are not run during build
    - These should be run during the normal build process or at least run
      as autopkgtests
- No cron jobs
- Build logs are generally clean
  - One minor lintian fail:
  W: libfido2-doc: manpage-has-errors-from-man
  usr/share/man/man3/fido_bio_template.3.gz 41: warning [p 1, 6.8i]: can't
  break line

- No Processes spawned
- Fair amount of dynamic memory management but all look quite defensive and
  correct
- File IO
  - Opens hid input devices, these are enumerated via udev so file paths
    are sane
- Logging is careful, doesn't appear to be any lurking format string vulns
- No Environment variable usage
- Use of privileged functions
  - 2 uses of ioctl() on hid devices to read the hidraw report descriptor
    size, and then to read the actual report descriptor itself
- Use of cryptography / random number sources etc
  - Uses openssl to parse public key from X509 certificates in the public
    functions fido_cred_verify() etc
- No use of temp files
- No use of networking
- No use of WebKit
- No use of PolicyKit

- No cppcheck errors / warnings
- Unknown Coverity results
  - Currently waiting to perform Coverity analysis
- No significant shellcheck results
  - Only in the fuzz test and functional test driver code
- licensecheck notes that a lot of the code does not contain known license
  markers (however all seem to state that is covered by a BSD-style license
  defined in the LICENSE file)

libfido2 appears quite well written and defensive, and includes a fuzz and
regression test framework which provide confidence that it is likely to
have no low-hanging security bugs and that it would be possible to perform
tests when doing security updates in the future. If possible, it would be
best if these tests could be run during the build or as autopkgtests.

Security team ACK for promoting libfido2 to main once some
autopkgtests are added if possible to run the regression tests
or some similar tests.

[Christian Ehrhardt  (paelzer)]

@Andreas, from reading the upstream discussion it seems they want (until they reach 1.0) to stick with main soname 0 and only bump minor versions within it.

That in mind the new build should IMHO look like:

libcbor.so -> libcbor.so.0.6.0
libcbor.so.0 -> libcbor.so.0.6.0
libcbor.so.0.6.0

And the binary package would still be libcbor0

Only once a transition to 1.0 happens it would go to binary package libcbor1 then.

Version 0 comes with no guarantees, that means you don't need a rebuild to pick up a new dependency as it was and will stay libcbor0. But we should do a libfido2-1 rebuild against it in a ppa to ensure it works still - since no guarantees also means "there could be unexpected breakage".

Please reach out to me once you are online, I want to make sure we are on the same page here (not that I misses an important detail in your request or so).

[Andreas Hasenack (ahasenack)]

I'll check a new build with the patch from https://github.com/PJK/libcbor/pull/131/files

[Andreas Hasenack (ahasenack)]

With that patch, I get:

./obj-x86_64-linux-gnu/src/libcbor.so.0.6
./obj-x86_64-linux-gnu/src/libcbor.so
./obj-x86_64-linux-gnu/src/libcbor.so.0.6.0

$ objdump -x ./obj-x86_64-linux-gnu/src/libcbor.so.0.6.0 | grep SONAME
  SONAME libcbor.so.0.6

[Andreas Hasenack (ahasenack)]

MP for the libcbor update, satisfying the MIR requirements (tests, and updated version):

https://code.launchpad.net/~ahasenack/ubuntu/+source/libcbor/+git/libcbor/+merge/381060

[Andreas Hasenack (ahasenack)]

> Security team ACK for promoting libfido2 to main once some
> autopkgtests are added if possible to run the regression tests
> or some similar tests.

I changed the libfido2 package to run the regress/ tests at build time. Since they are very silent when they work, and only noisy when they fail, I also added a second run with an injected failure and verify that the failure indeed happens.

This is in https://code.launchpad.net/~ahasenack/ubuntu/+source/libfido2/+git/libfido2/+ref/focal-libfido2-enable-tests which I will MP shortly.

PPA: https://launchpad.net/~ahasenack/+archive/ubuntu/openssh-fido/+packages

Direct link to a build log. Search for "SUCCESS:": https://launchpadlibrarian.net/470547472/buildlog_ubuntu-focal-ppc64el.libfido2_1.3.1-1ubuntu1~ppa1_BUILDING.txt.gz

(ppc64el because that's the first build that finished in that ppa)

Too bad I'm doing multiple builds, but they don't take that long. The ppc build above took less than 3min.

[Christian Ehrhardt  (paelzer)]

Since fido2 is acked but waiting on some tests I'll mark the task incomplete and assign to Andreas.
Once we have tests added (which we can nicely combine with the libcbor rebuild) this would be ready.

I already see this in the PPA:
 * d/rules: run regression tests at build time, and one more time
    with an injected failure to verify it's caught.

Any chance to also make a autopkgtest out of it for fido? I know we talked about how to get a authenticating device in there - let us know what the last update on that is.

[Andreas Hasenack (ahasenack)]

> Any chance to also make a autopkgtest out of it for fido?

I tried adding the regress/ tests as dep8, but in the end it didn't make sense to me. Thinking about SRUs, the dep8 run is very late in the process, only triggered after the sru was accepted. Catching these failures at build time is much quicker.

> I know we talked about how to get a authenticating device in there -
> let us know what the last update on that is.

I searched for some sort of software-based fido device, like softhsm[1] for DNS servers, but didn't find anything I could use. If we want something like that, it will have to be in q-a-t. I can still commit to do it, but given the security team's request was to at least enable regress/, I did that first, but at build time. I'm not sure I can get the q-a-t run, with a real fido device using usb passthrough in a vm, ready before the next freeze this monday.

[Christian Ehrhardt  (paelzer)]

Thanks Andreas for working on this!
Having the new versions and the build tests enabled is already great.

Let me summarize the next steps:
- Andreas will try to add a half-manual test to qa-regression-test, this isn't bound to uploads and can happen later, but before FF release would be a soft-deadline IMHO
- Seth to ack if the tests we have enabled for libfido2 are ok to go on for now
- Seth to complete the security review on libcbor (0.6.0-0ubuntu1 is in focal proposed)

@Andreas/Seth - do you agree?

[Andreas Hasenack (ahasenack)]

I do

[Andreas Hasenack (ahasenack)]

libcbor and libfido2 with the changes requested here migrated to the focal release pocket.

[Seth Arnold (seth-arnold)]

I reviewed libcbor version 0.6.0-0ubuntu1 as checked into focal-proposed; for this latest look I only inspected the packaging, trusting that the code quality I saw earlier hasn't degraded. (Yes, the old version had alignment issues, but was otherwise in good shape.)

No full form since I'm on vacation today.

I liked the tests added for libfido2. Thanks for adding them.

I liked the packaging for the new version of libcbor. Thanks for the extensive improvements.

Security team ACK for promoting libcbor to main.

Thanks

[Christian Ehrhardt  (paelzer)]

Comments #2 #16 #27 combined mean this is complete and ready.

Andreas will continue to provide qa-regression tests for this, but this isn't gating this promotion. It shows up in component-mismatches already - therefore the right state is Fix-Committed.

This waits for an AA to promote the two packages.Subscribing ubuntu-archive to do so ...

[Colin Watson (cjwatson)]

Moved to main:

$ change-override -s focal -c main -t libcbor
Override component to main
libcbor 0.6.0-0ubuntu1 in focal: universe/misc -> main
Override [y|N]? y
1 publication overridden.
$ change-override -s focal -c main libcbor0.6 libcbor-dev
Override component to main
libcbor0.6 0.6.0-0ubuntu1 in focal amd64: universe/libs/optional/100% -> main
libcbor0.6 0.6.0-0ubuntu1 in focal arm64: universe/libs/optional/100% -> main
libcbor0.6 0.6.0-0ubuntu1 in focal armhf: universe/libs/optional/100% -> main
libcbor0.6 0.6.0-0ubuntu1 in focal i386: universe/libs/optional/100% -> main
libcbor0.6 0.6.0-0ubuntu1 in focal ppc64el: universe/libs/optional/100% -> main
libcbor0.6 0.6.0-0ubuntu1 in focal s390x: universe/libs/optional/100% -> main
libcbor-dev 0.6.0-0ubuntu1 in focal amd64: universe/libdevel/optional/100% -> main
libcbor-dev 0.6.0-0ubuntu1 in focal arm64: universe/libdevel/optional/100% -> main
libcbor-dev 0.6.0-0ubuntu1 in focal armhf: universe/libdevel/optional/100% -> main
libcbor-dev 0.6.0-0ubuntu1 in focal i386: universe/libdevel/optional/100% -> main
libcbor-dev 0.6.0-0ubuntu1 in focal ppc64el: universe/libdevel/optional/100% -> main
libcbor-dev 0.6.0-0ubuntu1 in focal s390x: universe/libdevel/optional/100% -> main
Override [y|N]? y
12 publications overridden.
$ change-override -s focal -c main -t libfido2
Override component to main
libfido2 1.3.1-1ubuntu2 in focal: universe/misc -> main
Override [y|N]? y
1 publication overridden.
$ change-override -s focal -c main libfido2-1 libfido2-dev libfido2-doc
Override component to main
libfido2-1 1.3.1-1ubuntu2 in focal amd64: universe/libs/optional/100% -> main
libfido2-1 1.3.1-1ubuntu2 in focal arm64: universe/libs/optional/100% -> main
libfido2-1 1.3.1-1ubuntu2 in focal armhf: universe/libs/optional/100% -> main
libfido2-1 1.3.1-1ubuntu2 in focal i386: universe/libs/optional/100% -> main
libfido2-1 1.3.1-1ubuntu2 in focal ppc64el: universe/libs/optional/100% -> main
libfido2-1 1.3.1-1ubuntu2 in focal s390x: universe/libs/optional/100% -> main
libfido2-dev 1.3.1-1ubuntu2 in focal amd64: universe/libdevel/optional/100% -> main
libfido2-dev 1.3.1-1ubuntu2 in focal arm64: universe/libdevel/optional/100% -> main
libfido2-dev 1.3.1-1ubuntu2 in focal armhf: universe/libdevel/optional/100% -> main
libfido2-dev 1.3.1-1ubuntu2 in focal i386: universe/libdevel/optional/100% -> main
libfido2-dev 1.3.1-1ubuntu2 in focal ppc64el: universe/libdevel/optional/100% -> main
libfido2-dev 1.3.1-1ubuntu2 in focal s390x: universe/libdevel/optional/100% -> main
libfido2-doc 1.3.1-1ubuntu2 in focal amd64: universe/doc/optional/100% -> main
libfido2-doc 1.3.1-1ubuntu2 in focal arm64: universe/doc/optional/100% -> main
libfido2-doc 1.3.1-1ubuntu2 in focal armhf: universe/doc/optional/100% -> main
libfido2-doc 1.3.1-1ubuntu2 in focal i386: universe/doc/optional/100% -> main
libfido2-doc 1.3.1-1ubuntu2 in focal ppc64el: universe/doc/optional/100% -> main
libfido2-doc 1.3.1-1ubuntu2 in focal s390x: universe/doc/optional/100% -> main
Override [y|N]? y
18 publications overridden.

[Andreas Hasenack (ahasenack)]

qa-regression-testing MP up at https://code.launchpad.net/~ahasenack/qa-regression-testing/+git/qa-regression-testing/+merge/381582