Comment 1 for bug 1990582

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

Review for Package: thin

[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, but there are a few tasks which might
render the MIR cancelled once completed. So please first have a look at
the required TODOs. Once we know for sure that using another server
is no alternative, then we will enqueue it with security and you can
work at the same time on the further TODOs.

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

Required TODOs:
- Please have a POC try to see if PCS could run with establshed web-servers
  like apache/nginx, outline the drawbacks and problems to confirm the
  assumption that it won't work well.
  Or even better, find that it works well with those, adapt the packaging
  and cancel this MIR request.
- Please add an autopkgtest - this could, but does not need to happen here
  if you have a very elaborate test on pcs itself that really uses the server
  heavily please state so here and it shall be considered covered. But if not,
  then please add a dep8 test to "thin".
- Please ensure to Update to the latest version 1.8.1
- Please check, upstream updates rather rarely since 1.7 (3/2 years each).
  Are you sure the project is sufficiently active to rely on it?
  Please check the project and state what you have found.
Recommended TODOs:
- We'd like to promote this for ruby-embedded use cases right? Would it seem
  reasonable to you to to split the package into what can be used integrated
  (thin-bin) and the webserver that runs as a service (thin depending on
  thin-bin)? In that case we could leave "thin" in universe and thereby
  discourage the use of it as standalone server.
  If using apache/nginx in the first place fails then please have a look
  at this option.

[Duplication]
A web server, well surely we have those. But the use case as presented here
is not for a fully fledged webserver, but for an integrated one on PCS (see
linked bug). It allows a reduced feature set (and therefore attack surface)
compared to other servers and closely integrates e.g. Rack services which
are what usually is needed for ruby based web solutions.
Re-purposing pcs to use apache/nginx in main seems rather complex and not
apprpriate, but I'll ask the reporter to be sure it can't work well.
So although there are webservers, there is no other package in main providing
the very-same functionality, but I'd like to have a deeper check to prove it
can't work otherwise.

[Dependencies]
OK:
- There is a full dependency analysis in bug 1953341 and this is one of many.
  No "further" dependencies needed.
- 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
- 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:
- history of CVEs does not look concerning,
  but being a web-server I can't believe that - expect it isn't under that
  much scrutiny yet
- does not use webkit1,2
- does not use lib*v8 directly
- 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 run a daemon as root
- does parse data formats (files [images, video, audio,
  xml, json, asn.1], network packets, structures, ...) from
  an untrusted source.
- does open a port/socket
- does process arbitrary web content
=> None of the above is "wrong" but a webserver just is rather exposed by the
  nature of what it is doing.

[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.
    (via gem2deb-test-runner)
- no need for special HW
- no new python2 dependency

Problems:
- does not have a non-trivial test suite that runs as autopkgtest

[Packaging red flags]
OK:
- Ubuntu does carry a delta, but it is reasonable and maintenance under
  control.
  It was a test fix that is accepted in Debian, can be a sync again once
  LL opens.
- symbols tracking not applicable for this kind of code.
- d/watch is present and looks ok (if needed, e.g. non-native)
- Upstream update history is just barely ok (about yearly in the past, but then slower)
- promoting this does not seem to cause issues for MOTUs that so far
  maintained the package
- no massive Lintian warnings
- d/rules is rather clean
- It is not on the lto-disabled list

Problems:
- the current release is not packaged
- Debian/Ubuntu update history is too slow (1.5y behind)

[Upstream red flags]

OK:
- no Errors/warnings during the build
- no incautious use of malloc/sprintf (ruby)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (usage is OK inside
  tests)
- no use of user nobody (only in the examples)
- no use of setuid
- no important open bugs (crashers, etc) in Debian or Ubunt
- 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 (admin only)

Problems: None