Support client certificate for artifact resolve

[AndersAbel]

Support TLS client certificate to be used for the SOAP binding of artifact resolve queries.

[AndersAbel]

Originally part of compatibility with Norwegian ID-porten, but it doesn't appear to be needed. Might have been a confusion between TLS client certs and signing of ArtifactResolve messages (which is done).

[SWSSolutions]

There are services who requires an TLS client cert for artifact resolve (like in our case).

[AndersAbel]

@SWSSolutions Thanks for mentioning that. It's something that needs to be implemented then.

[MartijnKooij]

A Dutch identity provider requires the client cert to be included in the artifact resolve SOAP request (as in HttpWebRequest.ClientCertificates). Is this that same issue?

I can probably work on this, but I'm not sure if you would like this to be optional behaviour? As in, should I also add a new configuration to the IdentityProvider?

[AndersAbel]

It is the same issue, yes.

I prefer to not put too many things into the IdentityProvider config, this is something that should be handled in the SPOptions. The question is if we can use the same Service Certificate that is used for signing in general or if there needs to be a special certificate usage specified for TLS client certificate. Maybe we should extend CertificateUsage with a TLSClient = 4???

If we have a separate usage the behaviour will in effect be optional as a TLS client certficate will only be sent if actually configured.

[MartijnKooij]

Sounds reasonable enough. ... But CertificateUse would become a bit weird IMO because it currently has a "Both" option, used for "Signing" AND "Encryption".

I could rename the option to "All" but that would be a breaking change... And still weird behavior because then you can no longer combine all posible options anymore (for example you could not choose to use Signing AND TLSClient).

As far as I understand the matter, it is not useful to send a different certificate along with the request other than the one you signed the message with. So a new usage setting would not be used. We could add a new bool includeInRequest property to the ServiceCertificate, but that would still allow for weird and possibly even incorrect combinations?

I think the cleanest approach would be to drop the "Both" option and introduce the new TLSClient option. This would preferably be a breaking change.

If you don't want breaking changes I think the new bool ServiceCertificate property "includeInRequest" would be more than good enough (seeing that the feature is barely used and SAML in general wont grow in usage?)

[AndersAbel]

The naming "Both, Signing, Encryption" is taken from the metadata standard, so I'd prefer to keep it for consistency. The "Both" setting being 0 is also consistent with the standard. If nothing is specified, it means both usages. Bundling the TlsClient setting into it will probably just make things more confusing. The certificates are bad as it is with people getting stuck.

What about adding a new option, ArtifactResolutionTlsClientCertificate? That would be simple and clear.

[MartijnKooij]

I have created a PR for this: https://github.com/KentorIT/authservices/pull/716 I am fairly new to the world of git so please just tell me if I need to approach things differently.

Unfortunately the project at work will not actually be using this framework as the Dutch identity provider we required it for uses some other ehm... "odd" settings which are not compatible with AuthServices... I promised to make this PR so I have, but I can't spent too much time on it anymore... Not during office hours that is.

[AndersAbel]

Thanks for the PR and taking your time for the PR even though you don't need it. The build server indicates some kind of merge conflict but I'll sort it out and merge it.

Just curious - what "odd" things were they using that AuthServices can't handle? Always good to know what keeps people from selecting other solutions instead.

[MartijnKooij]

Hmm... I was about to answer that, but looking through the framework code again I am not that sure anymore that is is indeed an issue. So let me ask you first.

We discovered that the IdP only supported SHA-1 for signing the artifact resolve. In our PoCs I always used a SHA-256 one, and that worked just fine. But is SHA-1 still supported in the framework?

I just assumed it was an issue because... Well... Who still uses SHA-1? And I found quite a few source references regarding Sha256, assumption is the mother of all..

[AndersAbel]

SHA1 is still supported, but the default mininum strength is now SHA256 on platforms that support it. To accept incoming SHA1, set the MinIncomingSigningAlgorithm to "sha1".

Regarding artifact resolve there's also ideas to enable support for unsigned messages - trusting the TLS certificate for authenticity.

[MartijnKooij]

Thanks, I had overlooked the MinIncomingSigningAlgorithm attribute. I will discuss it in the team today as the decicion may have already been made "upstairs".

[AndersAbel]

Glad we could clear that out. Looks like some more documentation is needed on various scenarios. The flexibility in the library has come to a point where configuration has got really complex.

[MartijnKooij]

There's a new project at the office here where we do need to make the move to SAML2. Could you perhaps consider merging the PR for this issue?

[desjoerd]

Hi, I've created a new PR to support this which doesn't contain version bumps of .Net and Owin. I hope this can be merged. I hope you've got time to look at this :).

[AndersAbel]

This functionality is now pushed to master. I will make a release shortly, but haven't really been able to test it. If anyone can build the current master and test it with an Idp that would be highly appreciated. To config - create a ServiceCertificate with the new Usage flage TlsClient