Blob Blame History Raw
From 640492ecb4ee42edf33c343c08c01a549ed68a52 Mon Sep 17 00:00:00 2001
From: Julien Rische <jrische@redhat.com>
Date: Wed, 15 Mar 2023 15:56:34 +0100
Subject: [PATCH] [downstream] Allow to set PAC ticket signature as optional

MS-PAC states that "The ticket signature SHOULD be included in tickets
that are not encrypted to the krbtgt account". However, the
implementation of krb5_kdc_verify_ticket() will require the ticket
signature to be present in case the target of the request is a service
principal.

In gradual upgrade environments, it results in S4U2Proxy requests
against a 1.20 KDC using a service ticket generated by an older version
KDC to fail.

This commit adds a krb5_kdc_verify_ticket_ext() function with an extra
switch parameter to tolerate the absence of ticket signature in this
scenario. If the ticket signature is present, it has to be valid,
regardless of this parameter.

This parameter is set based on the "optional_pac_tkt_chksum" string
attribute of the TGT KDB entry.
---
 doc/admin/admin_commands/kadmin_local.rst |  6 ++++
 doc/appdev/refs/api/index.rst             |  1 +
 src/include/kdb.h                         |  1 +
 src/include/krb5/krb5.hin                 | 40 +++++++++++++++++++++++
 src/kdc/kdc_util.c                        | 32 ++++++++++++++----
 src/lib/krb5/krb/pac.c                    | 31 +++++++++++++++---
 src/lib/krb5/libkrb5.exports              |  1 +
 src/man/kadmin.man                        |  6 ++++
 8 files changed, 108 insertions(+), 10 deletions(-)

diff --git a/doc/admin/admin_commands/kadmin_local.rst b/doc/admin/admin_commands/kadmin_local.rst
index 2435b3c361..58ac79549f 100644
--- a/doc/admin/admin_commands/kadmin_local.rst
+++ b/doc/admin/admin_commands/kadmin_local.rst
@@ -658,6 +658,12 @@ KDC:
     Directory realm when using aes-sha2 keys on the local krbtgt
     entry.
 
+**optional_pac_tkt_chksum**
+    Boolean value defining the behavior of the KDC in case an expected
+    ticket checksum signed with one of this principal keys is not
+    present in the PAC. This is typically the case for TGS or
+    cross-realm TGS principals when processing S4U2Proxy requests.
+
 This command requires the **modify** privilege.
 
 Alias: **setstr**
diff --git a/doc/appdev/refs/api/index.rst b/doc/appdev/refs/api/index.rst
index d12be47c3c..9b95ebd0f9 100644
--- a/doc/appdev/refs/api/index.rst
+++ b/doc/appdev/refs/api/index.rst
@@ -225,6 +225,7 @@ Rarely used public interfaces
    krb5_is_referral_realm.rst
    krb5_kdc_sign_ticket.rst
    krb5_kdc_verify_ticket.rst
+   krb5_kdc_verify_ticket_ext.rst
    krb5_kt_add_entry.rst
    krb5_kt_end_seq_get.rst
    krb5_kt_get_entry.rst
diff --git a/src/include/kdb.h b/src/include/kdb.h
index 745b24f351..6075349e5e 100644
--- a/src/include/kdb.h
+++ b/src/include/kdb.h
@@ -136,6 +136,7 @@
 #define KRB5_KDB_SK_PAC_PRIVSVR_ENCTYPE         "pac_privsvr_enctype"
 #define KRB5_KDB_SK_SESSION_ENCTYPES            "session_enctypes"
 #define KRB5_KDB_SK_REQUIRE_AUTH                "require_auth"
+#define KRB5_KDB_SK_OPTIONAL_PAC_TKT_CHKSUM     "optional_pac_tkt_chksum"
 
 #if !defined(_WIN32)
 
diff --git a/src/include/krb5/krb5.hin b/src/include/krb5/krb5.hin
index 350bcf86f2..17e1b52266 100644
--- a/src/include/krb5/krb5.hin
+++ b/src/include/krb5/krb5.hin
@@ -8356,6 +8356,46 @@ krb5_kdc_verify_ticket(krb5_context context, const krb5_enc_tkt_part *enc_tkt,
                        const krb5_keyblock *server,
                        const krb5_keyblock *privsvr, krb5_pac *pac_out);
 
