Merge pull request #190 from jonlanglois/topic/enforce-encrypted-cond…

…itions Topic/enforce encrypted conditions
Clever · Oct 16, 2019 · ae0da4d · ae0da4d
2 parents ca2fdfd + a7db222
commit ae0da4d
Show file tree

Hide file tree

Showing 9 changed files with 435 additions and 37 deletions.
diff --git a/lib/saml2.coffee b/lib/saml2.coffee
@@ -422,11 +422,12 @@ add_namespaces_to_child_assertions = (xml_string) ->
 # Takes a DOM of a saml_response, private keys with which to attempt decryption and the
 # certificate(s) of the identity provider that issued it and will return a user object containing
 # the attributes or an error if keys are incorrect or the response is invalid.
-parse_authn_response = (saml_response, sp_private_keys, idp_certificates, allow_unencrypted, ignore_signature, require_session_index, cb) ->
+parse_authn_response = (saml_response, sp_private_keys, idp_certificates, allow_unencrypted, ignore_signature, require_session_index, ignore_timing, notbefore_skew, sp_audience, cb) ->
   user = {}
 
   async.waterfall [
     (cb_wf) ->
+      # Decrypt the assertion
       decrypt_assertion saml_response, sp_private_keys, (err, result) ->
         return cb_wf null, result unless err?
         return cb_wf err, result unless allow_unencrypted and err.message == "Expected 1 EncryptedAssertion; found 0."
@@ -435,6 +436,7 @@ parse_authn_response = (saml_response, sp_private_keys, idp_certificates, allow_
           return cb_wf new Error("Expected 1 Assertion or 1 EncryptedAssertion; found #{assertion.length}")
         cb_wf null, assertion[0].toString()
     (result, cb_wf) ->
+      # Validate the signature
       debug result
       if ignore_signature
         return cb_wf null, (new xmldom.DOMParser()).parseFromString(result)
@@ -450,25 +452,54 @@ parse_authn_response = (saml_response, sp_private_keys, idp_certificates, allow_
 
         for sd in signed_data
           signed_dom = (new xmldom.DOMParser()).parseFromString(sd)
+
           assertion = signed_dom.getElementsByTagNameNS(XMLNS.SAML, 'Assertion')
           if assertion.length is 1
             return cb_wf null, signed_dom
+
+          encryptedAssertion = signed_dom.getElementsByTagNameNS(XMLNS.SAML, 'EncryptedAssertion')
+          if encryptedAssertion.length is 1
+            return decrypt_assertion saml_response, sp_private_keys, (err, result) ->
+              return cb_wf null, (new xmldom.DOMParser()).parseFromString(result) unless err?
+              return cb_wf err
         return cb_wf new Error("Signed data did not contain a SAML Assertion!")
       return cb_wf new Error("SAML Assertion signature check failed! (checked #{idp_certificates.length} certificate(s))")
     (decrypted_assertion, cb_wf) ->
-      try
-        session_info = get_session_info decrypted_assertion, require_session_index
-        user.name_id = get_name_id decrypted_assertion
-        user.session_index = session_info.index
-        if session_info.not_on_or_after?
-          user.session_not_on_or_after = session_info.not_on_or_after
-
-        assertion_attributes = parse_assertion_attributes decrypted_assertion
-        user = _.extend user, pretty_assertion_attributes(assertion_attributes)
-        user = _.extend user, attributes: assertion_attributes
-        cb_wf null, { user }
-      catch err
-        return cb_wf err
+      # Validate the assertion conditions
+      conditions = decrypted_assertion.getElementsByTagNameNS(XMLNS.SAML, 'Conditions')[0]
+      if conditions?
+        if ignore_timing != true
+          for attribute in conditions.attributes
+            condition = attribute.name.toLowerCase()
+            if condition == 'notbefore' and Date.parse(attribute.value) > Date.now() + (notbefore_skew * 1000)
+              return cb_wf new SAMLError('SAML Response is not yet valid', {NotBefore: attribute.value})
+            if condition == 'notonorafter' and Date.parse(attribute.value) <= Date.now()
+              return cb_wf new SAMLError('SAML Response is no longer valid', {NotOnOrAfter: attribute.value})
+
+        audience_restriction = conditions.getElementsByTagNameNS(XMLNS.SAML, 'AudienceRestriction')[0]
+        audiences = audience_restriction?.getElementsByTagNameNS(XMLNS.SAML, 'Audience')
+        if audiences?.length > 0
+          validAudience = _.find audiences, (audience) ->
+            audienceValue = audience.firstChild?.data?.trim()
+            !_.isEmpty(audienceValue?.trim()) and (
+              (_.isRegExp(sp_audience) and sp_audience.test(audienceValue)) or
+              (_.isString(sp_audience) and sp_audience.toLowerCase() == audienceValue.toLowerCase())
+            )
+          if !validAudience?
+            return cb_wf new SAMLError('SAML Response is not valid for this audience')
+      return cb_wf null, decrypted_assertion
+    (validated_assertion, cb_wf) ->
+      # Populate attributes
+      session_info = get_session_info validated_assertion, require_session_index
+      user.name_id = get_name_id validated_assertion
+      user.session_index = session_info.index
+      if session_info.not_on_or_after?
+        user.session_not_on_or_after = session_info.not_on_or_after
+
+      assertion_attributes = parse_assertion_attributes validated_assertion
+      user = _.extend user, pretty_assertion_attributes(assertion_attributes)
+      user = _.extend user, attributes: assertion_attributes
+      cb_wf null, { user }
   ], cb
 
 parse_logout_request = (dom) ->
@@ -605,35 +636,16 @@ module.exports.ServiceProvider =
 
               response.type = 'authn_response'
 
-              conditions = saml_response.getElementsByTagNameNS(XMLNS.SAML, 'Conditions')[0]
-              if conditions?
-                if options.ignore_timing != true
-                  for attribute in conditions.attributes
-                    condition = attribute.name.toLowerCase()
-                    if condition == 'notbefore' and Date.parse(attribute.value) > Date.now() + (options.notbefore_skew * 1000)
-                      return cb_wf new SAMLError('SAML Response is not yet valid', {NotBefore: attribute.value})
-                    if condition == 'notonorafter' and Date.parse(attribute.value) <= Date.now()
-                      return cb_wf new SAMLError('SAML Response is no longer valid', {NotOnOrAfter: attribute.value})
-
-                audience_restriction = conditions.getElementsByTagNameNS(XMLNS.SAML, 'AudienceRestriction')[0]
-                audiences = audience_restriction?.getElementsByTagNameNS(XMLNS.SAML, 'Audience')
-                if audiences?.length > 0
-                  validAudience = _.find audiences, (audience) ->
-                    audienceValue = audience.firstChild?.data?.trim()
-                    !_.isEmpty(audienceValue?.trim()) and (
-                      (_.isRegExp(options.audience) and options.audience.test(audienceValue)) or
-                      (_.isString(options.audience) and options.audience.toLowerCase() == audienceValue.toLowerCase())
-                    )
-                  if !validAudience?
-                    return cb_wf new SAMLError('SAML Response is not valid for this audience')
-
               parse_authn_response(
                 saml_response,
                 [@private_key].concat(@alt_private_keys),
                 identity_provider.certificates,
                 options.allow_unencrypted_assertion,
                 options.ignore_signature,
                 options.require_session_index,
+                options.ignore_timing,
+                options.notbefore_skew,
+                options.audience
                 cb_wf)
 
             when saml_response.getElementsByTagNameNS(XMLNS.SAMLP, 'LogoutResponse').length is 1

diff --git a/test/data/response_notbefore_future_encrypted.xml b/test/data/response_notbefore_future_encrypted.xml
@@ -0,0 +1 @@
+PHNhbWxwOlJlc3BvbnNlIHhtbG5zOnNhbWxwPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6cHJvdG9jb2wiIHhtbG5zOnNhbWw9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphc3NlcnRpb24iIElEPSJfMiIgVmVyc2lvbj0iMi4wIiBJblJlc3BvbnNlVG89Il8xIiBEZXN0aW5hdGlvbj0iaHR0cHM6Ly9zcC5leGFtcGxlLmNvbS9hc3NlcnQiPg0KICAgIDxzYW1scDpTdGF0dXM+DQogICAgICAgIDxzYW1scDpTdGF0dXNDb2RlIFZhbHVlPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6c3RhdHVzOlN1Y2Nlc3MiLz4NCiAgICA8L3NhbWxwOlN0YXR1cz4NCiAgICANCjxzYW1sOkVuY3J5cHRlZEFzc2VydGlvbj48eGVuYzpFbmNyeXB0ZWREYXRhIHhtbG5zOnhlbmM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDEvMDQveG1sZW5jIyIgeG1sbnM6ZHNpZz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC8wOS94bWxkc2lnIyIgVHlwZT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS8wNC94bWxlbmMjRWxlbWVudCI+PHhlbmM6RW5jcnlwdGlvbk1ldGhvZCBBbGdvcml0aG09Imh0dHA6Ly93d3cudzMub3JnLzIwMDEvMDQveG1sZW5jI2FlczEyOC1jYmMiLz48ZHNpZzpLZXlJbmZvIHhtbG5zOmRzaWc9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvMDkveG1sZHNpZyMiPjx4ZW5jOkVuY3J5cHRlZEtleT48eGVuYzpFbmNyeXB0aW9uTWV0aG9kIEFsZ29yaXRobT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS8wNC94bWxlbmMjcnNhLW9hZXAtbWdmMXAiLz48eGVuYzpDaXBoZXJEYXRhPjx4ZW5jOkNpcGhlclZhbHVlPlduVU5MUnF0SEdrWE9uMXRKc2FWaDNBeXp2TDkrRnJKWWNoeE5BbkZDcGVWbG5nSC9oM3BSOFlLdG5BWEtmZHVSL2tVbXVRNFU4ZW14cDc3OXdjYzNqY2NEajJFcXZTUlhsNDM1MkhrQVV3T2Y2eWc3RURILzFKeFMrbHJRdURCd08yTCtmbVo1ckczdjdLeHlqc29GamMrQzhaNGNKaE5LWDcraFMyUWxBcHRDekNmNUhwaWk4TG9KVTk5bXUrZHJHckZZZ1A5SWV4QzZyWGRKSUNjM2NwWlh2d3ZKNDJYcXh5alNvTzU3OWFZTFdLZFdjdlVRVkhXaExIbTl4TTFkOVRDSGFRK0Q0NHlMM2xLUGViQVNMZGxwVWZ3S2V1UEs2MkZkN052ODVac2lWMm9WeEdzUmVmYk9zRk5hMEduTFNKSEpTYmxGYmZDQnlLaHJIV3BTQT09PC94ZW5jOkNpcGhlclZhbHVlPjwveGVuYzpDaXBoZXJEYXRhPjwveGVuYzpFbmNyeXB0ZWRLZXk+PC9kc2lnOktleUluZm8+DQogICA8eGVuYzpDaXBoZXJEYXRhPg0KICAgICAgPHhlbmM6Q2lwaGVyVmFsdWU+blVBYjk3SW0rbzBPN0pRd3V1R1dNdHpPR0hMZnFxVzVlVGVjejZYd0VldjBtQ3BsbHlRUzhHYnFIWHA1RWpVcXg1czZja3J5UXdKK2svT09uUTBFVnVCMStER1V6TmNjODJGWTF0NE9NRjJiRnVtUVBvK29GbUUyMWpHMnJabzhZSFdyMzNIVDNibzVYZUxPQ1JwTFRGRy9ZZ2lOSDRZN0dZblR4Sy9XZUtlN1JmY092QTZiWXBReWc0Qm1hdTZaeHBZaU42bmdMbFFqMmt4bFJHSG5yMWY0eFVGbjF0RVhvYThGTTNBTndiSlV2K1dyWEt4ZjlnZDNWSG9XL2VvVDVrK3pxLy9VejN4Wi9QTVhDa3loeGp0cmsxYWxkVTB5cEIxblV0UGo0N0lDNEJId2l1dEpneExMaTRrb2tRb0J0dU1PNzFyQ0c3bTFVUFI2Yy9idXlpaGMvZ0RuUTVWditzT3NndFA1NndoeTdoeVl1Y3hQRDUxMElSNlcySW5kMFp5eXNwcDRkM1JtZXBzSUZNRHNmcTE4VzZMWmFUQ1FSd0xyNTZORlJibjBaempEYy9GNEFhU1pleFdCTEg5VG9CcXdDVnpDNmorZjdGMWhEMHFQMU9jZ1dvcERFeHlTVllCNUtKQlIwUDNIRFBWdlg5VmNVN2lLVklQTUlRWEhVRnlMQjlZWjZBTCt4L1AvR2k1UG93MEh6bmU0TTFzQzk3eWc1RHU4WlQvcXM4cTBoVnJneTgweVR0Y3VwK1ZIZU96M3lxVWlWMDZJVlR1eStDRzZWbTR6am13RjB2b3BuanFKcElXRjMvN3ZZc1FmYlNJbld0OUdNdGYvQTgyNW4vRi8vMVpwRjFUL0pxTFRwR3NIczRRYmREdDJoR3pudThyWnpnMjZOT1dLR0wwVnZtdEJncmVWT3VnclYrV3FDNnVsZC9KWWFFZjFTSnNNOTZGcXdyME5tSXdWRFhJM09zZXc3QkRkTDVTRUV0UFRheHFXbVlybGpmRmZ0TXNLbGFBenU3SFpjU1V2Zi8wMTRobUJiaTBaY3N1cmVzQVBYMlFOcUJCaDJRYm95TS90VStxZSt2MXVsMTA3K2RLNFRnZU5oQWtsMHFnU1JsMmVNTkJDWUVPREZ0d0RmSVE3T3lLT1pFMzJsVzhmeGlMV3lleDdxczlDd2pCODRHeCtGMjVlSllPU0xJL0xvMUo4eE9McVN1UnVIYjRNNjR6bUZyalBENHdjcUZ2bTQ4d2UzWlVPVUVIREMyWDY4MjVoOWRvRGsrRDFtZFJoN3B5SEd0QmR6NTR4Tk5lajY5ZG1PNEJBcWRIZXhBNkNPTiszTndNYW02WmJoZm5RNkpscGVIUmtRZzkyeGh1d2NwWmo5bGpkSTl2bFM5T05XS09INGYwY3R1NEc3eFNjcGpDZkpBWlhuM2ZjeEg2U1VXY2g1UG1PYWFVazdCRmRLdTdzYitXcjNFWWsxd2F4aWtuNjNHdXJra1VhL2YrY1dDRWJZVXpZdWlHOUpkUVF1K1BHSmVjaXpKc21hL2lRTjl4NUxGUFNnM040L0RKZnVuRU9zN2tvVzhGR00zVWdnZ3h1N1VxY3lRNVphLzdVVmxoMmhtUFpsc09TNFlSNjh2VHFmaVYwQldsdmNoaE5ORTFxL3RYTWMxeHNhbGJVOS85UVcxcGxYTzMyb0F3ajVnSG0rcERSSUluakRKMDkxTFEzdnNweGVBbWJwOWJBYisyRC9DSXJaU21MWVhZMTY4aGFpVFFPYURHZXdtOElncm15UnhEWHkwWDFOU0FhL1U5Szd4cjZWNVhoaVVBVnVyVGN3Y1pnTVNOS29GMllwMlo5ajE4d0hpU09nd1Rna25UN0Z4K3BPMVdRMVBPVFJkN2JGa3RlMHR2a1BQaCttSk40cmN2N2ZpT3ljaEVnckdYNUo4YVp5ZUk5L2F6Z0RCVzdFdE1IRWNIakkyTFR0bEdXaG8wVjhBbzVFd09zd3lkRU5iTUFFNVBoWFhNc29kUXdadm1LZEt6U1AzWkR1VEhSUWVHcTdwc2NQc20xWERtZWxZOFFEeC84RUFpNzAwNy9leXA0Z1BKZVBTVlF3bHJGamFzS20rS1dQc1NqWG5CTGZ1TzVjOFZ6SDFhMDk5dlBxYTVZQzFQUUdPd3B4ZENxUG5HRmxuK2pTa05oaTNQcTZKNnZiTjVmbXBQS3VuWks3Y2E5b3VXNGlYRTBoUGhPY3l4bWZFeVV4R3NPQWtLSlBlZzdSVjRMUnpDNnB0cElDc0NXNU9PdW5nblhVSElWVnlRN0hXbDhBTStHclI2VjBidjBEcjh3U29IMmpjakRMclYxRHhtZktjd1kxV2JVNHZEWlBKYjFBZy85c2VJNEdwTmIyOFdmY3I5QUNhdlVsKzJEVjR2VXFKckRQRTFwRzFUVm1uUHhzQnNIbURiTThGL0FKUXRjRkNNYjRQcDdXbytrU3VWNVVibDlrR0VDMitGTkVTWlJsdlUxTVNNOFVBeGhkOHp0Tk1xcitVMTFDM2JlcWo2WjFpT1dDN21VU1dJMjByT2crTi9CY0pVUkZ4Mmo5QVpnS2J6dnBHcG4rSURSUXB2Sy9Saz08L3hlbmM6Q2lwaGVyVmFsdWU+DQogICA8L3hlbmM6Q2lwaGVyRGF0YT4NCjwveGVuYzpFbmNyeXB0ZWREYXRhPjwvc2FtbDpFbmNyeXB0ZWRBc3NlcnRpb24+PC9zYW1scDpSZXNwb25zZT4=
diff --git a/test/data/response_notbefore_future_encrypted_decoded.xml b/test/data/response_notbefore_future_encrypted_decoded.xml
@@ -0,0 +1,10 @@
+<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" ID="_2" Version="2.0" InResponseTo="_1" Destination="https://sp.example.com/assert">
+    <samlp:Status>
+        <samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/>
+    </samlp:Status>
+
+<saml:EncryptedAssertion><xenc:EncryptedData xmlns:xenc="http://www.w3.org/2001/04/xmlenc#" xmlns:dsig="http://www.w3.org/2000/09/xmldsig#" Type="http://www.w3.org/2001/04/xmlenc#Element"><xenc:EncryptionMethod Algorithm="http://www.w3.org/2001/04/xmlenc#aes128-cbc"/><dsig:KeyInfo xmlns:dsig="http://www.w3.org/2000/09/xmldsig#"><xenc:EncryptedKey><xenc:EncryptionMethod Algorithm="http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p"/><xenc:CipherData><xenc:CipherValue>WnUNLRqtHGkXOn1tJsaVh3AyzvL9+FrJYchxNAnFCpeVlngH/h3pR8YKtnAXKfduR/kUmuQ4U8emxp779wcc3jccDj2EqvSRXl4352HkAUwOf6yg7EDH/1JxS+lrQuDBwO2L+fmZ5rG3v7KxyjsoFjc+C8Z4cJhNKX7+hS2QlAptCzCf5Hpii8LoJU99mu+drGrFYgP9IexC6rXdJICc3cpZXvwvJ42XqxyjSoO579aYLWKdWcvUQVHWhLHm9xM1d9TCHaQ+D44yL3lKPebASLdlpUfwKeuPK62Fd7Nv85ZsiV2oVxGsRefbOsFNa0GnLSJHJSblFbfCByKhrHWpSA==</xenc:CipherValue></xenc:CipherData></xenc:EncryptedKey></dsig:KeyInfo>
+   <xenc:CipherData>
+      <xenc:CipherValue>nUAb97Im+o0O7JQwuuGWMtzOGHLfqqW5eTecz6XwEev0mCpllyQS8GbqHXp5EjUqx5s6ckryQwJ+k/OOnQ0EVuB1+DGUzNcc82FY1t4OMF2bFumQPo+oFmE21jG2rZo8YHWr33HT3bo5XeLOCRpLTFG/YgiNH4Y7GYnTxK/WeKe7RfcOvA6bYpQyg4Bmau6ZxpYiN6ngLlQj2kxlRGHnr1f4xUFn1tEXoa8FM3ANwbJUv+WrXKxf9gd3VHoW/eoT5k+zq//Uz3xZ/PMXCkyhxjtrk1aldU0ypB1nUtPj47IC4BHwiutJgxLLi4kokQoBtuMO71rCG7m1UPR6c/buyihc/gDnQ5Vv+sOsgtP56why7hyYucxPD510IR6W2Ind0Zyyspp4d3RmepsIFMDsfq18W6LZaTCQRwLr56NFRbn0ZzjDc/F4AaSZexWBLH9ToBqwCVzC6j+f7F1hD0qP1OcgWopDExySVYB5KJBR0P3HDPVvX9VcU7iKVIPMIQXHUFyLB9YZ6AL+x/P/Gi5Pow0Hzne4M1sC97yg5Du8ZT/qs8q0hVrgy80yTtcup+VHeOz3yqUiV06IVTuy+CG6Vm4zjmwF0vopnjqJpIWF3/7vYsQfbSInWt9GMtf/A825n/F//1ZpF1T/JqLTpGsHs4QbdDt2hGznu8rZzg26NOWKGL0VvmtBgreVOugrV+WqC6uld/JYaEf1SJsM96Fqwr0NmIwVDXI3Osew7BDdL5SEEtPTaxqWmYrljfFftMsKlaAzu7HZcSUvf/014hmBbi0ZcsuresAPX2QNqBBh2QboyM/tU+qe+v1ul107+dK4TgeNhAkl0qgSRl2eMNBCYEODFtwDfIQ7OyKOZE32lW8fxiLWyex7qs9CwjB84Gx+F25eJYOSLI/Lo1J8xOLqSuRuHb4M64zmFrjPD4wcqFvm48we3ZUOUEHDC2X6825h9doDk+D1mdRh7pyHGtBdz54xNNej69dmO4BAqdHexA6CON+3NwMam6ZbhfnQ6JlpeHRkQg92xhuwcpZj9ljdI9vlS9ONWKOH4f0ctu4G7xScpjCfJAZXn3fcxH6SUWch5PmOaaUk7BFdKu7sb+Wr3EYk1waxikn63GurkkUa/f+cWCEbYUzYuiG9JdQQu+PGJecizJsma/iQN9x5LFPSg3N4/DJfunEOs7koW8FGM3Ugggxu7UqcyQ5Za/7UVlh2hmPZlsOS4YR68vTqfiV0BWlvchhNNE1q/tXMc1xsalbU9/9QW1plXO32oAwj5gHm+pDRIInjDJ091LQ3vspxeAmbp9bAb+2D/CIrZSmLYXY168haiTQOaDGewm8IgrmyRxDXy0X1NSAa/U9K7xr6V5XhiUAVurTcwcZgMSNKoF2Yp2Z9j18wHiSOgwTgknT7Fx+pO1WQ1POTRd7bFkte0tvkPPh+mJN4rcv7fiOychEgrGX5J8aZyeI9/azgDBW7EtMHEcHjI2LTtlGWho0V8Ao5EwOswydENbMAE5PhXXMsodQwZvmKdKzSP3ZDuTHRQeGq7pscPsm1XDmelY8QDx/8EAi7007/eyp4gPJePSVQwlrFjasKm+KWPsSjXnBLfuO5c8VzH1a099vPqa5YC1PQGOwpxdCqPnGFln+jSkNhi3Pq6J6vbN5fmpPKunZK7ca9ouW4iXE0hPhOcyxmfEyUxGsOAkKJPeg7RV4LRzC6ptpICsCW5OOungnXUHIVVyQ7HWl8AM+GrR6V0bv0Dr8wSoH2jcjDLrV1DxmfKcwY1WbU4vDZPJb1Ag/9seI4GpNb28Wfcr9ACavUl+2DV4vUqJrDPE1pG1TVmnPxsBsHmDbM8F/AJQtcFCMb4Pp7Wo+kSuV5Ubl9kGEC2+FNESZRlvU1MSM8UAxhd8ztNMqr+U11C3beqj6Z1iOWC7mUSWI20rOg+N/BcJURFx2j9AZgKbzvpGpn+IDRQpvK/Rk=</xenc:CipherValue>
+   </xenc:CipherData>
+</xenc:EncryptedData></saml:EncryptedAssertion></samlp:Response>