kdc: Reduce max UDP reply size default to 500 bytes and document better (fix #1216) #1218

nicowilliams · 2024-01-10T23:53:44Z

See #1216. This change will drop the max UDP reply size to 500 bytes, causing the KDC to reply to all requests sent via UDP with a KRB5_ERR_RESPONSE_TOO_BIG error that causes clients to switch to using TCP.

nicowilliams · 2024-01-11T00:07:58Z

Well, all these test failures...

nicowilliams · 2024-01-11T16:51:31Z

The KRB5_ERR_RESPONSE_TOO_BIG handling in the FAST case is busted, and that's what's causing at least some of the test failures.

nicowilliams · 2024-01-11T17:48:40Z

The send_to_kdc logic for handling KRB5_ERR_RESPONSE_TOO_BIG can only do that for the outer KRB-ERROR, but in the FAST case the KRB5_ERR_RESPONSE_TOO_BIG will be in the inner KRB-ERROR, so we fail in the TGS case, though somehow not in the AS case. I believe it's secure to return KRB5_ERR_RESPONSE_TOO_BIG in the outer error and dispense with FAST in that case, but I'm not sure whether that will interoperate with MIT, Windows, etc. I don't believe the RFC (6113) discusses this case.

Of course we could and should fix the Heimdal client to handle KRB5_ERR_RESPONSE_TOO_BIG in the inner KRB-ERROR, but that's not a KDC-side-only fix, and rolling out the client-side fix first would be painful. Still, any site that finds its on-the-Internet KDCs being used in an amplification attack can just stop advertising UDP support in DNS and then stop allowing UDP access to its KDCs via IP filtering.

nicowilliams · 2024-01-11T17:55:56Z

The MIT Kerberos KDC sends always KRB5KRB_ERR_RESPONSE_TOO_BIG as a naked, non-FAST error. Heimdal should do the same then!

kdc/fast.c

nicowilliams · 2024-01-11T18:13:35Z

The place where we handle the too-big error in kdc/kerberos5.c and kdc/krb5tgs.c may not be quite right in that I think it causes other errors to be sent as usual.

jsutton24 · 2024-01-11T21:07:59Z

I believe it's secure to return KRB5_ERR_RESPONSE_TOO_BIG in the outer error and dispense with FAST in that case, but I'm not sure whether that will interoperate with MIT, Windows, etc. I don't believe the RFC (6113) discusses this case.

I think the third paragraph of section 5.4.4 mentions it:

“If the Kerberos FAST padata is included in the request but not included in the error reply, it is a matter of the local policy on the client to accept the information in the error message without integrity protection. However, the client SHOULD process the KDC errors as the result of the KDC's inability to accept the AP_REQ armor and potentially retry another request with a different armor when applicable. […]”

riastradh · 2024-01-12T03:56:00Z

I haven't looked closely at any of this but the prospect of a server returning any kind of fine-grained error information to an unauthenticated client, and of a client acting meaningfully on any unauthenticated error reports, is making my skin crawl.

nicowilliams · 2024-01-12T05:21:44Z

I haven't looked closely at any of this but the prospect of a server returning any kind of fine-grained error information to an unauthenticated client, and of a client acting meaningfully on any unauthenticated error reports, is making my skin crawl.

It's not ready for review. Kerberos relies on some "fine-grained error information" (e.g., what enctypes are supported, salts, etc.) to be provided to unauthenticated clients in order for them to authenticate. Anyways, when I'm done, if you set max-kdc-datagram-reply-length to 0 then all UDP requests will elicit a minimal KRB5_ERR_RESPONSE_TOO_BIG error.

nicowilliams · 2024-01-16T16:35:37Z

I think this is almost ready for code review. The code is getting excercised, but now we're now not testing [kdc] max-kdc-datagram-reply-length values larger than zero, so I should update one of the krb5.conf.in files in the tests to set it to something else. Also, we should check the logs to make sure that it's actually being used....

nicowilliams · 2024-01-16T17:08:48Z

And now with testing.

nicowilliams · 2024-01-16T17:09:05Z

@riastradh would you like to do a code review?

nicowilliams · 2024-01-16T19:38:35Z

I've also added two somewhat unrelated commits, one to speed up tests/kdc/check-kdc, and one to fix the OS X build which was failing tests/kdc/check-hdb-mitdb.

nicowilliams · 2024-01-16T20:57:06Z

I might as well do the here-document thing for most uses of kadmin in the tests as it may significantly speed up all the tests.

nicowilliams · 2024-01-16T22:30:03Z

I've pushed the extraneous commits separately to master. This branch is ready now.

kdc/kerberos5.c

kdc/fast.c

tests/gss/check-context.in

kdc/fast.c

kdc/krb5tgs.c

kdc/fast.c

nicowilliams · 2024-01-20T21:47:12Z

Also minimizing the sname in the KRB-ERROR causes Java interop issues, specifically causing the tests/java/check-kinit to fail with:

Authentication failed:java.lang.IllegalArgumentException: Empty nameStrings not allowed
	at java.security.jgss/sun.security.krb5.PrincipalName.validateNameStrings(PrincipalName.java:172)
	at java.security.jgss/sun.security.krb5.PrincipalName.<init>(PrincipalName.java:282)
	at java.security.jgss/sun.security.krb5.PrincipalName.parse(PrincipalName.java:320)
	at java.security.jgss/sun.security.krb5.internal.KRBError.init(KRBError.java:362)
	at java.security.jgss/sun.security.krb5.internal.KRBError.<init>(KRBError.java:189)
	at java.security.jgss/sun.security.krb5.KrbAsRep.<init>(KrbAsRep.java:63)
	at java.security.jgss/sun.security.krb5.KrbAsReqBuilder.send(KrbAsReqBuilder.java:345)
	at java.security.jgss/sun.security.krb5.KrbAsReqBuilder.action(KrbAsReqBuilder.java:498)
	at jdk.security.auth/com.sun.security.auth.module.Krb5LoginModule.attemptAuthentication(Krb5LoginModule.java:746)
	at jdk.security.auth/com.sun.security.auth.module.Krb5LoginModule.login(Krb5LoginModule.java:592)
	at java.base/javax.security.auth.login.LoginContext.invoke(LoginContext.java:747)
	at java.base/javax.security.auth.login.LoginContext$4.run(LoginContext.java:672)
	at java.base/javax.security.auth.login.LoginContext$4.run(LoginContext.java:670)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at java.base/javax.security.auth.login.LoginContext.invokePriv(LoginContext.java:670)
	at java.base/javax.security.auth.login.LoginContext.login(LoginContext.java:581)
	at KerberosInit.main(KerberosInit.java:84)

This is due to a) the sname field of KRB-ERROR not being OPTIONAL and b) krb5_mk_error_ext() with NULL server causing the sname field of the KRB-ERROR to have zero components (and the srealm gets set to a bogus realm).