+/**
+ * Verify a PAC, possibly including ticket signature
+ *
+ * @param [in] context              Library context
+ * @param [in] enc_tkt              Ticket enc-part, possibly containing a PAC
+ * @param [in] server_princ         Canonicalized name of ticket server
+ * @param [in] server               Key to validate server checksum (or NULL)
+ * @param [in] privsvr              Key to validate KDC checksum (or NULL)
+ * @paran [in] optional_tkt_chksum  Whether to require a ticket checksum
+ * @param [out] pac_out             Verified PAC (NULL if no PAC included)
+ *
+ * This function is an extension of krb5_kdc_verify_ticket(), adding the @a
+ * optional_tkt_chksum parameter allowing to tolerate the absence of the PAC
+ * ticket signature.
+ *
+ * If a PAC is present in @a enc_tkt, verify its signatures.  If @a privsvr is
+ * not NULL and @a server_princ is not a krbtgt or kadmin/changepw service and
+ * @a optional_tkt_chksum is FALSE, require a ticket signature over @a enc_tkt
+ * in addition to the KDC signature. Place the verified PAC in @a pac_out.  If
+ * an invalid PAC signature is found, return an error matching the Windows KDC
+ * protocol code for that condition as closely as possible.
+ *
+ * If no PAC is present in @a enc_tkt, set @a pac_out to NULL and return
+ * successfully.
+ *
+ * @note This function does not validate the PAC_CLIENT_INFO buffer.  If a
+ * specific value is expected, the caller can make a separate call to
+ * krb5_pac_verify_ext() with a principal but no keys.
+ *
+ * @retval 0 Success; otherwise - Kerberos error codes
+ */
+krb5_error_code KRB5_CALLCONV
+krb5_kdc_verify_ticket_ext(krb5_context context,
+                           const krb5_enc_tkt_part *enc_tkt,
+                           krb5_const_principal server_princ,
+                           const krb5_keyblock *server,
+                           const krb5_keyblock *privsvr,
+                           krb5_boolean optional_tkt_chksum,
+                           krb5_pac *pac_out);
+
 /** @deprecated Use krb5_kdc_sign_ticket() instead. */
 krb5_error_code KRB5_CALLCONV
 krb5_pac_sign(krb5_context context, krb5_pac pac, krb5_timestamp authtime,
diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c
index fe4e48209a..93415ba862 100644
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -560,16 +560,36 @@ cleanup:
 static krb5_error_code
 try_verify_pac(krb5_context context, const krb5_enc_tkt_part *enc_tkt,
                krb5_db_entry *server, krb5_keyblock *server_key,
-               const krb5_keyblock *tgt_key, krb5_pac *pac_out)
+               krb5_db_entry *tgt, const krb5_keyblock *tgt_key,
+               krb5_pac *pac_out)
 {
     krb5_error_code ret;
+    krb5_boolean optional_tkt_chksum;
+    char *str = NULL;
     krb5_keyblock *privsvr_key;
 
     ret = pac_privsvr_key(context, server, tgt_key, &privsvr_key);
     if (ret)
         return ret;
-    ret = krb5_kdc_verify_ticket(context, enc_tkt, server->princ, server_key,
-                                 privsvr_key, pac_out);
+
+    /* Check if the absence of ticket signature is tolerated for this realm */
+    ret = krb5_dbe_get_string(context, tgt,
+                              KRB5_KDB_SK_OPTIONAL_PAC_TKT_CHKSUM, &str);
+    /* TODO: should be using _krb5_conf_boolean(), but os-proto.h is not
+     * available here.
+     */
+    optional_tkt_chksum = !ret && str && (strncasecmp(str, "true", 4) == 0
+                                       || strncasecmp(str, "t",    1) == 0
+                                       || strncasecmp(str, "yes",  3) == 0
+                                       || strncasecmp(str, "y",    1) == 0
+                                       || strncasecmp(str, "1",    1) == 0
+                                       || strncasecmp(str, "on",   2) == 0);
+
+    krb5_dbe_free_string(context, str);
+
+    ret = krb5_kdc_verify_ticket_ext(context, enc_tkt, server->princ,
+                                     server_key, privsvr_key,
+                                     optional_tkt_chksum, pac_out);
     krb5_free_keyblock(context, privsvr_key);
     return ret;
 }
@@ -599,7 +619,7 @@ get_verified_pac(krb5_context context, const krb5_enc_tkt_part *enc_tkt,
                                       server_key, NULL, pac_out);
     }
 
