Attachment 13863 Details for Bug 13131 – Work in progress patches

[patch] Work in progress patches

tmp.diff.txt (text/plain), 8.31 KB, created by Stefan Metzmacher on 2017-12-13 12:37:56 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2017-12-13 12:37:56 UTC

Size: 8.31 KB

patch

obsolete

>From addd78907b330b9e1e9a3e8f5f7f821bcdd99ac1 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 20 Sep 2017 23:05:09 +0200
>Subject: [PATCH 1/3] HEIMDAL:kdc: fix memory leak when decryption
> AuthorizationData
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13131
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source4/heimdal/kdc/krb5tgs.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c
>index a888788..6bc4b6f 100644
>--- a/source4/heimdal/kdc/krb5tgs.c
>+++ b/source4/heimdal/kdc/krb5tgs.c
>@@ -1388,11 +1388,13 @@ tgs_parse_request(krb5_context context,
> 	}
> 	ALLOC(*auth_data);
> 	if (*auth_data == NULL) {
>+	    krb5_data_free(&ad);
> 	    krb5_auth_con_free(context, ac);
> 	    ret = KRB5KRB_AP_ERR_BAD_INTEGRITY; /* ? */
> 	    goto out;
> 	}
> 	ret = decode_AuthorizationData(ad.data, ad.length, *auth_data, NULL);
>+	krb5_data_free(&ad);
> 	if(ret){
> 	    krb5_auth_con_free(context, ac);
> 	    free(*auth_data);
>-- 
>1.9.1
>
>
>From 2a7add06e02c319f822d1a52010c513126751fde Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 20 Sep 2017 23:05:09 +0200
>Subject: [PATCH 2/3] HEIMDAL:kdc: decrypt b->enc_authorization_data in
> tgs_build_reply()
>
>We do this after checking for constraint delegation (S4U2Proxy).
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13131
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source4/heimdal/kdc/krb5tgs.c | 115 ++++++++++++++++++++----------------------
> 1 file changed, 56 insertions(+), 59 deletions(-)
>
>diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c
>index 6bc4b6f..f5b4137 100644
>--- a/source4/heimdal/kdc/krb5tgs.c
>+++ b/source4/heimdal/kdc/krb5tgs.c
>@@ -1159,7 +1159,6 @@ tgs_parse_request(krb5_context context,
> 		  const struct sockaddr *from_addr,
> 		  time_t **csec,
> 		  int **cusec,
>-		  AuthorizationData **auth_data,
> 		  krb5_keyblock **replykey,
> 		  int *rk_is_subkey)
> {
>@@ -1170,14 +1169,11 @@ tgs_parse_request(krb5_context context,
>     krb5_auth_context ac = NULL;
>     krb5_flags ap_req_options;
>     krb5_flags verify_ap_req_flags;
>-    krb5_crypto crypto;
>     Key *tkey;
>     krb5_keyblock *subkey = NULL;
>-    unsigned usage;
>     krb5uint32 kvno = 0;
>     krb5uint32 *kvno_ptr = NULL;
> 
>-    *auth_data = NULL;
>     *csec  = NULL;
>     *cusec = NULL;
>     *replykey = NULL;
>@@ -1328,7 +1324,6 @@ tgs_parse_request(krb5_context context,
> 	goto out;
>     }
> 
>-    usage = KRB5_KU_TGS_REQ_AUTH_DAT_SUBKEY;
>     *rk_is_subkey = 1;
> 
>     ret = krb5_auth_con_getremotesubkey(context, ac, &subkey);
>@@ -1340,7 +1335,6 @@ tgs_parse_request(krb5_context context,
> 	goto out;
>     }
>     if(subkey == NULL){
>-	usage = KRB5_KU_TGS_REQ_AUTH_DAT_SESSION;
> 	*rk_is_subkey = 0;
> 
> 	ret = krb5_auth_con_getkey(context, ac, &subkey);
>@@ -1362,49 +1356,6 @@ tgs_parse_request(krb5_context context,
> 
>     *replykey = subkey;
> 
>-    if (b->enc_authorization_data) {
>-	krb5_data ad;
>-
>-	ret = krb5_crypto_init(context, subkey, 0, &crypto);
>-	if (ret) {
>-	    const char *msg = krb5_get_error_message(context, ret);
>-	    krb5_auth_con_free(context, ac);
>-	    kdc_log(context, config, 0, "krb5_crypto_init failed: %s", msg);
>-	    krb5_free_error_message(context, msg);
>-	    goto out;
>-	}
>-	ret = krb5_decrypt_EncryptedData (context,
>-					  crypto,
>-					  usage,
>-					  b->enc_authorization_data,
>-					  &ad);
>-	krb5_crypto_destroy(context, crypto);
>-	if(ret){
>-	    krb5_auth_con_free(context, ac);
>-	    kdc_log(context, config, 0,
>-		    "Failed to decrypt enc-authorization-data");
>-	    ret = KRB5KRB_AP_ERR_BAD_INTEGRITY; /* ? */
>-	    goto out;
>-	}
>-	ALLOC(*auth_data);
>-	if (*auth_data == NULL) {
>-	    krb5_data_free(&ad);
>-	    krb5_auth_con_free(context, ac);
>-	    ret = KRB5KRB_AP_ERR_BAD_INTEGRITY; /* ? */
>-	    goto out;
>-	}
>-	ret = decode_AuthorizationData(ad.data, ad.length, *auth_data, NULL);
>-	krb5_data_free(&ad);
>-	if(ret){
>-	    krb5_auth_con_free(context, ac);
>-	    free(*auth_data);
>-	    *auth_data = NULL;
>-	    kdc_log(context, config, 0, "Failed to decode authorization data");
>-	    ret = KRB5KRB_AP_ERR_BAD_INTEGRITY; /* ? */
>-	    goto out;
>-	}
>-    }
>-
>     krb5_auth_con_free(context, ac);
> 
> out:
>@@ -1502,7 +1453,6 @@ tgs_build_reply(krb5_context context,
> 		krb5_data *reply,
> 		const char *from,
> 		const char **e_text,
>-		AuthorizationData **auth_data,
> 		const struct sockaddr *from_addr)
> {
>     krb5_error_code ret;
>@@ -1518,6 +1468,9 @@ tgs_build_reply(krb5_context context,
>     krb5_keyblock sessionkey;
>     krb5_kvno kvno;
>     krb5_data rspac;
>+    AuthorizationData *auth_data = NULL;
>+    const EncryptionKey *auth_data_key = replykey;
>+    unsigned auth_data_usage;
> 
>     hdb_entry_ex *krbtgt_out = NULL;
> 
>@@ -1542,6 +1495,12 @@ tgs_build_reply(krb5_context context,
>     s = b->sname;
>     r = b->realm;
> 
>+    if (rk_is_subkey != 0) {
>+	auth_data_usage = KRB5_KU_TGS_REQ_AUTH_DAT_SUBKEY;
>+    } else {
>+	auth_data_usage = KRB5_KU_TGS_REQ_AUTH_DAT_SESSION;
>+    }
>+
>     if (b->kdc_options.canonicalize)
> 	flags |= HDB_F_CANON;
> 
>@@ -2189,6 +2148,47 @@ server_lookup:
> 		"from %s (%s) to %s", tpn, cpn, dpn, spn);
>     }
> 
>+    if (b->enc_authorization_data) {
>+	krb5_data ad;
>+	krb5_crypto crypto;
>+
>+	ret = krb5_crypto_init(context, auth_data_key, 0, &crypto);
>+	if (ret) {
>+	    const char *msg = krb5_get_error_message(context, ret);
>+	    kdc_log(context, config, 0, "krb5_crypto_init failed: %s", msg);
>+	    krb5_free_error_message(context, msg);
>+	    goto out;
>+	}
>+
>+	ret = krb5_decrypt_EncryptedData (context,
>+					  crypto,
>+					  auth_data_usage,
>+					  b->enc_authorization_data,
>+					  &ad);
>+	krb5_crypto_destroy(context, crypto);
>+	if(ret){
>+	    kdc_log(context, config, 0,
>+		    "Failed to decrypt enc-authorization-data");
>+	    ret = KRB5KRB_AP_ERR_BAD_INTEGRITY; /* ? */
>+	    goto out;
>+	}
>+	ALLOC(auth_data);
>+	if (auth_data == NULL) {
>+	    krb5_data_free(&ad);
>+	    ret = KRB5KRB_AP_ERR_BAD_INTEGRITY; /* ? */
>+	    goto out;
>+	}
>+	ret = decode_AuthorizationData(ad.data, ad.length, auth_data, NULL);
>+	krb5_data_free(&ad);
>+	if(ret){
>+	    free(auth_data);
>+	    auth_data = NULL;
>+	    kdc_log(context, config, 0, "Failed to decode authorization data");
>+	    ret = KRB5KRB_AP_ERR_BAD_INTEGRITY; /* ? */
>+	    goto out;
>+	}
>+    }
>+
>     /*
>      * Check flags
>      */
>@@ -2264,7 +2264,7 @@ server_lookup:
> 			 ekey,
> 			 &sessionkey,
> 			 kvno,
>-			 *auth_data,
>+			 auth_data,
> 			 server,
> 			 server->entry.principal,
> 			 spn,
>@@ -2309,6 +2309,11 @@ out:
> 	free(ref_realm);
>     free_METHOD_DATA(&enc_pa_data);
> 
>+    if (auth_data) {
>+       free_AuthorizationData(auth_data);
>+       free(auth_data);
>+    }
>+
>     free_EncTicketPart(&adtkt);
> 
>     return ret;
>@@ -2327,7 +2332,6 @@ _kdc_tgs_rep(krb5_context context,
> 	     struct sockaddr *from_addr,
> 	     int datagram_reply)
> {
>-    AuthorizationData *auth_data = NULL;
>     krb5_error_code ret;
>     int i = 0;
>     const PA_DATA *tgs_req;
>@@ -2366,7 +2370,6 @@ _kdc_tgs_rep(krb5_context context,
> 			    &e_text,
> 			    from, from_addr,
> 			    &csec, &cusec,
>-			    &auth_data,
> 			    &replykey,
> 			    &rk_is_subkey);
>     if (ret == HDB_ERR_NOT_FOUND_HERE) {
>@@ -2391,7 +2394,6 @@ _kdc_tgs_rep(krb5_context context,
> 			  data,
> 			  from,
> 			  &e_text,
>-			  &auth_data,
> 			  from_addr);
>     if (ret) {
> 	kdc_log(context, config, 0,
>@@ -2428,10 +2430,5 @@ out:
>     if(krbtgt)
> 	_kdc_free_ent(context, krbtgt);
> 
>-    if (auth_data) {
>-	free_AuthorizationData(auth_data);
>-	free(auth_data);
>-    }
>-
>     return ret;
> }
>-- 
>1.9.1
>
>
>From 4e4e850dd3ffed9f25f18082f730cd45598ef16b Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 20 Sep 2017 23:05:09 +0200
>Subject: [PATCH 3/3] HEIMDAL:kdc: if we don't have an authenticator subkey for
> S4U2Proxy we need to use the additional tickets key
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13131
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source4/heimdal/kdc/krb5tgs.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c
>index f5b4137..d59eb97 100644
>--- a/source4/heimdal/kdc/krb5tgs.c
>+++ b/source4/heimdal/kdc/krb5tgs.c
>@@ -2144,6 +2144,10 @@ server_lookup:
> 	    goto out;
> 	}
> 
>+	if (rk_is_subkey == 0) {
>+	    auth_data_key = &adtkt.key;
>+	}
>+
> 	kdc_log(context, config, 0, "constrained delegation for %s "
> 		"from %s (%s) to %s", tpn, cpn, dpn, spn);
>     }
>-- 
>1.9.1
>

Actions: View

Attachments on bug 13131: 13863