Comment 2 for bug 1610286

I reviewed libapache2-mod-auth-mellon version 0.12.0-1 as checked into
zesty. This should not be considered a full security audit but rather a
quick gauge of maintainability.

- Four previous CVEs were reported against this module. While this is
  unfortunate I don't think it's unduly distressing.

- libapache2-mod-auth-mellon provides an authn and authz interface for
  Apache that can perform queries against a centralized SAML IdP.
- Build-Depends: debhelper, autotools-dev, dh-apache2, apache2-dev,
  libcurl3-dev, liblasso3-dev
- Does not itself daemonize
- pre/post inst/rm are automatically generated
- No initscripts
- No dbus services
- No setuid files
- No binaries in the path
- No sudo fragments
- No udev rules
- No test suite
- No cron jobs
- Clean build logs
- Clean cppcheck

- No subprocesses spawned
- Memory manage is usual for apache modules
- File IO is under control of configuration files
- Logging is very extensive
- Environment variables can be read as directed by configuration files
- No privileged system calls
- Does use curl to download data
- No obviously privileged portions of the code
- The use of /var/tmp/mellonLock should be changed
- No WebKit
- No JavaScript
- No PolicyKit

Here's some notes I took while reading the code; Olav will release a
new version soon with these issues addressed. Olav was responsive and
thoughtful.

- am_postdir_cleanup() could be extremely expensive if it had to walk a
  directory of twenty thousand saved requests. It has no mechanism to
  bail after a certain amount of work. am_save_post() appears to call
  am_postdir_cleanup() unconditionally on every saved post. This has
  the potential to have spiraling costs until finally all threads are
  spending all their time re-walking the same pile of files looking for
  old ones to delete.
  [The default setting is 100.]
- am_hc_block_write() is tail-recursive. If the compiler fails to optimize
  the tail call away, the call depth might wind up blowing out the stack
  frame available to the thread. How large is this stack frame? What is
  the largest amount of data that curl can be expected to retrieve? Having
  a thousand little 1012 byte structures in memory just to handle a one
  megabyte download sounds suboptimal.
  [Curl currently calls this function with no more than 16kB blocks, the
  responses from auth servers are typically < 4kB.]
- auth_mellon_commands claims the lockfile path is /tmp/mellonLock -- this
  is not a safe default if it's correct. (I think it's no longer correct,
  but /var/tmp/mellonLock isn't better -- someplace only the webserver
  user can write to would be ideal. mellonPost directory perhaps?)
  [Not currently used on Linux,

Security team ACK for promoting libapache2-mod-auth-mellon to main.

Thanks