Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using parseLoginResponse leads to typeError #222

Closed
laimikko1 opened this issue Oct 18, 2018 · 7 comments
Closed

Using parseLoginResponse leads to typeError #222

laimikko1 opened this issue Oct 18, 2018 · 7 comments

Comments

@laimikko1
Copy link

We are using Samlify as an SP and authenticating using a 3rd party idp solution.

At first I tried to set up our SP and idp as advised in the documentation

const sp = ServiceProvider({
  metadata: readFileSync('./our_sp_metada.xml'),
});

and idp using the metadata, returned to us by the WAYF/DS service that our idp provider uses

const samlify = require('samlify')
...
const metadata = await getMetadata(req.query.entityID)
const idp = IdentityProvider({
  metadata
});

This worked pretty far, as I was able to redirect our login request from our 3rd party WAYF/DS to the test idp login service, managed to authenticate and received a SAML response.
However, when trying to parse the login response using

router.post('/assert', async (req: Request, res: Response): Promise<void> => {
  try {
    const response = await sp.parseLoginResponse(idp, 'post', req)
    console.log(response)
  } catch (error) {
    console.log(error)
  } 

I catched the following error

backend | TypeError: Cannot read property 'map' of null
backend |     at /usr/src/app/node_modules/samlify/build/src/libsaml.js:304:49
backend |     at Array.forEach (<anonymous>)
backend |     at Object.verifySignature (/usr/src/app/node_modules/samlify/build/src/libsaml.js:289:23)
backend |     at /usr/src/app/node_modules/samlify/build/src/flow.js:169:48
backend |     at step (/usr/src/app/node_modules/samlify/build/src/flow.js:32:23)
backend |     at Object.next (/usr/src/app/node_modules/samlify/build/src/flow.js:13:53)
backend |     at fulfilled (/usr/src/app/node_modules/samlify/build/src/flow.js:4:58)
backend |     at <anonymous>
backend |     at process._tickCallback (internal/process/next_tick.js:189:7)

After a bit of head scratching I realised that the assertion of the response is probably both signed and encrypted. I then followed the Samlify documentation and modified my sp

const sp = samlify.ServiceProvider({
  metadata: fs.readFileSync('./our_metadata/metadata.xml'),
  encPrivateKey: fs.readFileSync('./our_cert_key.pem'),
  privateKey: fs.readFileSync('./our_cert_key.pem')
})

And added

<SPSSODescriptor 
    WantAssertionsSigned="true" 
    protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">

to my SP metadata.
(We are using the same certificate for everything in our testing environment.)

I then tried to parse the login response but received the same error as above.

Here's our idp and SP logged:

IdentityProvider {
  entitySetting: 
   { wantLogoutResponseSigned: false,
     messageSigningOrder: 'sign-then-encrypt',
     wantLogoutRequestSigned: false,
     allowCreate: false,
     isAssertionEncrypted: false,
     requestSignatureAlgorithm: 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256',
     dataEncryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#aes256-cbc',
     keyEncryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#rsa-1_5',
     generateID: [Function: generateID],
     relayState: '',
     wantAuthnRequestsSigned: false,
     tagPrefix: { encryptedAssertion: 'saml' },
     metadata: '<?xml version="..."</EntityDescriptor>' },
  entityMeta: 
   IdpMetadata {
     xmlString: '<?xml version="1.0" encoding="UTF-8" ...</EntityDescriptor>',
     meta: 
      { wantAuthnRequestsSigned: undefined,
        singleSignOnService: [Object],
        entityDescriptor: '<EntityDescriptor .... </EntityDescriptor>',
        entityID: '[entityID of our idp]',
        certificate: {},
        singleLogoutService: [Object],
        nameIDFormat: [Array] } } }

What stands out is the empty certificate in meta information.

ServiceProvider {
  entitySetting: 
   { wantLogoutResponseSigned: false,
     messageSigningOrder: 'sign-then-encrypt',
     wantLogoutRequestSigned: false,
     allowCreate: false,
     isAssertionEncrypted: false,
     requestSignatureAlgorithm: 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256',
     dataEncryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#aes256-cbc',
     keyEncryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#rsa-1_5',
     generateID: [Function: generateID],
     relayState: [frontend login page url],
     authnRequestsSigned: false,
     wantAssertionsSigned: true,
     wantMessageSigned: false,
     metadata: <Buffer ... >,
     encPrivateKey: <Buffer ... >,
     privateKey: <Buffer  ... > },
  entityMeta: 
   SpMetadata {
     xmlString: '<?xml version="1.0" ...</EntityDescriptor>\n',
     meta: 
      { spSSODescriptor: [Object],
        assertionConsumerService: [Object],
        entityDescriptor: '<EntityDescriptor xmlns="urn:oasis:names:tc:SAML:2.0:metadata" ...</EntityDescriptor>',
        entityID: '[our_SP_entityID]',
        certificate: [Object],
        singleLogoutService: [],
        nameIDFormat: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient' } } }

Our test idp metadata is public and found at
https://testidp.funet.fi/idp/shibboleth

Our SP metadata is published at
https://defa-staging.cs.helsinki.fi/api/login/metadata

If you require other information, feel free to ask and I'll try to provide it. Thanks!

@tngan
Copy link
Owner

tngan commented Oct 18, 2018

@laimikko1 After a quick review, there are mainly two problems.

When you construct the IdP, please also set isAssertionEncrypted to be true.
I have found there is a mistake in the implementation. The use in KeyDescriptor node should be optional mentioned in the specification, but here I implement it to be required, since the one in your IdP metadata doesn't contain the use property, so the metadata parser gets empty object for certificate.

I will fix the parser later on. Meanwhile, you can configure your IdP metadata as follow and continue your internal testing.

<KeyDescriptor use="signing">
  <KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
    <X509Data>
      <X509Certificate>...</X509Certificate>
    </X509Data>
  </KeyInfo>
</KeyDescriptor>
<KeyDescriptor use="encryption">
  <KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
    <X509Data>
      <X509Certificate>...</X509Certificate>
    </X509Data>
  </KeyInfo>
</KeyDescriptor>

@tngan
Copy link
Owner

tngan commented Oct 20, 2018

The latest version should support the certificate declaration with/without declaring use.

@laimikko1
Copy link
Author

Thanks for the response and patch.
There were few errors on my end that caused issues.

  1. The nameIDPolicy wasn't correctly configured, so the SAML response was in error state due to it. I added the following to SP
const sp = samlify.ServiceProvider({
  metadata: fs.readFileSync('./src/utils/metadata.xml'),
  encPrivateKey: fs.readFileSync('./src/utils/key.pem'),
  privateKey: fs.readFileSync('./src/utils/key.pem'),
  loginNameIDFormat: "transient",
})

and it now returns a valid response.

  1. Our idp signs the response and assertion and also encrypts the assertion. I followed the documentation and added
idp = samlify.IdentityProvider({
    metadata: fs.readFileSync('./src/utils/idp-metadata.xml'),
    isAssertionEncrypted: true,
    wantMessageSigned: true,
    signatureConfig: {
      prefix: 'ds',
      location: {
        reference: '/samlp:Response/saml:Issuer',
        action: 'after'
    }
  }
})

Idp also expects us to sign our auth request so I modified my SP metadata to match that.
The SP metadata is still public
https://defa-staging.cs.helsinki.fi/api/login/metadata

I'm still using the test idp metadata. I've modifed it a bit as you instructed. It can be seen below

<?xml version="1.0" encoding="UTF-8" standalone="no"?><EntityDescriptor xmlns="urn:oasis:names:tc:SAML:2.0:metadata" xmlns:mdui="urn:oasis:names:tc:SAML:metadata:ui" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" entityID="[entityID]" validUntil="2018-10-22T04:50:20+03:00" xsi:schemaLocation="urn:oasis:names:tc:SAML:2.0:metadata saml-schema-metadata-2.0.xsd urn:mace:shibboleth:metadata:1.0 shibboleth-metadata-1.0.xsd http://www.w3.org/2000/09/xmldsig# xmldsig-core-schema.xsd"><ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
<ds:SignedInfo>
<ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
<ds:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/>
<ds:Reference URI="">
<ds:Transforms>
<ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>
<ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
</ds:Transforms>
<ds:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/>
<ds:DigestValue>QwoRLp4+XrbRpHtVDU3Jew/jupDFBgCnxv43KlryQmk=</ds:DigestValue>
</ds:Reference>
</ds:SignedInfo>
<ds:SignatureValue>
[JnKk...]
</ds:SignatureValue>
<ds:KeyInfo>
<ds:KeyValue>
<ds:RSAKeyValue>
<ds:Modulus>
[0uxae...]
</ds:Modulus>
<ds:Exponent>AQAB</ds:Exponent>
</ds:RSAKeyValue>
</ds:KeyValue>
<ds:X509Data>
<ds:X509Certificate>
[...ilNkbg==]
</ds:X509Certificate>
</ds:X509Data>
</ds:KeyInfo>
</ds:Signature>
  <IDPSSODescriptor xmlns:ds="http://www.w3.org/2000/09/xmldsig#" WantAuthnRequestsSigned="true" xmlns:shibmd="urn:mace:shibboleth:metadata:1.0" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
    <Extensions>
      <shibmd:Scope regexp="false">funet.fi</shibmd:Scope>
      <shibmd:Scope regexp="false">yliopisto.fi</shibmd:Scope>
      <mdui:UIInfo>
          ...
      </mdui:UIInfo>
    </Extensions>
    <KeyDescriptor use="signing">
      <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
        <ds:X509Data>
          <ds:X509Certificate>[...PZo0=]</ds:X509Certificate>
        </ds:X509Data>
      </ds:KeyInfo>
    </KeyDescriptor>
    <SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="[Location]"/>
    <SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="[Location]"/>
    <NameIDFormat>urn:oasis:names:tc:SAML:2.0:nameid-format:transient</NameIDFormat>
    <NameIDFormat>urn:oasis:names:tc:SAML:2.0:nameid-format:persistent</NameIDFormat>
    <SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="[Location]"/>
  </IDPSSODescriptor>
  <Organization>
  </Organization>
  <ContactPerson contactType="technical">
  </ContactPerson>
  <ContactPerson contactType="support">
  </ContactPerson>
</EntityDescriptor>

Also, logging my idp and SP certificates, I can see that my SP uses, for both signing and encryption, the certificate outlined in metadata.

sp

{ 
  signing: '...Blw==' ,
  encryption: '...Blw=='
}

And idp also uses the cert for signing that I outlined in the metadata.

{ signing: '...PZo0=' }

I tried to validate my SAML response using an online SAML response tool. I added my SP private key, the idp cert that's in use for signing the response and the correct assertion urls. etc, and it was a valid response.

However, when trying to parse the response, I get

Error: ERR_FAILED_TO_VERIFY_SIGNATURE
    at /usr/src/app/node_modules/samlify/build/src/libsaml.js:323:27
    at Array.forEach (<anonymous>)
    at Object.verifySignature (/usr/src/app/node_modules/samlify/build/src/libsaml.js:289:23)
    at /usr/src/app/node_modules/samlify/build/src/flow.js:169:48
    at step (/usr/src/app/node_modules/samlify/build/src/flow.js:32:23)
    at Object.next (/usr/src/app/node_modules/samlify/build/src/flow.js:13:53)
    at fulfilled (/usr/src/app/node_modules/samlify/build/src/flow.js:4:58)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:189:7)

I'd expect the signature cert to be the same for both message and assertion signing for the idp.

I also checked the SAML response and the signature cert matched the one outlined in my idp for signing and the encryption cert matched the one outlined in my SP.

Here's my received SAML response. I've removed unnecessary data, hopefully it's still readable.

<?xml version="1.0" encoding="UTF-8"?>
<saml2p:Response Destination="[our SP assertion url]"
    ID=".." InResponseTo=".."
    IssueInstant="2018-10-23T10:50:04.021Z" Version="2.0"
    xmlns:saml2p="urn:oasis:names:tc:SAML:2.0:protocol">
    <saml2:Issuer xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion">[ipd entityId]</saml2:Issuer>
    <ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
        <ds:SignedInfo>
            <ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
            <ds:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/>
            <ds:Reference URI="...">
                <ds:Transforms>
                    <ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>
                    <ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
                </ds:Transforms>
                <ds:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/>
                <ds:DigestValue>[digest value]</ds:DigestValue>
            </ds:Reference>
        </ds:SignedInfo>
        <ds:SignatureValue>
            [...value...]
        </ds:SignatureValue>
        <ds:KeyInfo>
            <ds:X509Data>
                <ds:X509Certificate>[...PZo0=]</ds:X509Certificate>
            </ds:X509Data>
        </ds:KeyInfo>
    </ds:Signature>
    <saml2p:Status><saml2p:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/></saml2p:Status>
    <saml2:EncryptedAssertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion">
        <xenc:EncryptedData Id="..."
            Type="http://www.w3.org/2001/04/xmlenc#Element" xmlns:xenc="http://www.w3.org/2001/04/xmlenc#"><xenc:EncryptionMethod Algorithm="http://www.w3.org/2001/04/xmlenc#aes128-cbc"
            xmlns:xenc="http://www.w3.org/2001/04/xmlenc#"/>
            <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
                <xenc:EncryptedKey Id="..."
                    Recipient="[our entityId]" xmlns:xenc="http://www.w3.org/2001/04/xmlenc#">
                    <xenc:EncryptionMethod Algorithm="http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p"
                        xmlns:xenc="http://www.w3.org/2001/04/xmlenc#"><ds:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"
                        xmlns:ds="http://www.w3.org/2000/09/xmldsig#"/></xenc:EncryptionMethod>
                    <ds:KeyInfo>
                        <ds:X509Data>
                            <ds:X509Certificate>[...Blw==]</ds:X509Certificate>
                        </ds:X509Data>
                    </ds:KeyInfo>
                    <xenc:CipherData xmlns:xenc="http://www.w3.org/2001/04/xmlenc#">
                        <xenc:CipherValue>[...]</xenc:CipherValue>
                    </xenc:CipherData>
                </xenc:EncryptedKey>
            </ds:KeyInfo>
            <xenc:CipherData xmlns:xenc="http://www.w3.org/2001/04/xmlenc#">
                <xenc:CipherValue>[...]</xenc:CipherValue>
            </xenc:CipherData>
        </xenc:EncryptedData>
    </saml2:EncryptedAssertion>
</saml2p:Response>

@tngan
Copy link
Owner

tngan commented Oct 23, 2018

@laimikko1 You may need to define messageSigningOrder: 'sign-then-encrypt' in your idp setting as well. Seems like the default value is taken out for some reasons, but you can check if it works first.

@tngan
Copy link
Owner

tngan commented Oct 23, 2018

Ignore my above mentioned comment, the default value is still there.

@tngan
Copy link
Owner

tngan commented Oct 23, 2018

@laimikko1 I think I may know the problem. In order to get the highest level of data integrity check, IDP signs the assertion first, the next step is to encrypt the assertion, then finally signs the entire message.

By default, the messageSigningOrder is sign-then-encrypt. Therefore, the parsing process is reversed, in other words, decrypt then verify.

Once the assertion is being decrypt, the structure will become

Signature - Message
Assertion
  Signature - Assertion

In our current implementation, we will verify the message signature first, however, the message signature is signed when the assertion is being encrypted. Therefore, the signature verification will fail. I will review the flow soon. Thanks for your report.

@tngan tngan added bug and removed enhancement labels Oct 24, 2018
@laimikko1
Copy link
Author

It works now. The mistake was on my end as I thought our idp provider implemented 'sign-then-encrypt' instead of 'encrypt-then-sign'.

Another issue I ran into was that our idp doesn't set

<IDPSSODescriptor 
...
    WantAuthnRequestsSigned="true"
....

attribute in it's metadata, but still expects authnrequests to be signed. I couldn't get Samlify to recognize that the requests must be signed by modifying idp entitySettings, so I bypassed it by adding the value to the xml metadata manually.

Thanks for your help on the issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants