I reviewed oath-toolkit version 2.6.1-1.2 as checked into cosmic. This
isn't a full security audit but rather a quick gauge of maintainability.
I especially did not audit the cryptography or xmlsec containers.

- seems dead upstream -- more than two years since the last checkin, which
  failed CI
- gitorious.org url in the README is dead
- FTBFS for me
- Large amount of very fragile-looking vendored code
- Appears to log passwords

- One CVE in our database: reported in December 2013 and fixed in
  February 2014.


- oath-toolkit provides totp, hotp, and Portable Symmetric Key Container
  (PSKC) tools -- library, PAM module, utilities
- links against xmlsec1; includes md5, sha1, sha2, sha3, aes, perhaps
  other algorithms
- Build-Depends: cdbs, debhelper, libpam0g-dev, datefudge, gtk-doc-tools,
  dblatex, libxml2-utils, libxmlsec1-dev, dh-autoreconf
- Does not daemonize
- No networking
- postinst/prerm scripts run pam-auth-update, ldconfig
- No initscript
- No systemd unit files
- No setuid files
- oathtool executable in the path
- No sudo fragments
- No udev rules
- Some tests are run as part of the build, including the gl/ directories.
  Tests look like smoke-tests rather than full regressions.
- No cron jobs
- No systemd timers
- Build logs show incorrect code in tests

- No subprocesses spawned
- The gl/ memory allocation routines are way too fragile and intricate.
  The memory management in the rest of the program is mixed, there's more
  integer multiplicates handed to malloc(3) than I like to see, but the
  ones I inspected all counted a number of objects first, which likely
  makes them safe, if not ideal.
- Logging is mixed -- these tools may log passwords (!) while in use.
  (The format strings are in the binaries. This is troubling.)
- No use of environment
- No privileged syscalls
- I did not inspect the cryptographic primitives or protocols
- I did not notice privileged portions of the code
- No temporary files
- No WebKit
- No PolicyKit
- cppcheck reports several issues, some in gl/tests/ and two false
  positives in gl/xmalloc (cppcheck doesn't understand the intention of
  the routines).

- query_prompt is allocated too short, no space is set aside for a NUL
  terminating byte. This may need a CVE.

- MAX_OTP_LEN check doesn't *always* constrain the length of a password,
  it feels likely to be a mistake. This may need a CVE.

- This appears to log a password: DBG (("conv returned: %s", resp->resp));
- This appears to log a password: DBG (("OTP too short: %s", password));
- This appears to log a password: DBG (("OTP shorter than digits=%d: %s", cfg.digits, password));
- This appears to log a password: DBG (("OTP too long (and no digits=): %s", password));
- This appears to log a password: DBG (("Password: %s ", onlypasswd));
- This appears to log a password: DBG (("authenticate rc %d (%s: %s) last otp %s", rc,
- ./oathtool/oathtool.c appears to log passwords:
  - "password \"%s\" not found in range %ld .. %ld",
  - "password \"%s\" not found in range %ld .. %ld",

This may need a CVE.

- gengetopt feels awkward at best:
  "replacement of strdup, which is not standard" -- strdup is in posix.1-2001
  so I have severe doubts that this tool is maintained

Gnulib vendored code, perhaps other sources too:

- There's a huge amount of copied source code that looks extremely brittle
  and prone to failure, prone to needing maintenance, and might be vastly
  out of date compared to recent versions of wherever they were stolen
  from:
  - xnmalloc() and XNMALLOC() have integer overflow problems; use calloc()
    instead. Or remove entirely, as nothing uses these.
  - YYCOPY()'s definition with __builtin_memcpy() may have integer overflow
    problem
  - get_tz() is grossly overcomplicated
  - parse_datetime() is grossly overcomplicated in part because get_tz() is
    overcomplicated
  - parse_datetime() may fail with input such as TZ="""""""""...
    I suspect this function is not suitable for untrusted inputs.
  - malloca looks brittle and difficult to debug
  - environment handling looks brittle and difficult to debug


I do not like the current state of this package. The vast amount of
vendored code is troubling. The upstream appears abandoned. When it wasn't
abandoned, a security fix took three months to integrate. The fact that
passwords can be easily logged suggests no one serious has deployed this.
Years without rebuilds (and frequent FTBFSs) suggest no one else cares
about this package.

At a minimum, the FTBFS needs to be fixed before we can support this
package. (This looks to be in progress.)

I would *really* prefer to see all the gl/ code patched out.

I'd be happiest if we could find a solution for ceph to *not* use this
library.

So no ACK or NAK yet, I'm hoping someone has great ideas here that can
avoid the question.

Thanks