Bug 14770 (CVE-2021-3671) - CVE-2021-3671 [SECURITY] Samba, Heimdal and MIT Kerberos crash on missing sname in TGS-REQ
Summary: CVE-2021-3671 [SECURITY] Samba, Heimdal and MIT Kerberos crash on missing sna...
Status: NEW
Alias: CVE-2021-3671
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.15.0rc1
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Joseph Sutton
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-07-29 01:30 UTC by Andrew Bartlett
Modified: 2021-09-16 08:55 UTC (History)
6 users (show)

See Also:


Attachments
backtrace for current Samba with Heimdal (6.22 KB, application/octet-stream)
2021-07-29 01:35 UTC, Joseph Sutton
no flags Details
backtrace for current MIT Kerberos (4.77 KB, text/plain)
2021-07-29 23:05 UTC, Joseph Sutton
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Bartlett 2021-07-29 01:30:41 UTC
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.
Comment 1 Joseph Sutton 2021-07-29 01:35:48 UTC
Created attachment 16703 [details]
backtrace for current Samba with Heimdal
Comment 2 Andrew Bartlett 2021-07-29 02:11:53 UTC
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);
Comment 3 Luke Howard 2021-07-29 02:17:46 UTC
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.
Comment 4 Luke Howard 2021-07-29 02:21:26 UTC
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.
Comment 5 Luke Howard 2021-07-29 02:23:51 UTC
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?
Comment 6 Andrew Bartlett 2021-07-29 02:37:11 UTC
(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!
Comment 7 Luke Howard 2021-07-29 02:37:50 UTC
Sure. Can you test that change fixes the bug?
Comment 8 Joseph Sutton 2021-07-29 05:41:41 UTC
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.
Comment 9 Luke Howard 2021-07-29 05:48:55 UTC
OK, probably should be _kdc_audit_addreason() and kdc_log() then
Comment 10 Luke Howard 2021-07-29 05:49:12 UTC
Feel free to file a pull request with Heimdal when you are ready to see this integrated
Comment 11 Greg Hudson 2021-07-29 19:29:55 UTC
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.
Comment 12 Andrew Bartlett 2021-07-29 21:41:50 UTC
(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
Comment 13 Joseph Sutton 2021-07-29 23:05:01 UTC
Created attachment 16704 [details]
backtrace for current MIT Kerberos
Comment 14 Greg Hudson 2021-07-30 14:43:27 UTC
(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.
Comment 15 Greg Hudson 2021-07-30 18:12:41 UTC
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
Comment 16 Greg Hudson 2021-08-03 05:21:25 UTC
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);
Comment 17 Luke Howard 2021-08-04 08:39:31 UTC
Is it better to return KRB5KRB_ERR_GENERIC for a protocol violation than KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN?
Comment 18 Greg Hudson 2021-08-04 16:50:19 UTC
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).
Comment 19 Greg Hudson 2021-08-04 22:14:47 UTC
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.
Comment 20 Greg Hudson 2021-08-05 21:02:00 UTC
I'm ready to push the fix.  Do Heimdal people want me to wait?
Comment 21 Andrew Bartlett 2021-08-06 00:09:15 UTC
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.
Comment 22 Andrew Bartlett 2021-08-16 04:33:39 UTC
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.
Comment 23 Luke Howard 2021-08-27 00:57:51 UTC
Just following up on this. Are you happy for me to commit a fix for Heimdal to master?
Comment 24 Andrew Bartlett 2021-08-27 01:35:55 UTC
Yes, and good timing, I put up our new policy this morning.  We will deal with this as 'just a bug'.
Comment 25 Luke Howard 2021-08-27 01:46:25 UTC
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.
Comment 26 Andrew Bartlett 2021-08-31 05:13:18 UTC
Removing embargo per new policy.
Comment 27 Luke Howard 2021-08-31 05:32:31 UTC
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
Comment 28 Samba QA Contact 2021-09-02 14:29:03 UTC
This bug was referenced in samba master:

79dda329f2a8382f1e46b50f4b9692e78d687826
15f9f040fe537ebd30419a4751aa0f13b20f242b
0cb4b939f192376bf5e33637863a91a20f74c5a5
b8e2515552ffa158fab1e86a39004de4cc419da5
3330eaf39c6174f2d90fe4d8e016efb97005d1e5
e373c6461a88c44303ea8cdbebc2d78dd15dec4a
1e4d757394a0bbda587d5ff91801f88539b712b1
bbbb13caf7bd2440c80f4f4775725b7863d16a5b
c6d7e19ecfb264c6f79df5a20e830e4ea6fdb340
24914ae17d49f634fafc1bdeb88859293da05f79
ebd673e976aea5dd481a75f180fd526995c4fda0
b0f4455e524cbbfb13202220e7095f466b083a2f
10baaf08523200e47451aa1862430977b0365b59
Comment 29 Andrew Bartlett 2021-09-03 09:33:47 UTC
(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.
Comment 30 Samba QA Contact 2021-09-08 13:55:41 UTC
This bug was referenced in samba v4-15-test:

c9b594a1a21ea7b2d02f2b6811206edc1f245471
1ae386bf72564850f0660eb4d9b74076ed74bd91
d9de103cc587aa98e0e79781bcb387dac4ee1302
f50f9618efa49fc7a2a56b2f1c99fb6b2c0c5bcd
2762a9dcee4e37eb238007558bc790543a796a17
13cb266426646bdbb786aba734a95f25e1bfff2f
83ba64c9106ff5bd53848c46a3f493045db868d4
b628cda6604c8bd3552eac7e8ba5d203638e51c2
c837f43a9cdd03a1f45913d1e19a71fbb3373af0
8d17a87523bf5d3660840cd5b81738b8b98f61fa
dcbec3eab5253f0c5c9ad30e3a406a4fed4c1d29
0fd150e48447d362e47c4c3f9e7bf0930db03afd
7ca641892b389f2bd6f13afb862c632a8375cff6
Comment 31 Samba QA Contact 2021-09-09 06:38:24 UTC
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
Comment 32 Samba QA Contact 2021-09-16 08:03:06 UTC
This bug was referenced in samba v4-14-test:

7a938531dd0173c31f4c197d6f1035fb28eb87fc
5c4de75af508a1774c727c88e3515a6e6756e381
2444c94cb3a09aa2c5da5742c7f43064162dabe9
474ddf8fdda539a1878f3b83700d5ad06346c1aa
12c9c5b7d29a025deeb6d264b52022bf43638f4e
f18cff2b0e1040462f25869e51123cb5dbd147e9
112e362525317efc8537fe6e9672bfd39f3930f8
63e5d195a5a258b45b7f2556e2b2188c97d5616d
7b4c9eea2534d04917d3272c34ad42f6c1378209
a21afdbcd7bd921341ae38b972914ec93e3d56c7
53b48cbe9a8e20007f45568519c81f95c172a5ad
Comment 33 Samba QA Contact 2021-09-16 08:55:04 UTC
This bug was referenced in samba v4-13-test:

497b461238bf69eb5ff92c4b849b8f56bbcbac5e
a67cda7159f3c7e9c381a13705011dd9c93742ae
d3a611377bdda70e6940b6f3fff03cc6240f6a5b
bd76f6d47e756692243a77e7628324e333c566a0
8a8872f7070a6f2c89e2ba38d89df0e27bca9f71
cabc5b114dc094e36b4c052ed524757990ec6321
b5e11c10966dcbb9ca4e751c6c378e2f9ed6e358
57800189c5f4a92058ff293f8583805ebcf9928d
1e27b45f49c1a6d610ec498e48b4ed4f6e85c772
7a2a6e0bcb0f9508322e940360b95eae52572cb2
b7d16fdc65397114bcc9199bbd4092f54d11e565