We can further minimize the KRB-ERROR by using a smaller bogus srealm and making the sname a 1-component name with a small bogus component value.

krb5_mk_error_ext() with a NULL server name would generate a KRB-ERROR with its sname field set to a zero-component principal name. This causes Java's Krb5 client to throw an IllegalArgumentException with `Empty nameStrings not allowed` as its message. Now krb5_mk_error_ext() with a NULL server name will set the sname field to a one-component name with the one component being "<>". We also minimize the srealm, setting it to "<>" as well.

…fix heimdal#1216)

nicowilliams · 2024-01-21T19:37:59Z

After some discussion we think that defaulting this to zero is asking for trouble, so we'll go with 500.

(Also, I think I found a referrals bug. @lhoward @josephsutton1 @jaltman ? Let's see if fixing it breaks tests.)

kdc/krb5tgs.c

abartlet · 2024-01-31T09:39:59Z

Thanks @nicowilliams for putting the explanation about Samba's strange NOT_FOUND_HERE handling into the code, that will help others understand our strange RODC hook.

The code looks pretty good to me at an initial glance.

nicowilliams changed the title ~~kdc: Reduce max UDP reply size default to 1200 bytes and document better (fix #1216)~~ kdc: Reduce max UDP reply size default to 0 bytes and document better (fix #1216) Jan 10, 2024

nicowilliams force-pushed the reduce-max-kdc-udp-reply-def branch from 7d2c3b0 to e92437e Compare January 10, 2024 23:54

nicowilliams force-pushed the reduce-max-kdc-udp-reply-def branch from e92437e to feed2c0 Compare January 11, 2024 18:02

nicowilliams commented Jan 11, 2024

View reviewed changes

kdc/fast.c Show resolved Hide resolved

nicowilliams force-pushed the reduce-max-kdc-udp-reply-def branch 2 times, most recently from ba739f6 to a0b37aa Compare January 16, 2024 16:31

nicowilliams force-pushed the reduce-max-kdc-udp-reply-def branch from a0b37aa to 9829b81 Compare January 16, 2024 17:08

nicowilliams force-pushed the reduce-max-kdc-udp-reply-def branch from 9829b81 to 9cd47ec Compare January 16, 2024 19:36

nicowilliams force-pushed the reduce-max-kdc-udp-reply-def branch 2 times, most recently from f0c1008 to 93bcc1d Compare January 16, 2024 22:29