Attachment 17701 Details for Bug 15197 – Patches for v4-15-test v4 (rebased on current v4-15-test)

Patches for v4-15-test v4 (rebased on current v4-15-test)

bfixes-tmp415.txt (text/plain), 10.33 KB, created by Stefan Metzmacher on 2022-12-14 16:25:21 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2022-12-14 16:25:21 UTC

Size: 10.33 KB

patch

obsolete

>From fd3cdcc1800a4185857494626de9ba1c368dbcdb Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Tue, 4 Oct 2022 12:25:08 +1300
>Subject: [PATCH 1/3] tests/krb5: Add test requesting a service ticket expiring
> post-2038
>
>Windows 11 22H2 performs such requests, with year 9999.
>The test fails with KDC_ERR_BAD_INTEGRITY on older
>Heimdal versions, which are unable to verify a checksum
>over the modified request body (due to a re-encoding failure).
>
>REF: https://github.com/heimdal/heimdal/issues/1011
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15197
>
>[abartlet@samba.org Add knownfail for backport - as Samba
> 4.15 and earlier fail this test, adapted commit
> 67811e121fbef08337675d473390160793544719 to test
> paraemters in 4.15]
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>(backported from commit 67811e121fbef08337675d473390160793544719)
>---
> python/samba/tests/krb5/kdc_tgs_tests.py | 14 ++++++++++++++
> selftest/knownfail.d/windows11-22h2      |  2 ++
> 2 files changed, 16 insertions(+)
> create mode 100644 selftest/knownfail.d/windows11-22h2
>
>diff --git a/python/samba/tests/krb5/kdc_tgs_tests.py b/python/samba/tests/krb5/kdc_tgs_tests.py
>index e876efe1a6dd..37a13ba90242 100755
>--- a/python/samba/tests/krb5/kdc_tgs_tests.py
>+++ b/python/samba/tests/krb5/kdc_tgs_tests.py
>@@ -178,6 +178,7 @@ class KdcTgsBaseTests(KDCBaseTest):
>                  sname=None,
>                  srealm=None,
>                  use_fast=False,
>+                 till=None,
>                  expect_claims=True,
>                  etypes=None,
>                  expected_ticket_etype=None,
>@@ -294,6 +295,7 @@ class KdcTgsBaseTests(KDCBaseTest):
>                                          cname=None,
>                                          realm=srealm,
>                                          sname=sname,
>+                                         till_time=till,
>                                          etypes=etypes,
>                                          additional_tickets=additional_tickets)
>         if expected_error:
>@@ -2392,6 +2394,18 @@ class KdcTgsTests(KdcTgsBaseTests):
>         self._run_tgs(tgt, expected_error=(KDC_ERR_TGT_REVOKED,
>                                            KDC_ERR_C_PRINCIPAL_UNKNOWN))
> 
>+    # Test making a TGS request for a ticket expiring post-2038.
>+    def test_tgs_req_future_till(self):
>+        creds = self._get_creds()
>+        tgt = self._get_tgt(creds)
>+
>+        target_creds = self.get_service_creds()
>+        self._tgs_req(
>+            tgt=tgt,
>+            expected_error=0,
>+            target_creds=target_creds,
>+            till='99990913024805Z')
>+
>     def _modify_renewable(self, enc_part):
>         # Set the renewable flag.
>         enc_part = self.modify_ticket_flag(enc_part, 'renewable', value=True)
>diff --git a/selftest/knownfail.d/windows11-22h2 b/selftest/knownfail.d/windows11-22h2
>new file mode 100644
>index 000000000000..69980ce763aa
>--- /dev/null
>+++ b/selftest/knownfail.d/windows11-22h2
>@@ -0,0 +1,2 @@
>+# This tests shows the new timestamp from Windows 11 22H2 which fails in this version
>+^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_tgs_req_future_till
>\ No newline at end of file
>-- 
>2.34.1
>
>
>From ff5d6ada80e90e5fd67086e52f7e82f91bbafcc0 Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Thu, 20 Oct 2022 12:36:44 +1300
>Subject: [PATCH 2/3] tests/krb5: Add test requesting a TGT expiring post-2038
>
>This demonstrates the behaviour of Windows 11 22H2 over Kerberos,
>which changed to use a year 9999 date for a forever timetime in
>tickets.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15197
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>
>Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
>Autobuild-Date(master): Thu Oct 20 05:00:23 UTC 2022 on sn-devel-184
>
>(backported from commit 50cbdecf2e276e5f87b9c2d95fd3ca86d11a08e2)
>
>[abartlet@samba.org Adapted from 50cbdecf2e276e5f87b9c2d95fd3ca86d11a08e2
> as the kerberos tests have changed parameters in newer versions
> breaking the context]
>---
> python/samba/tests/krb5/as_req_tests.py | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
>diff --git a/python/samba/tests/krb5/as_req_tests.py b/python/samba/tests/krb5/as_req_tests.py
>index da2c0b9d0979..0d9a771b80dc 100755
>--- a/python/samba/tests/krb5/as_req_tests.py
>+++ b/python/samba/tests/krb5/as_req_tests.py
>@@ -42,7 +42,7 @@ global_hexdump = False
> 
> class AsReqBaseTest(KDCBaseTest):
>     def _run_as_req_enc_timestamp(self, client_creds, sname=None,
>-                                  expected_error=None,
>+                                  expected_error=None, till=None,
>                                   expected_pa_error=None, expect_pa_edata=None):
>         client_account = client_creds.get_username()
>         client_as_etypes = self.get_default_enctypes()
>@@ -63,7 +63,8 @@ class AsReqBaseTest(KDCBaseTest):
>         expected_sname = sname
>         expected_salt = client_creds.get_salt()
> 
>-        till = self.get_KerberosTime(offset=36000)
>+        if till is None:
>+            till = self.get_KerberosTime(offset=36000)
> 
>         initial_etypes = client_as_etypes
>         initial_kdc_options = krb5_asn1.KDCOptions('forwardable')
>@@ -252,6 +253,14 @@ class AsReqKerberosTests(AsReqBaseTest):
>                 sname=wrong_krbtgt_princ,
>                 expected_error=KDC_ERR_S_PRINCIPAL_UNKNOWN)
> 
>+    # Test that we can make a request for a ticket expiring post-2038.
>+    def test_future_till(self):
>+        client_creds = self.get_client_creds()
>+
>+        self._run_as_req_enc_timestamp(
>+            client_creds,
>+            till='99990913024805Z')
>+
> 
> if __name__ == "__main__":
>     global_asn1_print = False
>-- 
>2.34.1
>
>
>From b5dc88ed58e68db61f109d69bb272da56f318796 Mon Sep 17 00:00:00 2001
>From: Luke Howard <lukeh@padl.com>
>Date: Thu, 20 Oct 2022 13:27:31 +1300
>Subject: [PATCH 3/3] kdc: avoid re-encoding KDC-REQ-BODY
>
>Use --preserve-binary=KDC-REQ-BODY option to ASN.1 compiler to avoid
>re-encoding KDC-REQ-BODYs for verification in GSS preauth, TGS and PKINIT.
>
>[abartlet@samba.org adapted from Heimdal commit
> ebfd48e40a1b61bf5a6b8d00fe5c581e24652b6e
> by removing references to FAST and GSS-pre-auth.
>
> This fixes the Windows 11 22H2 issue with TGS-REQ
> as seen at https://github.com/heimdal/heimdal/issues/1011 and so
> removes the knownfail file for this test]
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15197
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> selftest/knownfail.d/windows11-22h2 |  2 --
> source4/heimdal/kdc/krb5tgs.c       | 24 ++----------------------
> source4/heimdal/kdc/pkinit.c        | 16 ++--------------
> source4/heimdal/lib/asn1/krb5.opt   |  1 +
> 4 files changed, 5 insertions(+), 38 deletions(-)
> delete mode 100644 selftest/knownfail.d/windows11-22h2
>
>diff --git a/selftest/knownfail.d/windows11-22h2 b/selftest/knownfail.d/windows11-22h2
>deleted file mode 100644
>index 69980ce763aa..000000000000
>--- a/selftest/knownfail.d/windows11-22h2
>+++ /dev/null
>@@ -1,2 +0,0 @@
>-# This tests shows the new timestamp from Windows 11 22H2 which fails in this version
>-^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_tgs_req_future_till
>\ No newline at end of file
>diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c
>index b8c8c39a3d47..3461cf0ef575 100644
>--- a/source4/heimdal/kdc/krb5tgs.c
>+++ b/source4/heimdal/kdc/krb5tgs.c
>@@ -780,9 +780,6 @@ tgs_check_authenticator(krb5_context context,
> 			krb5_keyblock *key)
> {
>     krb5_authenticator auth;
>-    size_t len = 0;
>-    unsigned char *buf;
>-    size_t buf_size;
>     krb5_error_code ret;
>     krb5_crypto crypto;
> 
>@@ -808,25 +805,9 @@ tgs_check_authenticator(krb5_context context,
> 	goto out;
>     }
> 
>-    /* XXX should not re-encode this */
>-    ASN1_MALLOC_ENCODE(KDC_REQ_BODY, buf, buf_size, b, &len, ret);
>-    if(ret){
>-	const char *msg = krb5_get_error_message(context, ret);
>-	kdc_log(context, config, 0, "Failed to encode KDC-REQ-BODY: %s", msg);
>-	krb5_free_error_message(context, msg);
>-	goto out;
>-    }
>-    if(buf_size != len) {
>-	free(buf);
>-	kdc_log(context, config, 0, "Internal error in ASN.1 encoder");
>-	*e_text = "KDC internal error";
>-	ret = KRB5KRB_ERR_GENERIC;
>-	goto out;
>-    }
>     ret = krb5_crypto_init(context, key, 0, &crypto);
>     if (ret) {
> 	const char *msg = krb5_get_error_message(context, ret);
>-	free(buf);
> 	kdc_log(context, config, 0, "krb5_crypto_init failed: %s", msg);
> 	krb5_free_error_message(context, msg);
> 	goto out;
>@@ -834,10 +815,9 @@ tgs_check_authenticator(krb5_context context,
>     ret = krb5_verify_checksum(context,
> 			       crypto,
> 			       KRB5_KU_TGS_REQ_AUTH_CKSUM,
>-			       buf,
>-			       len,
>+			       b->_save.data,
>+			       b->_save.length,
> 			       auth->cksum);
>-    free(buf);
>     krb5_crypto_destroy(context, crypto);
>     if(ret){
> 	const char *msg = krb5_get_error_message(context, ret);
>diff --git a/source4/heimdal/kdc/pkinit.c b/source4/heimdal/kdc/pkinit.c
>index ad7f3efc10a7..64ea4c00e41b 100644
>--- a/source4/heimdal/kdc/pkinit.c
>+++ b/source4/heimdal/kdc/pkinit.c
>@@ -113,10 +113,7 @@ pk_check_pkauthenticator(krb5_context context,
> 			 PKAuthenticator *a,
> 			 const KDC_REQ *req)
> {
>-    u_char *buf = NULL;
>-    size_t buf_size;
>     krb5_error_code ret;
>-    size_t len = 0;
>     krb5_timestamp now;
>     Checksum checksum;
> 
>@@ -128,22 +125,13 @@ pk_check_pkauthenticator(krb5_context context,
> 	return KRB5KRB_AP_ERR_SKEW;
>     }
> 
>-    ASN1_MALLOC_ENCODE(KDC_REQ_BODY, buf, buf_size, &req->req_body, &len, ret);
>-    if (ret) {
>-	krb5_clear_error_message(context);
>-	return ret;
>-    }
>-    if (buf_size != len)
>-	krb5_abortx(context, "Internal error in ASN.1 encoder");
>-
>     ret = krb5_create_checksum(context,
> 			       NULL,
> 			       0,
> 			       CKSUMTYPE_SHA1,
>-			       buf,
>-			       len,
>+			       req->req_body._save.data,
>+			       req->req_body._save.length,
> 			       &checksum);
>-    free(buf);
>     if (ret) {
> 	krb5_clear_error_message(context);
> 	return ret;
>diff --git a/source4/heimdal/lib/asn1/krb5.opt b/source4/heimdal/lib/asn1/krb5.opt
>index 1d6d5e8989f1..5acc596d39ce 100644
>--- a/source4/heimdal/lib/asn1/krb5.opt
>+++ b/source4/heimdal/lib/asn1/krb5.opt
>@@ -4,3 +4,4 @@
> --sequence=METHOD-DATA
> --sequence=ETYPE-INFO
> --sequence=ETYPE-INFO2
>+--preserve-binary=KDC-REQ-BODY
>-- 
>2.34.1
>

Flags:

abartlet: review+

Actions: View

Attachments on bug 15197: 17586 | 17587 | 17588 | 17591 | 17592 | 17593 | 17594 | 17595 | 17596 | 17701