pac4j-saml: validation of certificates

[hng...@tripwire.com]

Greetings,

Pardon me if I'm posting to the wrong group; I wasn't sure if this group or the users group is more appropriate.

I was trying to figure out whether pac4j-saml validates the certificates in the SAML responses. i.e. checking whether the certificate is revoked or has expired. It looks like there is no validation at all. Am I mistaken?

If I want to validate the certificates in the responses, what's the best way to do so?

I thought I could just provide my own implementation of SAML2ResponseValidator to SAML2Client in order to validate the certificates, but there is no mechanism to swap the implementation of any of the interfaces that SAML2Client uses, short of extending SAML2Client itself. Was that the intended design?

Regards

-hai

[Jérôme LELEU]

Hi,

No, it's the right group for your question.

I didn't read the source code thoroughly, but I thought it was done in the validateSignature method. I don't know what it would not be in the SAML2DefaultResponseValidator, maybe because of self-signed certificates, but in that case, it's missing.

So either we had that to the default validator or we make things extensible by adding a setter and instantiating the default validator only if we have not already provided one.

Would you mind opening a Github issue and submitting a pull request for that? I guess it can fit in pac4j v2.1 version if we remain backward compatible.

Thanks.

Best regards,

Jérôme

--
You received this message because you are subscribed to the Google Groups "pac4j-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to pac4j-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Misagh]

Yes, you are mistaken.

The validation process has nothing to do with the certificate itself. The contents of the key are the only things that are used. The validity of the certificate itself is meaningless. This is not an issue pac4j would want to solve, as it simply is not an issue.

--

You received this message because you are subscribed to the Google Groups "pac4j-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to pac4j-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

- Misagh

[Hai Nguyen]

Misagh,

Please educate me why an expired certificate is not an issue. I'd like to understand it better.

I understand that pac4j validates the SAML response. Reading SAML2DefaultResponseValidator clearly shows that it validates the IssueInstant, Issuer, StatusCode, Signature etc. But the reason that SAML messages are signed is so that the receiver can trust they come from the right source. That trust comes from the certificate. If the certificate is expired, or worse, revoked, it seems prudent to no longer trust the signing of the SAML messages from that IdP. Why should that not be an issue?

---

From: Misagh [misagh....@gmail.com]
Sent: Saturday, May 13, 2017 11:00 AM
To: Hai Nguyen
Cc: pac4j-dev
Subject: Re: pac4j-saml: validation of certificates

[Misagh]

Trust does not come from the certificate. Trust comes from the public key, and having exchanged it via metadata with the other party. The certificate is not really "a certificate"; it's only a convenient mechanism used to hold and transmit the public key as part of metadata. Most IdPs and SPs even, dare I say, simply generate self-signed keys and set the expiration date to 20-30 years from the creation date. There are those that care about other properties of the certificate, but those are rather rare in the grand scheme of things.

[Hai Nguyen]

That was the point I tried to make. The public/private key pair is used for signing or encrypting data. The X.509  certificate, a convenient way to distribute the public key, has an expiration date that is used to limit the time the key strength is deemed sufficient. That means that whatever is encrypted with the private key can be considered as good/secure within the validity of the certificate. People who use a very long expiration date for they self-signed certificate do so because they don't want to deal with the complication of certificates management. But that doesn't mean it's a desired security practise.

Now, if the private key is compromised, the owner of that key would issue a certificate revocation to tell holders of the corresponding public key to no longer trust encryption or signing done with that private key. In that case, even if the SAML response is valid and its signing can be verified, I still cannot trust it because someone else might have stolen the private key and issues/signs the SAML messages in place of the IdP.

It's fine if pac4j doesn't do certificate validation, but that still doesn't mean it's a non-issue. It just means that those who care about checking the validity of the IdP certificate must do it through other means.

---

From: pac4...@googlegroups.com [pac4...@googlegroups.com] on behalf of Misagh [misagh....@gmail.com]
Sent: Monday, May 15, 2017 8:32 AM
To: pac4j-dev

You received this message because you are subscribed to a topic in the Google Groups "pac4j-dev" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/pac4j-dev/2ZQPPsMbdT0/unsubscribe.
To unsubscribe from this group and all its topics, send an email to pac4j-dev+...@googlegroups.com.

[Hai Nguyen]

Jérôme,

I haven't filed an issue yet, because I'm still pondering whether validating the certificate for each SAML response is the right thing to do. Just validating that the certificate used for singing hasn't expired isn't too bad, but checking for certificate revocation there could be a big performance issue.

