Comment 6 for bug 2070025

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Review for Source Package: wsdd

[Summary]
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, so I'll assign ubuntu-security

List of specific binary packages to be promoted to main: wsdd
Specific binary packages built, but NOT to be promoted to main: wsdd-server

Notes:
Required TODOs:
- #1 The build time tests you added are nice, but they just check for a few
  known regressions. Is there any chance to test for the function that you
  want to support from nautilus in an autopkgtest? We'd not mind where that
  is implemented, e.g. if you say this is easier from nautilus than from wsdd
  that would be ok.
  You are the experts, so feel free to correct me. But I'd think of something
  like setting up samba + wsdd-server on one side of a network and
  nautilus + wsdd on the other checking if it finds the shares. The samba
  tests themselve do a lot of this, feel free to copy what you need.
  (this one is already on your known to-do list in your report, thanks!)
- #2 Please consider adding a d/watch file
  (this one is already on your known to-do list in your report, thanks!)
- #3 Given the known security issues that even the package description states
  and furthermore the use in user context suggests that we should add a pretty
  strict apparmor profile before we promote it. Could you please have a look
  at that?
- #4 I failed to see how this will be used from nautilus, which makes it hard
  to rate the risk and exposure. I assume the security team wants to know
  as well. To avoid everyone searching for that, would you mind outlining how
  the "wsdd is required in Ubuntu main for enabling win10 shares discovery
  in nautilus" described by you will work in detail? Which component will call
  which other in which environment and with which options to achieve that?

Recommended TODOs:
- #4 Just to be nice and to further add on the "should be behind firewall" case,
  upstream provides example config for ufw and firewalld. It might make sense
  to ship those as part of wsdd which would also fix
  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1073265

[Rationale, Duplication and Ownership]
There is no other package in main providing the same functionality.
There is wsdd2, obviously similar but less maintained and in universe as well.

A team is committed to own long term maintenance of this package => Desktop

The rationale given in the report seems valid and useful for Ubuntu.

[Dependencies]
OK:
- no other Dependencies to MIR due to this. wsdd-server is held back by choice
  not by dependencies.
- no -dev/-debug/-doc packages that need exclusion
- 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
- no static linking (python)
- does not have unexpected Built-Using entries
- not a go package, no extra constraints to consider in that regard
- not a rust package, no extra constraints to consider in that regard

Problems: None

[Security]
OK:
- does not run a daemon as root (this is only for wsdd)
- does not use webkit1,2
- does not use lib*v8 directly
- does not expose any external endpoint (port/socket/... or similar)
- 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)
- does not deal with cryptography (en-/decryption, certificates,
  signing, ...)

Problems:
- does parse data formats (the protocol messages are from anywhere on the
  local network) from an untrusted source.
- history of CVEs does not look concerning. So why is it a problem then. This
  is the history of wsdd as a package and there it is true. But the protocol
  and concept has issues. Look no further than the package description which
  says "It DOES have security issues, but it is designed for use in a trusted
  environment inside a firewall."
  I think we can not guarantee that this runs inside a firewall :-/ What now?
  I could think of apparmor isolation, of running it in a container, ...?
  Either way it needs securities POV if this is tolerable or not - and which
  mitigations would be nice to have or mandatory.

- I think this does not yet make appropriate (for its exposure) use of
  established risk mitigation features.
  There is an example systemd service which is not shipped that does well in
  using various risk mitigations like DynamicUser (implies many other private
  env namespacing), RuntimeDirectory, and dropping privileges. That is part
  of wsdd-server which isn't in discussion here right now. But the involvement
  of wsdd in nautilus (intended use case) or in general (direct call by the user
  is not guarded in any way). That in contrast to the known security issues
  which state "designed for use in a trusted environment inside a firewall"
  which I think we can not guarantee implies that /usr/bin/wsdd should get
  a pretty strict apparmor profile. Its function should be fine with that
  and that could be our way to consider it "a trusted environment".

[Common blockers]
OK:
- does not FTBFS currently
- does have a test suite that runs at build time (added in Oracular
  in 2:0.8-1ubuntu1)
  - test suite fails will fail the build upon error.
- This does not need special HW for build or test
- no new python2 dependency
- Python package, but using dh_python

Problems:
- Does not have a non-trivial test suite that runs as autopkgtest.
  That would not be an invalidation on its own, but also the build time
  regression tests are just two runs against two past mistakes, not a full
  feature check or anything close.

[Packaging red flags]
OK:
- Ubuntu does carry a delta, but it is reasonable and maintenance under
  control (adding testing)
- symbols tracking not applicable for this kind of code.
- Upstream update history is good (in bursts but ok)
- Debian/Ubuntu update history is good (following upstream)
- 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
- debian/rules is rather clean
- It is not on the lto-disabled list

Problems:
- debian/watch is not present andbut also not needed (e.g. native)

[Upstream red flags]
OK:
- no Errors/warnings during the build
- no incautious use of malloc/sprintf (python, no natice code)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (usage is OK inside
  tests)
- no use of setuid / setgid
- no important open bugs (crashers, etc) in Debian or Ubuntu
- no dependency on webkit, qtwebkit or libseed
- not part of the UI for extra checks (only nautilus itself later)

Problems: None