CVE-2020-25719 mit-samba: If we use client_princ, always lookup the db entry

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14561

Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
[abartlet@samba.org backported due to support for MIT KDB < 10
 in Samba 4.14]

diff --git a/source4/kdc/mit-kdb/kdb_samba_policies.c b/source4/kdc/mit-kdb/kdb_samba_policies.c
index 9197551ed6198dd28ba3b9b39797926aeff2cf8c..dce87c500497ca622c5602f32abe9a187c004ed9 100644 (file)
--- a/source4/kdc/mit-kdb/kdb_samba_policies.c
+++ b/source4/kdc/mit-kdb/kdb_samba_policies.c
@@ -323,6 +323,8 @@ krb5_error_code kdb_samba_db_sign_auth_data(krb5_context context,
                                            krb5_authdata ***signed_auth_data)
 {
 #endif
+       krb5_const_principal ks_client_princ = NULL;
+       krb5_db_entry *client_entry = NULL;
        krb5_authdata **authdata = NULL;
        krb5_boolean is_as_req;
        krb5_error_code code;
@@ -341,8 +343,72 @@ krb5_error_code kdb_samba_db_sign_auth_data(krb5_context context,
 
        is_as_req = ((flags & KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY) != 0);
 
+       /*
+        * When using s4u2proxy client_princ actually refers to the proxied user
+        * while client->princ to the proxy service asking for the TGS on behalf
+        * of the proxied user. So always use client_princ in preference.
+        *
+        * Note that when client principal is not NULL, client entry might be
+        * NULL for cross-realm case, so we need to make sure to not
+        * dereference NULL pointer here.
+        */
+       if (client_princ != NULL) {
+               ks_client_princ = client_princ;
+               if (!is_as_req) {
+                       krb5_boolean is_equal = false;
+
+                       if (client != NULL && client->princ != NULL) {
+                               is_equal =
+                                       krb5_principal_compare(context,
+                                                              client_princ,
+                                                              client->princ);
+                       }
+
+                       /*
+                        * When client principal is the same as supplied client
+                        * entry, don't fetch it.
+                        */
+                       if (!is_equal) {
+                               code = ks_get_principal(context,
+                                                       ks_client_princ,
+                                                       0,
+                                                       &client_entry);
+                               if (code != 0) {
+                                       char *client_name = NULL;
+
+                                       (void)krb5_unparse_name(context,
+                                                               ks_client_princ,
+                                                               &client_name);
+
+                                       DBG_DEBUG("We didn't find the client "
+                                                 "principal [%s] in our "
+                                                 "database.\n",
+                                                 client_name);
+                                       SAFE_FREE(client_name);
+
+                                       /*
+                                        * If we didn't find client_princ in our
+                                        * database it might be from another
+                                        * realm.
+                                        */
+                                       client_entry = NULL;
+                               }
+                       }
+               }
+       } else {
+               if (client == NULL) {
+                       *signed_auth_data = NULL;
+                       return 0;
+               }
+               ks_client_princ = client->princ;
+       }
+
+       if (client_entry == NULL) {
+               client_entry = client;
+       }
+
        if (is_as_req && (flags & KRB5_KDB_FLAG_INCLUDE_PAC)) {
-               code = ks_get_pac(context, client, client_key, &pac);
+               code = ks_get_pac(context, client_entry, client_key, &pac);
                if (code != 0) {
                        goto done;
                }
@@ -351,8 +417,8 @@ krb5_error_code kdb_samba_db_sign_auth_data(krb5_context context,
        if (!is_as_req) {
                code = ks_verify_pac(context,
                                     flags,
-                                    client_princ,
-                                    client,
+                                    ks_client_princ,
+                                    client_entry,
                                     server,
                                     krbtgt,
                                     server_key,
@@ -365,9 +431,9 @@ krb5_error_code kdb_samba_db_sign_auth_data(krb5_context context,
                }
        }
 
-       if (pac == NULL && client != NULL) {
+       if (pac == NULL) {
 
-               code = ks_get_pac(context, client, client_key, &pac);
+               code = ks_get_pac(context, client_entry, client_key, &pac);
                if (code != 0) {
                        goto done;
                }
@@ -378,7 +444,7 @@ krb5_error_code kdb_samba_db_sign_auth_data(krb5_context context,
                goto done;
        }
 
-       code = krb5_pac_sign(context, pac, authtime, client_princ,
+       code = krb5_pac_sign(context, pac, authtime, ks_client_princ,
                        server_key, krbtgt_key, &pac_data);
        if (code != 0) {
                DBG_ERR("krb5_pac_sign failed: %d\n", code);
@@ -412,6 +478,9 @@ krb5_error_code kdb_samba_db_sign_auth_data(krb5_context context,
        code = 0;
 
 done:
+       if (client_entry != NULL && client_entry != client) {
+               ks_free_principal(context, client_entry);
+       }
        krb5_pac_free(context, pac);
        krb5_free_authdata(context, authdata);