Comment 6 for bug 1407695

I reviewed xmlsec1 version 1.2.20-2ubuntu1 as found in vivid. This
shouldn't be considered a full security audit but rather a quick gauge of
maintainability.

Two CVEs, one affecting many XML Security implementations, one specific
to the use of xslt in this library; both appeared to have been handled
quickly and with minimal patches available. While doing this review I
asked about the potential for the null prefix attack with the GnuTLS
and OpenSSL backends, and the author responded immediately, performed
his own review quickly, and responded clearly.

- xmlsec1 provides an XML Security interface to be used by other programs.
- Build-depends: debhelper, dh-autoreconf, chrpath, pkg-config,
  libxml2-dev, libxslt1-dev, libssl-dev, libgnutls28-dev, libnss3-dev,
  libgcrypt20-dev
- Requires GnuTLS, OpenSSL, and NSS.
- Does not itself do networking.
- Does not daemonize
- No customized pre/post inst/rm
- No init scripts
- No dbus services
- No setuid executables
- xmlsec1 and xmlsec1-config executables
- No sudo fragments
- No udev rules
- Extensive tests run during the build
- No cronjobs
- Some build warnings, mismatched types in printf format strings

- No subprocesses spawned
- Memory management looked careful
- Files to be opened are passed by clients
- Logging looked careful, extensive error reporting mechanisms
- No use of environment variables
- No privileged operations
- Extensive use of cryptography, only spot checks were possible, skipped
  all MS Cryptography; the checked sites looked careful.
- Only networking was in an example server not used in binary packaging
- No temporary file handling
- Does not use WebKit
- Does not use qtjsbackend
- Does not use PolicyKit
- Mostly clean cppcheck

This code is fairly dense and technically demanding as it lives in the
intersection of both XML and cryptography. The style is highly disciplined
with excellent error checking throughout, asserts and "friendlier" error
mechanisms are used extensively.

- gcrypt/hmac.c xmlSecGCryptHmacExecute() updates dgstSize rather than
  ctx->dgstSize -- is this correct?
- xkms-server.c main() fprintf format string bug
- dl.c getFunctions may be used uninitialized
- Some malloc() calls are casted, this isn't needed if a prototype is
  visible.
- xmlSecGnuTLSX509FindCert(), xmlSecOpenSSLX509FindCert(), among others,
  assume that subjectName, issuerName, are standard C strings rather than
  byte strings with a length attached. Author says this isn't a bug,
  the same user supplies both certificates and inputs.

Security team ACK for promoting xmlsec1 to main.

Thanks