php / pecl-authentication-krb5

PECL krb5 extension
MIT License
10 stars 2 forks source link

Feature request: add channel binding support #2

Closed tvdijen closed 4 months ago

tvdijen commented 10 months ago

A shortage of Kerberos-over-HTTP is that there are no distinguished SPN's for HTTP- and HTTPS-services (1). This means that a ticket that's being transmitted over an insecure HTTP-connection can also be used for HTTPS-connections to the same host. Besides this, Kerberos is also known to be vulnerable for MitM-attacks where the service-label of the SPN can be altered when an alternative service is available on the same host (2).

Both of these attacks can be mitigated by using RFC 5056 'channel binding' (also known as Extended Protection for Authentication (EPA) in Microsoft environments). This takes the hash of the TLS server certificate and uses it inside the cryptographic part of the protocol. This prevents that an AP-REQ messages that an attacker acquired through an insecure channel can be used and sent to a secure channel on the same host.

Currently this library has no support for passing the server certificate (hash) to the Kerberos-validator:

afbeelding

tvdijen commented 10 months ago

https://github.com/php/pecl-authentication-krb5/pull/1#issuecomment-1840513670

From what I understand this would basically come down to allow passing the binding information into init/accept_sec_context, which I don't think would be that hard to implement.

mbechler commented 9 months ago

So, I found some time to tackle this. Added a GSSAPIChannelBinding class (see channel.c) encapsulating the binding data. init/acceptSecContext parameter handling still gets quite ugly (and does no longer match the GSSAPI spec), but I don't think there is much that can be done without breaking old code. I did not bother doing anything regarding address encoding, that is left to the user, but I guess you are only needing application data anyways. Testing appreciated.

tvdijen commented 9 months ago

Thanks Moritz! I think I'm still not fully understanding the concept of channel binding and how it would fit into my code. I'm currently only using the KRB5NegotiateAuth class to validate incoming tickets against a keytab. I think I'm gonna need some time to figure things out

mbechler commented 7 months ago

Sorry for the delay. I hadn't considered the KRB5NegotiateAuth wrapper in my initial implementation only the "raw" GSSAPIContext. That also will need an additional parameter to allow passing in the GSSAPIChannelBinding object. With regards on how to use it, a real specification seems kind of hard to come by but https://learn.microsoft.com/en-us/archive/blogs/openspecification/ntlm-and-channel-binding-hash-aka-extended-protection-for-authentication has a reasonable example of how to construct the binding data.

tvdijen commented 5 months ago

Hi @mbechler ! I'm still struggling to understand this and my attempts are leading nowhere until now.

It is my understanding that for SPNEGO, the client will add context to the ticket (the certificate-hash of the server) and that on the server-end this context can be verified to the expected value.

I think what this means is that I have to create a ChannelBinding-object and set the expected value there, then pass it to doAuthentication so that it can be passed along the call to gss_accept_sec_context for actual verification.

Now with my little knowledge of C / Zend, I was able to come to this: https://github.com/tvdijen/pecl-authentication-krb5/commit/4f7b52452af866db98f957ccc9989615cdea65dc

Unfortunately this fails and I can't understand why:

TypeError: KRB5NegotiateAuth::doAuthentication(): Argument #1 ($channel) must be of type string, GSSAPIChannelBinding given

I'm not even sure what I'm doing here is right.. Can you please help me out?

mbechler commented 5 months ago

Hi, there is an extra ARG_PATH (i.e an extra string/path argument) in zend_parse_parameters: https://github.com/tvdijen/pecl-authentication-krb5/commit/4f7b52452af866db98f957ccc9989615cdea65dc#diff-7c472312c5f748df8f4be5d50254e491b55079121bf62b2d0e45d1fafeb06363R314

tvdijen commented 5 months ago

Ah, that should make a difference.. But can you tell if I'm on the right track, concept-wise? Do I set the cert-hash on the ChannelBinding-class and pass it to doAuthentication and let that deal with the verification, or can I somehow pull the channel binding data from the KRB5NegotiateAuth class and do the verification in my PHP code?

mbechler commented 5 months ago

Looks good to me. API wise, I probably would have passed the channel binding data through the constructor as all other configuration goes through there - but no need to worry about that for a proof of concept. As far as I know you need to set the certificate hash as application data (prefixed with tls-server-end-point: as per https://datatracker.ietf.org/doc/html/rfc5929#section-4.1) - I don't believe the actual binding data is even transmitted so there is no way to pull that.

tvdijen commented 5 months ago

OK, thank you very much for the feedback! I will continue my struggle ;)

tvdijen commented 5 months ago

Quite a few steps further now: https://github.com/tvdijen/pecl-authentication-krb5/commit/a30b5445592f959ce972b94e7374f14d3f08606c

I can add the ChannelBinding-object or leave it away.. When I leave it away, everything still works as it used to. When passing the ChannelBinding as follows, I get an exception:

$binding = new GSSAPIChannelBinding();
$binding->setApplicationData('tls-server-end-point 2fe7a0fc8e8e068d30db4e5f083cbf3d925c3051c79df3f2fff3ee7f0911f1e0'); // SHA-256 fingerprint
$auth = new KRB5NegotiateAuth($this->keytab, $this->spn, $binding);
Warning - KRB5NegotiateAuth::doAuthentication(): Incorrect channel bindings were supplied (262144,100002) at src/Auth/Source/Negotiate.php:143
mbechler commented 5 months ago

This sounds like the correct error when there is a mismatch between the binding data used by the client and the server to me. I think that should be: tls-server-end-point: + the hash in binary, not hex

https://github.com/yan-too/jdk15u-dev/blob/c0ac7c7ba35dfa81de67fab781ae11bbe624b3c4/src/java.naming/share/classes/com/sun/jndi/ldap/sasl/TlsChannelBinding.java#L109

tvdijen commented 5 months ago

Changed it to:

$binding->setApplicationData(sprintf(
    'tls-server-end-point:%s',
    pack('H*', '2fe7a0fc8e8e068d30db4e5f083cbf3d925c3051c79df3f2fff3ee7f0911f1e0'),
);

That seems to work, in a way that my application logs that user X is authenticated, but then it mysteriously fails with a "client denied by server configuration" .. Could it be that I made a mistake somewhere with memory management that makes PHP do crazy things?

PHP-FPM logs are not very helpful here, but I can see this:

May  1 14:32:06 php-fpm[1633008]: [WARNING] [pool mypool] child 1633016 exited on signal 6 (SIGABRT) after 4855.080404 seconds from start
May  1 14:32:06 php-fpm[1633008]: [NOTICE] [pool mypool] child 1646853 started

This only happens when I trigger SPNEGO-authentication and not on any other page I try to access.

mbechler commented 5 months ago

_chanbindings memory is managed by the GSSAPIChannelBinding PHP object, ultimately KRB5NegotiateAuth will have to keep a reference to that object, but in a first step you might get away by removing the free.

tvdijen commented 5 months ago

Yes, that works :) Thanks so much for the help Moritz! You deserve a 🏅! Can we :ship: this into a release?