OpenSAML 3 Java - VU#475445 Workaround

As described in the vulnerability note VU#475445, OpenSAML 3 library (including Java library) is vulnerable to the assertions that use C14N canonicalization algorithm. Duo had found this CVE. A quick fix is below.

; ..
(:require [clojure.string :as str])
; ..

(def ^:dynamic *subject-val*)

(defn get-subject-from-node [nodes i c]
  (when (and (< i c) (not (realized? *subject-val*)))
    (let [node (.item nodes i)
          node-name (.getNodeName node)]
      (when (str/includes? node-name "Assertion")
        (get-subject-from-node (.getChildNodes node) 0 (.getLength (.getChildNodes node))))
      (when (str/includes? node-name "Subject")
        (get-subject-from-node (.getChildNodes node) 0 (.getLength (.getChildNodes node))))
      (when (str/includes? node-name "AuthenticationStatement")
        (get-subject-from-node (.getChildNodes node) 0 (.getLength (.getChildNodes node))))
      (when (str/includes? node-name "NameID")  ; SAML v2
        (deliver *subject-val* (.getTextContent node)))
      (when (str/includes? node-name "NameIdentifier")  ; SAML v1
        (deliver *subject-val* (.getTextContent node)))
      (get-subject-from-node nodes (inc i) c)))
  @*subject-val*)

This will extract subject from SAMLResponse in v1 or v2 format.

(defn get-subject [doc-elem assertion]
  (binding [*subject-val* (promise)]
    (let [name-id (.getNameID (.getSubject assertion))
          sub (.getValue name-id)
          resp-node (.getFirstChild (.getParentNode doc-elem))
          resp-node-childs (.getChildNodes resp-node)
          sub-cve (get-subject-from-node resp-node-childs 0 (.getLength resp-node-childs))]
      (if (= sub sub-cve) sub sub-cve))))

Here we proceed with the OpenSAML 3 unmarshalled object and obtained the subject value by calling (.getValue name-id). But since the library is vulnerable, we do a manual extraction of the subject value by calling (get-subject-from-node resp-node-childs 0 (.getLength resp-node-childs)). If they do not match, we return the sub-cve value as get-subject-from-node extracts the subject ignoring any comment added to the subject value.

Example

<saml:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">2<!-- foo -->1@example.com</saml:NameID></code>

The assertion can be replaced with a subject value as the above. Since C14N ignores comment, even if we modify the XML, to add comments after signing, the validation will ignore the added comments while recomputing the signature.

NB: the subject 2<!-- foo --> should not be in escaped form in the SAMLResponse.

With this, opensaml 3 will give us sub as 1@example.com and the workaround function will give sub-cve as 21@example.com.

Sidenote

; ...
(:import [javax.xml.parsers DocumentBuilderFactory]
         [org.xml.sax SAXException])
; ...

(def disallow-doctype-dec "http://apache.org/xml/features/disallow-doctype-decl")
(def external-parameter-entities "http://xml.org/sax/features/external-parameter-entities")
(def load-external-dtd "http://apache.org/xml/features/nonvalidating/load-external-dtd")

(defn get-doc-builder 
  "Returns a document builder, which should be called for each thread as parse is not thread safe"
  []
  (let [doc-builder-factory (DocumentBuilderFactory/newInstance)]
    (.setNamespaceAware doc-builder-factory true)
    ;; prevent XXE
    (.setFeature doc-builder-factory disallow-doctype-dec true)
    (.setFeature doc-builder-factory external-parameter-entities false)
    (.setFeature doc-builder-factory load-external-dtd false)
    (.setXIncludeAware doc-builder-factory false)
    (.setExpandEntityReferences doc-builder-factory false)
    (.newDocumentBuilder doc-builder-factory)))

(defn parse-xml
  "Parse the given xml which can be a input stream, file, URI."
  [xml]
  (try
    (.parse (get-doc-builder) xml)
    (catch SAXException excep
      (throw (ex-info "XML parse exception." {:cause [:err-xml-parse]})))))

Here we use JAXP parser to parse the XML and use OpenSAML 3 to do the unmarshalling of the parsed XML org.w3c.dom.Document to OpenSAML 3 objects for easy extraction and validation of the SAMLResponse.