Credit for this to Joseph Sutton of Catalyst. I'm just writing things up for him. The main AS-REQ and TGS-REQ in Kerberos is defined as: KDC-REQ-BODY ::= SEQUENCE { kdc-options[0] KDCOptions, cname[1] PrincipalName OPTIONAL, -- Used only in AS-REQ realm[2] Realm, -- Server's realm -- Also client's in AS-REQ sname[3] PrincipalName OPTIONAL, from[4] KerberosTime OPTIONAL, till[5] KerberosTime OPTIONAL, rtime[6] KerberosTime OPTIONAL, nonce[7] Krb5Int32, etype[8] SEQUENCE OF ENCTYPE, -- EncryptionType, -- in preference order addresses[9] HostAddresses OPTIONAL, enc-authorization-data[10] EncryptedData OPTIONAL, -- Encrypted AuthorizationData encoding additional-tickets[11] SEQUENCE OF Ticket OPTIONAL } In Heimdal the sname is not checked for being NULL in a TGS-REQ. In MIT the sname is not checked for being NULL in a tunnelled TGS-REQ under FAST. The Heimdal issue impacts on both current Heimdal (from git) and the old version in current Samba. The MIT issue was tested against current git master. We will shortly upload a patch to exploit this, built on a new Samba testsuite, shortly, backtraces and proof of concept MIT and Heimdal patches.
Created attachment 16703 [details] backtrace for current Samba with Heimdal
Note that this is already fixed in Apple's version 398.1.2 in 2016 with: https://gitlab.com/catalyst-samba/opensource.apple.com-heimdal/-/commit/3791c640880164ac4aad0f9f96586dd6ab34aaec?page=4#82e60b4bbafe4508fbf7c8456c7d4f18e8ce204a_1517_1523 (so there is as good a fix to bring in as any) In Heimdal the issue is that we have: static krb5_error_code tgs_build_reply(astgs_request_t priv, hdb_entry_ex *krbtgt, krb5_enctype krbtgt_etype, const krb5_keyblock *replykey, int rk_is_subkey, krb5_ticket *ticket, const char **e_text, AuthorizationData **auth_data, const struct sockaddr *from_addr) ... PrincipalName *s; ... s = b->sname; ... _krb5_principalname2krb5_principal(context, &sp, *s, r);
Heimdal doesn't treat DoS as a security issue. If you can prepare a pull request we will commit a fix to master straight away.
Note that sname can legitimately be absent for user-to-user, see below from RFC 4120: cname and sname These fields are the same as those described for the ticket in section 5.3. The sname may only be absent when the ENC-TKT-IN- SKEY option is specified. If the sname is absent, the name of the server is taken from the name of the client in the ticket passed as additional-tickets. So it's not sufficient to bail in tgs_build_reply() if b->sname == NULL.
I guess something like this: lukeh@fuglen kdc % git diff . diff --git a/kdc/krb5tgs.c b/kdc/krb5tgs.c index e8a3d1b37..6af24620c 100644 --- a/kdc/krb5tgs.c +++ b/kdc/krb5tgs.c @@ -1699,6 +1699,10 @@ tgs_build_reply(astgs_request_t priv, s = &adtkt.cname; r = adtkt.crealm; + } else if (s == NULL) { + ret = KRB5KRB_ERR_GENERIC; + _kdc_set_e_text(r, "No server in request"); + goto out; } _krb5_principalname2krb5_principal(context, &sp, *s, r); should be OK?
(In reply to Luke Howard from comment #3) Thanks for the context. If you could hold off pushing that change until we make our release that would be awesome. Thanks!
Sure. Can you test that change fixes the bug?
It seems to fix the bug, but I had to change the _kdc_set_e_text() call to krb5_set_error_message() for it to compile on current Samba versions.
OK, probably should be _kdc_audit_addreason() and kdc_log() then
Feel free to file a pull request with Heimdal when you are ready to see this integrated
Just to be clear, I should expect an MIT krb5 backtrace and POC patch here soon? MIT krb5's normal procedure for KDC denial of service attacks is to assign a CVE number, but not to do advance disclosure. I can request a CVE from MITRE if Joseph and Andrew haven't done so and don't plan to.
(In reply to Greg Hudson from comment #11) Yes, you should expect a proper backtrace soon. Sorry for not getting those up yesterday, we just ran out of time. I've asked for a CVE for Samba's use of Heimdal, I understand MITRE wants one CVE per product these days, so feel free to get one for MIT. Do you want a coordinated release, or would you like Heimdal just apply a patch and release with the next version asyncronously? Thanks, Andrew Bartlett
Created attachment 16704 [details] backtrace for current MIT Kerberos
(In reply to Andrew Bartlett from comment #12) I will request a CVE number from MITRE. I don't think Heimdal needs to delay for us, since the impact is low and the Heimdal fix doesn't point directly to the MIT vulnerability.
MITRE has assigned CVE-2021-37750 for the MIT krb5 vulnerability. Since Luke mentioned that RFC 4120 allows sname to be absent for U2U requests, I'll link to the ticket about MIT's KDC not permitting that (which is probably a WONTFIX): https://krbdev.mit.edu/rt/Ticket/Display.html?id=8668
I wrote up a candidate patch for MIT krb5. I don't know if I will be able to put together an automated test, but I will test it by hand. diff --git a/src/kdc/do_tgs_req.c b/src/kdc/do_tgs_req.c index 582e497cc9..32dc65fa8e 100644 --- a/src/kdc/do_tgs_req.c +++ b/src/kdc/do_tgs_req.c @@ -204,6 +204,11 @@ process_tgs_req(krb5_kdc_req *request, krb5_data *pkt, status = "FIND_FAST"; goto cleanup; } + if (sprinc == NULL) { + status = "NULL_SERVER"; + errcode = KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN; + goto cleanup; + } errcode = get_local_tgt(kdc_context, &sprinc->realm, header_server, &local_tgt, &local_tgt_storage, &local_tgt_key);
Is it better to return KRB5KRB_ERR_GENERIC for a protocol violation than KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN?
Maybe. I copied and adapted this check from the AS code, so I'd lean towards being consistent there. However, if the server is missing from the outer body, the client will see a KRB5KDC_ERR_WRONG_REALM error, more or less by accident (setup_server_realm returns NULL, which is indistinguishable from the KDC being configured for multiple realms and none of them matching). Ultimately it doesn't seem very important, since the standard doesn't specify an error code for invalid requests (as far as I can tell).
After testing, I found that the MIT KDC won't respond at all to a request with a missing server field. If it's missing in the outer body, the dispatch code fails to set up the realm (as mentioned in the previous comment) and there's no code to generate an error response. If it's missing in the inner body and the KDC doesn't crash (pre-1.14 or with the candidate patch), the code will try to generate a response and fail because the KRB-ERROR server field is mandatory. (Heimdal has code in krb5_mk_error_ext() to handle a null server but we do not.) Since that is already our behavior for a missing server in the outer request body, I'm not worried about it being the behavior for the inner body as well. So I'm still happy with the candidate patch.
I'm ready to push the fix. Do Heimdal people want me to wait?
Current Samba practice is to do the full embargoed security release process for a Denial of Service. That is to follow https://wiki.samba.org/index.php/Samba_Security_Process I've got a current proposal to change that to match Heimdal and MIT (bug, perhaps get a CVE but then just move on async and in public), but I'm just waiting for feedback. I'll get back to you soon.
I've asked for feedback as proposed and the sense I get is that for a simple crash like this, which Samba will recover from, we can just fix this in a normal release. As that is also the view of MIT and Heimdal then we should all fix this independently as we are able, rather than do the full multi-vendor coordinated thing. Thanks for your patience.
Just following up on this. Are you happy for me to commit a fix for Heimdal to master?
Yes, and good timing, I put up our new policy this morning. We will deal with this as 'just a bug'.
Fixed in Heimdal. commit 04171147948d0a3636bc6374181926f0fb2ec83a Author: Luke Howard <lukeh@padl.com> Date: Fri Aug 27 11:42:48 2021 +1000 kdc: validate sname in TGS-REQ In tgs_build_reply(), validate the server name in the TGS-REQ is present before dereferencing.
Removing embargo per new policy.
Note my fix also had a bug in it :( You also need this one. commit 773802aecfb4b6a73817fa522faeb55b2a7cdb2a Author: Luke Howard <lukeh@padl.com> Date: Fri Aug 27 15:11:54 2021 +1000 kdc: fix _kdc_set_e_text argument in previous commit "r" is the realm, not the TGS request; that is priv
This bug was referenced in samba master: 79dda329f2a8382f1e46b50f4b9692e78d687826 15f9f040fe537ebd30419a4751aa0f13b20f242b 0cb4b939f192376bf5e33637863a91a20f74c5a5 b8e2515552ffa158fab1e86a39004de4cc419da5 3330eaf39c6174f2d90fe4d8e016efb97005d1e5 e373c6461a88c44303ea8cdbebc2d78dd15dec4a 1e4d757394a0bbda587d5ff91801f88539b712b1 bbbb13caf7bd2440c80f4f4775725b7863d16a5b c6d7e19ecfb264c6f79df5a20e830e4ea6fdb340 24914ae17d49f634fafc1bdeb88859293da05f79 ebd673e976aea5dd481a75f180fd526995c4fda0 b0f4455e524cbbfb13202220e7095f466b083a2f 10baaf08523200e47451aa1862430977b0365b59
(In reply to Luke Howard from comment #27) Thanks. In my backport to our version we just used a potentially pointless: + krb5_set_error_message(context, ret, "No server in request"); so we are safe.
This bug was referenced in samba v4-15-test: c9b594a1a21ea7b2d02f2b6811206edc1f245471 1ae386bf72564850f0660eb4d9b74076ed74bd91 d9de103cc587aa98e0e79781bcb387dac4ee1302 f50f9618efa49fc7a2a56b2f1c99fb6b2c0c5bcd 2762a9dcee4e37eb238007558bc790543a796a17 13cb266426646bdbb786aba734a95f25e1bfff2f 83ba64c9106ff5bd53848c46a3f493045db868d4 b628cda6604c8bd3552eac7e8ba5d203638e51c2 c837f43a9cdd03a1f45913d1e19a71fbb3373af0 8d17a87523bf5d3660840cd5b81738b8b98f61fa dcbec3eab5253f0c5c9ad30e3a406a4fed4c1d29 0fd150e48447d362e47c4c3f9e7bf0930db03afd 7ca641892b389f2bd6f13afb862c632a8375cff6
This bug was referenced in samba v4-15-stable (Release samba-4.15.0rc6): c9b594a1a21ea7b2d02f2b6811206edc1f245471 1ae386bf72564850f0660eb4d9b74076ed74bd91 d9de103cc587aa98e0e79781bcb387dac4ee1302 f50f9618efa49fc7a2a56b2f1c99fb6b2c0c5bcd 2762a9dcee4e37eb238007558bc790543a796a17 13cb266426646bdbb786aba734a95f25e1bfff2f 83ba64c9106ff5bd53848c46a3f493045db868d4 b628cda6604c8bd3552eac7e8ba5d203638e51c2 c837f43a9cdd03a1f45913d1e19a71fbb3373af0 8d17a87523bf5d3660840cd5b81738b8b98f61fa dcbec3eab5253f0c5c9ad30e3a406a4fed4c1d29 0fd150e48447d362e47c4c3f9e7bf0930db03afd 7ca641892b389f2bd6f13afb862c632a8375cff6
This bug was referenced in samba v4-14-test: 7a938531dd0173c31f4c197d6f1035fb28eb87fc 5c4de75af508a1774c727c88e3515a6e6756e381 2444c94cb3a09aa2c5da5742c7f43064162dabe9 474ddf8fdda539a1878f3b83700d5ad06346c1aa 12c9c5b7d29a025deeb6d264b52022bf43638f4e f18cff2b0e1040462f25869e51123cb5dbd147e9 112e362525317efc8537fe6e9672bfd39f3930f8 63e5d195a5a258b45b7f2556e2b2188c97d5616d 7b4c9eea2534d04917d3272c34ad42f6c1378209 a21afdbcd7bd921341ae38b972914ec93e3d56c7 53b48cbe9a8e20007f45568519c81f95c172a5ad
This bug was referenced in samba v4-13-test: 497b461238bf69eb5ff92c4b849b8f56bbcbac5e a67cda7159f3c7e9c381a13705011dd9c93742ae d3a611377bdda70e6940b6f3fff03cc6240f6a5b bd76f6d47e756692243a77e7628324e333c566a0 8a8872f7070a6f2c89e2ba38d89df0e27bca9f71 cabc5b114dc094e36b4c052ed524757990ec6321 b5e11c10966dcbb9ca4e751c6c378e2f9ed6e358 57800189c5f4a92058ff293f8583805ebcf9928d 1e27b45f49c1a6d610ec498e48b4ed4f6e85c772 7a2a6e0bcb0f9508322e940360b95eae52572cb2 b7d16fdc65397114bcc9199bbd4092f54d11e565
This bug was referenced in samba v4-13-stable (Release samba-4.13.12): 497b461238bf69eb5ff92c4b849b8f56bbcbac5e a67cda7159f3c7e9c381a13705011dd9c93742ae d3a611377bdda70e6940b6f3fff03cc6240f6a5b bd76f6d47e756692243a77e7628324e333c566a0 8a8872f7070a6f2c89e2ba38d89df0e27bca9f71 cabc5b114dc094e36b4c052ed524757990ec6321 b5e11c10966dcbb9ca4e751c6c378e2f9ed6e358 57800189c5f4a92058ff293f8583805ebcf9928d 1e27b45f49c1a6d610ec498e48b4ed4f6e85c772 7a2a6e0bcb0f9508322e940360b95eae52572cb2 b7d16fdc65397114bcc9199bbd4092f54d11e565
This bug was referenced in samba v4-14-stable (Release samba-4.14.8): 7a938531dd0173c31f4c197d6f1035fb28eb87fc 5c4de75af508a1774c727c88e3515a6e6756e381 2444c94cb3a09aa2c5da5742c7f43064162dabe9 474ddf8fdda539a1878f3b83700d5ad06346c1aa 12c9c5b7d29a025deeb6d264b52022bf43638f4e f18cff2b0e1040462f25869e51123cb5dbd147e9 112e362525317efc8537fe6e9672bfd39f3930f8 63e5d195a5a258b45b7f2556e2b2188c97d5616d 7b4c9eea2534d04917d3272c34ad42f6c1378209 a21afdbcd7bd921341ae38b972914ec93e3d56c7 53b48cbe9a8e20007f45568519c81f95c172a5ad