The branch, v4-9-stable has been updated
       via  9dfd4419b50 VERSION: Disable GIT_SNAPSHOT for the 4.9.8 release.
       via  ff8e3fbae80 WHATSNEW: Add release notes for Samba 4.9.8.
       via  de3fa5d6b94 CVE-2018-16860 Heimdal KDC: Reject PA-S4U2Self with 
unkeyed checksum
       via  52200468716 CVE-2018-16860 selftest: Add test for S4U2Self with 
unkeyed checksum
       via  bc1b0ade6ff VERSION: Bump version up to 4.9.8...
      from  c8e9b9fe7cc VERSION: Disable GIT_SNAPSHOT for the 4.9.7 release.

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-9-stable


- Log -----------------------------------------------------------------
commit 9dfd4419b50b17ed916957372829057af8e27893
Author: Karolin Seeger <ksee...@samba.org>
Date:   Tue May 7 12:25:56 2019 +0200

    VERSION: Disable GIT_SNAPSHOT for the 4.9.8 release.
    
    CVE-2018-16860 (Samba AD DC S4U2Self/S4U2Proxy unkeyed checksum)
    
    Signed-off-by: Karolin Seeger <ksee...@samba.org>

commit ff8e3fbae80e62f1b0f8b638a171e913a14b231a
Author: Karolin Seeger <ksee...@samba.org>
Date:   Tue May 7 12:24:55 2019 +0200

    WHATSNEW: Add release notes for Samba 4.9.8.
    
    CVE-2018-16860 (Samba AD DC S4U2Self/S4U2Proxy unkeyed checksum)
    
    Signed-off-by: Karolin Seeger <ksee...@samba.org>

commit de3fa5d6b9462bd8c5bc01cf1ae89fa997009ae7
Author: Isaac Boukris <ibouk...@gmail.com>
Date:   Wed Jan 30 23:49:07 2019 +0200

    CVE-2018-16860 Heimdal KDC: Reject PA-S4U2Self with unkeyed checksum
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13685
    
    Signed-off-by: Isaac Boukris <ibouk...@gmail.com>
    Reviewed-by: Andrew Bartlett <abart...@samba.org>
    Signed-off-by: Andrew Bartlett <abart...@samba.org>

commit 522004687162c3dfad87581ce930b21c9ecdf834
Author: Isaac Boukris <ibouk...@gmail.com>
Date:   Thu Apr 25 22:12:10 2019 +1200

    CVE-2018-16860 selftest: Add test for S4U2Self with unkeyed checksum
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13685
    
    Signed-off-by: Isaac Boukris <ibouk...@gmail.com>
    Signed-off-by: Andrew Bartlett <abart...@samba.org>

commit bc1b0ade6ff84fd16fa58d357497b317ba04cbff
Author: Karolin Seeger <ksee...@samba.org>
Date:   Tue Apr 16 12:39:04 2019 +0200

    VERSION: Bump version up to 4.9.8...
    
    and re-enable GIT_SNAPSHOT.
    
    Signed-off-by: Karolin Seeger <ksee...@samba.org>
    (cherry picked from commit 86de3470b4c342857d1c8408929ef4637fdf1937)

-----------------------------------------------------------------------

Summary of changes:
 VERSION                                  |   2 +-
 WHATSNEW.txt                             |  58 +++++++++++++++-
 source4/heimdal/kdc/krb5tgs.c            |   7 ++
 source4/torture/krb5/kdc-canon-heimdal.c | 115 +++++++++++++++++++++++++++++--
 4 files changed, 175 insertions(+), 7 deletions(-)


Changeset truncated at 500 lines:

diff --git a/VERSION b/VERSION
index ba6fe8a24b9..1833b6c24d0 100644
--- a/VERSION
+++ b/VERSION
@@ -25,7 +25,7 @@
 ########################################################
 SAMBA_VERSION_MAJOR=4
 SAMBA_VERSION_MINOR=9
-SAMBA_VERSION_RELEASE=7
+SAMBA_VERSION_RELEASE=8
 
 ########################################################
 # If a official release has a serious bug              #
diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index d135527fa1b..b0191a14442 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -1,3 +1,57 @@
+                   =============================
+                   Release Notes for Samba 4.9.8
+                            May 14, 2019
+                   =============================
+
+
+This is a security release in order to address the following defect:
+
+o  CVE-2018-16860 (Samba AD DC S4U2Self/S4U2Proxy unkeyed checksum)
+
+
+=======
+Details
+=======
+
+o  CVE-2018-16860:
+   The checksum validation in the S4U2Self handler in the embedded Heimdal KDC
+   did not first confirm that the checksum was keyed, allowing replacement of
+   the requested target (client) principal.
+
+For more details and workarounds, please refer to the security advisory.
+
+
+Changes since 4.9.7:
+--------------------
+
+o  Isaac Boukris <ibouk...@gmail.com>
+   * BUG 13685: CVE-2018-16860: Heimdal KDC: Reject PA-S4U2Self with unkeyed
+     checksum.
+
+
+#######################################
+Reporting bugs & Development Discussion
+#######################################
+
+Please discuss this release on the samba-technical mailing list or by
+joining the #samba-technical IRC channel on irc.freenode.net.
+
+If you do report problems then please try to send high quality
+feedback. If you don't provide vital information to help us track down
+the problem then you will probably be ignored.  All bug reports should
+be filed under the "Samba 4.1 and newer" product in the project's Bugzilla
+database (https://bugzilla.samba.org/).
+
+
+======================================================================
+== Our Code, Our Bugs, Our Responsibility.
+== The Samba Team
+======================================================================
+
+
+Release notes for older releases follow:
+----------------------------------------
+
                    =============================
                    Release Notes for Samba 4.9.7
                             May 1, 2019
@@ -111,8 +165,8 @@ database (https://bugzilla.samba.org/).
 ======================================================================
 
 
-Release notes for older releases follow:
-----------------------------------------
+----------------------------------------------------------------------
+
 
                    =============================
                    Release Notes for Samba 4.9.6
diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c
index a888788bb6f..ff7d93138c0 100644
--- a/source4/heimdal/kdc/krb5tgs.c
+++ b/source4/heimdal/kdc/krb5tgs.c
@@ -1925,6 +1925,13 @@ server_lookup:
                goto out;
            }
 
+           if (!krb5_checksum_is_keyed(context, self.cksum.cksumtype)) {
+               free_PA_S4U2Self(&self);
+               kdc_log(context, config, 0, "Reject PA-S4U2Self with unkeyed 
checksum");
+               ret = KRB5KRB_AP_ERR_INAPP_CKSUM;
+               goto out;
+           }
+
            ret = _krb5_s4u2self_to_checksumdata(context, &self, &datack);
            if (ret)
                goto out;
diff --git a/source4/torture/krb5/kdc-canon-heimdal.c 
b/source4/torture/krb5/kdc-canon-heimdal.c
index 5b782a23fc4..fdca1ca414c 100644
--- a/source4/torture/krb5/kdc-canon-heimdal.c
+++ b/source4/torture/krb5/kdc-canon-heimdal.c
@@ -43,7 +43,9 @@
 #define TEST_UPN              0x0000040
 #define TEST_S4U2SELF         0x0000080
 #define TEST_REMOVEDOLLAR     0x0000100
-#define TEST_ALL              0x00001FF
+#define TEST_AS_REQ_SPN       0x0000200 /* not used */
+#define TEST_MITM_S4U2SELF    0x0000400
+#define TEST_ALL              0x00007FF
 
 struct test_data {
        const char *test_name;
@@ -61,6 +63,7 @@ struct test_data {
        bool upn;
        bool other_upn_suffix;
        bool s4u2self;
+       bool mitm_s4u2self;
        bool removedollar;
        const char *krb5_service;
        const char *krb5_hostname;
@@ -209,6 +212,67 @@ static bool test_accept_ticket(struct torture_context 
*tctx,
        return true;
 }
 
+krb5_error_code
+_krb5_s4u2self_to_checksumdata(krb5_context context,
+                              const PA_S4U2Self *self,
+                              krb5_data *data);
+
+/* Helper function to modify the principal in PA_FOR_USER padata */
+static bool change_for_user_principal(struct torture_krb5_context 
*test_context,
+                                     krb5_data *modified_send_buf)
+{
+       PA_DATA *for_user;
+       int i = 0;
+       size_t used;
+       krb5_error_code ret;
+       PA_S4U2Self self, mod_self;
+       krb5_data cksum_data;
+       krb5_principal admin;
+       heim_octet_string orig_padata_value;
+       krb5_context k5_ctx = test_context->smb_krb5_context->krb5_context;
+
+       for_user = krb5_find_padata(test_context->tgs_req.padata->val,
+                                   test_context->tgs_req.padata->len, 
KRB5_PADATA_FOR_USER, &i);
+       torture_assert(test_context->tctx, for_user != NULL, "No PA_FOR_USER in 
s4u2self request");
+       orig_padata_value = for_user->padata_value;
+
+       torture_assert_int_equal(test_context->tctx,
+                                krb5_make_principal(k5_ctx, &admin, 
test_context->test_data->realm,
+                                                    "Administrator", NULL),
+                                0, "krb5_make_principal() failed");
+       torture_assert_int_equal(test_context->tctx,
+                                decode_PA_S4U2Self(for_user->padata_value.data,
+                                                   
for_user->padata_value.length, &self, NULL),
+                                0, "decode_PA_S4U2Self() failed");
+       mod_self = self;
+       mod_self.name = admin->name;
+
+       torture_assert_int_equal(test_context->tctx,
+                                _krb5_s4u2self_to_checksumdata(k5_ctx, 
&mod_self, &cksum_data),
+                                0, "_krb5_s4u2self_to_checksumdata() failed");
+       torture_assert_int_equal(test_context->tctx,
+                                krb5_create_checksum(k5_ctx, NULL, 
KRB5_KU_OTHER_CKSUM,
+                                                     CKSUMTYPE_CRC32, 
cksum_data.data,
+                                                     cksum_data.length, 
&mod_self.cksum),
+                                0, "krb5_create_checksum() failed");
+
+       ASN1_MALLOC_ENCODE(PA_S4U2Self, for_user->padata_value.data, 
for_user->padata_value.length,
+                          &mod_self, &used, ret);
+       torture_assert(test_context->tctx, ret == 0, "Failed to encode 
PA_S4U2Self ASN1 struct");
+       ASN1_MALLOC_ENCODE(TGS_REQ, modified_send_buf->data, 
modified_send_buf->length,
+                          &test_context->tgs_req, &used, ret);
+       torture_assert(test_context->tctx, ret == 0, "Failed to encode TGS_REQ 
ASN1 struct");
+
+       free(for_user->padata_value.data);
+       for_user->padata_value = orig_padata_value;
+
+       free_PA_S4U2Self(&self);
+       krb5_data_free(&cksum_data);
+       free_Checksum(&mod_self.cksum);
+
+       return true;
+}
+
 /*
  * TEST_AS_REQ and TEST_AS_REQ_SELF - SEND
  *
@@ -598,7 +662,12 @@ static bool 
torture_krb5_pre_send_tgs_req_canon_test(struct torture_krb5_context
 
        }
 
-       *modified_send_buf = *send_buf;
+       if (test_context->test_data->mitm_s4u2self) {
+               torture_assert(test_context->tctx, 
change_for_user_principal(test_context, modified_send_buf),
+                              "Failed to modify PA_FOR_USER principal name");
+       } else {
+               *modified_send_buf = *send_buf;
+       }
 
        return true;
 }
@@ -617,6 +686,7 @@ static bool 
torture_krb5_post_recv_tgs_req_canon_test(struct torture_krb5_contex
 {
        KRB_ERROR error;
        size_t used;
+       krb5_error_code expected_error;
 
        /*
         * If this account did not have a servicePrincipalName, then
@@ -627,9 +697,13 @@ static bool 
torture_krb5_post_recv_tgs_req_canon_test(struct torture_krb5_contex
                torture_assert_int_equal(test_context->tctx,
                                         error.pvno, 5,
                                         "Got wrong error.pvno");
+               expected_error = KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN - 
KRB5KDC_ERR_NONE;
+               if (error.error_code != expected_error && 
test_context->test_data->mitm_s4u2self) {
+                       expected_error = KRB5KRB_AP_ERR_INAPP_CKSUM - 
KRB5KDC_ERR_NONE;
+               }
                torture_assert_int_equal(test_context->tctx,
                                         error.error_code,
-                                        KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN - 
KRB5KDC_ERR_NONE,
+                                        expected_error,
                                         "Got wrong error.error_code");
        } else {
                torture_assert_int_equal(test_context->tctx,
@@ -672,6 +746,8 @@ static bool 
torture_krb5_post_recv_tgs_req_canon_test(struct torture_krb5_contex
                torture_assert_int_equal(test_context->tctx,
                                         
*test_context->tgs_rep.ticket.enc_part.kvno & 0xFFFF0000,
                                         0, "Unexpecedly got a RODC number in 
the KVNO, should just be principal KVNO");
+               torture_assert(test_context->tctx, 
test_context->test_data->mitm_s4u2self == false,
+                              "KDC accepted PA_S4U2Self with unkeyed 
checksum!");
                free_TGS_REP(&test_context->tgs_rep);
        }
        torture_assert(test_context->tctx, test_context->packet_count == 0, 
"too many packets");
@@ -1845,7 +1921,23 @@ static bool torture_krb5_as_req_canon(struct 
torture_context *tctx, const void *
                 */
                if (torture_setting_bool(tctx, "expect_machine_account", false)
                    && (test_data->enterprise || test_data->upn == false)) {
+
+                       if (test_data->mitm_s4u2self) {
+                               torture_assert_int_equal(tctx, k5ret, 
KRB5KRB_AP_ERR_INAPP_CKSUM,
+                                                        assertion_message);
+                               /* Done testing mitm-s4u2self */
+                               return true;
+                       }
+
                        torture_assert_int_equal(tctx, k5ret, 0, 
assertion_message);
+
+                       /* Check that the impersonate principal is not being 
canonicalized by the KDC. */
+                       if (test_data->s4u2self) {
+                               torture_assert(tctx, 
krb5_principal_compare(k5_context, server_creds->client,
+                                                                           
principal),
+                                              "TGS-REP cname does not match 
requested client principal");
+                       }
+
                        torture_assert_int_equal(tctx, 
krb5_cc_store_cred(k5_context,
                                                                          
ccache, server_creds),
                                                 0, "krb5_cc_store_cred 
failed");
@@ -2222,11 +2314,25 @@ struct torture_suite *torture_krb5_canon(TALLOC_CTX 
*mem_ctx)
                                             (i & TEST_NETBIOS_REALM) ? 
"netbios-realm" : "krb5-realm",
                                             (i & TEST_WIN2K) ? "win2k" : 
"no-win2k",
                                             (i & TEST_UPN) ? "upn" : "no-upn",
-                                            (i & TEST_S4U2SELF) ? "s4u2self" : 
"normal",
+                                            (i & TEST_S4U2SELF) ? (i & 
TEST_MITM_S4U2SELF) ? "mitm-s4u2self" : "s4u2self" : "normal",
                                             (i & TEST_REMOVEDOLLAR) ? 
"removedollar" : "keepdollar");
 
                struct test_data *test_data = talloc_zero(suite, struct 
test_data);
 
+               if (i & TEST_MITM_S4U2SELF) {
+                       if (!(i & TEST_S4U2SELF)) {
+                               continue;
+                       }
+               }
+
+               /*
+                * Due to backport: this flag is not used until Samba
+                * 4.10
+                */
+               if (i & TEST_AS_REQ_SPN) {
+                       continue;
+               }
+
                test_data->test_name = name;
                test_data->real_realm
                        = strupper_talloc(test_data,
@@ -2246,6 +2352,7 @@ struct torture_suite *torture_krb5_canon(TALLOC_CTX 
*mem_ctx)
                test_data->win2k = (i & TEST_WIN2K) != 0;
                test_data->upn = (i & TEST_UPN) != 0;
                test_data->s4u2self = (i & TEST_S4U2SELF) != 0;
+               test_data->mitm_s4u2self = (i & TEST_MITM_S4U2SELF) != 0;
                test_data->removedollar = (i & TEST_REMOVEDOLLAR) != 0;
                torture_suite_add_simple_tcase_const(suite, name, 
torture_krb5_as_req_canon,
                                                     test_data);


-- 
Samba Shared Repository

Reply via email to