-    ret = try_verify_pac(context, enc_tkt, server, server_key, tgt_key,
+    ret = try_verify_pac(context, enc_tkt, server, server_key, tgt, tgt_key,
                          pac_out);
     if (ret != KRB5KRB_AP_ERR_MODIFIED && ret != KRB5_BAD_ENCTYPE)
         return ret;
@@ -613,8 +633,8 @@ get_verified_pac(krb5_context context, const krb5_enc_tkt_part *enc_tkt,
         ret = krb5_dbe_decrypt_key_data(context, NULL, kd, &old_key, NULL);
         if (ret)
             return ret;
-        ret = try_verify_pac(context, enc_tkt, server, server_key, &old_key,
-                             pac_out);
+        ret = try_verify_pac(context, enc_tkt, server, server_key, tgt,
+                             &old_key, pac_out);
         krb5_free_keyblock_contents(context, &old_key);
         if (!ret)
             return 0;
diff --git a/src/lib/krb5/krb/pac.c b/src/lib/krb5/krb/pac.c
index 5d1fdf1ba0..0c0e2ada68 100644
--- a/src/lib/krb5/krb/pac.c
+++ b/src/lib/krb5/krb/pac.c
@@ -594,6 +594,19 @@ krb5_kdc_verify_ticket(krb5_context context, const krb5_enc_tkt_part *enc_tkt,
                        krb5_const_principal server_princ,
                        const krb5_keyblock *server,
                        const krb5_keyblock *privsvr, krb5_pac *pac_out)
+{
+    return krb5_kdc_verify_ticket_ext(context, enc_tkt, server_princ, server,
+                                      privsvr, FALSE, pac_out);
+}
+
+krb5_error_code KRB5_CALLCONV
+krb5_kdc_verify_ticket_ext(krb5_context context,
+                           const krb5_enc_tkt_part *enc_tkt,
+                           krb5_const_principal server_princ,
+                           const krb5_keyblock *server,
+                           const krb5_keyblock *privsvr,
+                           krb5_boolean optional_tkt_chksum,
+                           krb5_pac *pac_out)
 {
     krb5_error_code ret;
     krb5_pac pac = NULL;
@@ -602,7 +615,7 @@ krb5_kdc_verify_ticket(krb5_context context, const krb5_enc_tkt_part *enc_tkt,
     krb5_authdata *orig, **ifrel = NULL, **recoded_ifrel = NULL;
     uint8_t z = 0;
     krb5_authdata zpac = { KV5M_AUTHDATA, KRB5_AUTHDATA_WIN2K_PAC, 1, &z };
-    krb5_boolean is_service_tkt;
+    krb5_boolean is_service_tkt, has_tkt_chksum = FALSE;
     size_t i, j;
 
     *pac_out = NULL;
@@ -667,11 +680,21 @@ krb5_kdc_verify_ticket(krb5_context context, const krb5_enc_tkt_part *enc_tkt,
 
         ret = verify_checksum(context, pac, KRB5_PAC_TICKET_CHECKSUM, privsvr,
                               KRB5_KEYUSAGE_APP_DATA_CKSUM, recoded_tkt);
-        if (ret)
-            goto cleanup;
+        if (ret) {
+            if (!optional_tkt_chksum)
+                goto cleanup;
+            else if (ret != ENOENT)
+                goto cleanup;
+            /* Otherwise ticket signature is absent but optional. Proceed... */
+        } else {
+            has_tkt_chksum = TRUE;
+        }
     }
+    /* Else, we make the assumption the ticket signature is absent in case this
+     * is not a service ticket.
+     */
 
-    ret = verify_pac_checksums(context, pac, is_service_tkt, server, privsvr);
+    ret = verify_pac_checksums(context, pac, has_tkt_chksum, server, privsvr);
     if (ret)
         goto cleanup;
 
diff --git a/src/lib/krb5/libkrb5.exports b/src/lib/krb5/libkrb5.exports
index 4c50e935a2..d4b0455c8c 100644
--- a/src/lib/krb5/libkrb5.exports
+++ b/src/lib/krb5/libkrb5.exports
@@ -463,6 +463,7 @@ krb5_is_thread_safe
 krb5_kdc_rep_decrypt_proc
 krb5_kdc_sign_ticket
 krb5_kdc_verify_ticket
+krb5_kdc_verify_ticket_ext
 krb5_kt_add_entry
 krb5_kt_client_default
 krb5_kt_close
diff --git a/src/man/kadmin.man b/src/man/kadmin.man
index 461207021b..e8d78309cb 100644
--- a/src/man/kadmin.man
+++ b/src/man/kadmin.man
@@ -724,6 +724,12 @@ encryption type.  It may be necessary to set this value to
 "aes256\-sha1" on the cross\-realm krbtgt entry for an Active
 Directory realm when using aes\-sha2 keys on the local krbtgt
 entry.
+.TP
+\fBoptional_pac_tkt_chksum\fP
+Boolean value defining the behavior of the KDC in case an expected ticket
+checksum signed with one of this principal keys is not present in the PAC. This
+is typically the case for TGS or cross-realm TGS principals when processing
+S4U2Proxy requests.
 .UNINDENT
 .sp
 This command requires the \fBmodify\fP privilege.
-- 
2.41.0