Ideally, the SAML client would not redirect to the IdP if the its certificates are not valid. But I don't know how that would fit into your plan of making pac4j multi-tenant. (BTW, I agree that initialization should not take a WebContext. That seemed strange to me when I first read the code).

I'm exploring validating the IdP certificates outside of pac4j, since doing that kind of validation once or twice a day should be OK.
Thanks
-h.

---

From: Jérôme LELEU [lel...@gmail.com]
Sent: Saturday, May 13, 2017 1:25 AM

To: Hai Nguyen
Cc: pac4j-dev
Subject: Re: pac4j-saml: validation of certificates

[Misagh]

On Mon, May 15, 2017 at 10:28 AM, Hai Nguyen <hng...@tripwire.com> wrote:

That was the point I tried to make. The public/private key pair is used for signing or encrypting data. The X.509  certificate, a convenient way to distribute the public key, has an expiration date that is used to limit the time the key strength is deemed sufficient. That means that whatever is encrypted with the private key can be considered as good/secure within the validity of the certificate.

​Not quite. That means whatever is encrypted with the private key can be considered good/secure within the validity of the key itself. You CAN validate the certificate too, sure, but that generally turns out to be irrelevant. If I, the IdP, create a self-signed keypair, RSA-2048 with an expiration date of 2037, do you think you're going to get much out of validating the expiration date as an SP? 

It's a good measure. Generally, it's a pointless measure.

​

People who use a very long expiration date for they self-signed certificate do so because they don't want to deal with the complication of certificates management. But that doesn't mean it's a desired security practise.

​

I fully agree. Key rotation is terribly, stupidly difficult, but implementations ​

today riff on t

​his point.​

 

​That's an important clarification I should add. Speaking from memory here, but I don't think you do actually find anything in the spec in terms of what properties of the certificate, if any, should be checked which is why most impls tend to do this differently...and as I say, most (IdPs, such as CAS or Shibboleth) in fact ignore it. ​

I have yet to find an SP or an IdP that

​would 

care about these things

​. The Shibboleth IdP/SP in fact entirely ignore these things too, and so have almost every other SP I have run into​

and as I say, relying on the public key and cross checking the response with metadata has proven to be good enough.

​​

A certificate's expiration date has nothing to do with the security of the corresponding private key

​. If that is not helpful, y

ou're welcome to reach out to the SAML IdP developers (Shibboleth in particular), or in particular the InCommon federations guidelines to learn more about this aspect, which mostly share the

​same ​

viewpoint and are probably a better audience to answer the question more accurately.

​

Now, if the private key is compromised, the owner of that key would issue a certificate revocation to tell holders of the corresponding public key to no longer trust encryption or signing done with that private key. In that case, even if the SAML response is valid and its signing can be verified, I still cannot trust it because someone else might have stolen the private key and issues/signs the SAML messages in place of the IdP.

​I don't follow how this relates to a certificate, to be honest. ​If a private key is lost or stolen, the owner would go through the proper channels as you say to revoke the certificate and rotate keys with a new private key. This is then put into the metadata and exchanged with the other parties,  preferably as part of an aggregate or MDQ. So yes, you will not be able to trust it for the time it takes for you to get your hands on the new version of the metadata which contains the corresponding relevant [new] keys...and yes for the duration of that window, you'll be vulnerable, and yes if you have bilateral integrations you'd be in trouble but I fail to see how checking properties of the certificate actually can help in this scenario. 

Let's say a compromised private key is leaked and I as the adversary end up sending you spoofed SAML responses. I am sure, as a capable attacker, I can forge my way into a decent certificate that has all the valid fields and is set to expire in yet another 20 years. Or 5 days. Doesn't matter. All it takes is one try, which why I am saying, you care a lot more about the security of the key itself, and practices around that and not what holds it. This is not TLS. 

Now, I am not saying you MUST not validate those things. I am saying SHOULD not, as SHOULD is also the tone/approach taken by most impls today. InCommon for instance frowns upon expired certificates in the metadata. So please do validate if you find it important for your use cases, as Jérôme says. So long there is a toggle of the behavior, and it's off by default, we should be fine. I don't think we'd get much of out it today, and in most cases it will cause more trouble than good practically speaking. 

Virus-free. www.avast.com

[Jérôme LELEU]

Hi,

I don't think we should change the current behaviour, but allowing to define your own should be possible nonetheless.

Would you mind opening an issue and proposing a simple pull request to be able to plugin your own SAML2ResponseValidator?

Thanks.

Best regards,

Jérôme

--