Attachment 5752 Details for Bug 7099 – Patch for fixing the bug

[patch] Patch for fixing the bug

0001-s3-Allow-previous-password-to-be-stored-and-use-it-t.patch (text/plain), 10.26 KB, created by Matthieu Patou on 2010-05-31 13:02:21 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Matthieu Patou

Created: 2010-05-31 13:02:21 UTC

Size: 10.26 KB

patch

obsolete

>From b548112b643aac29db9bc84b4f02f15fd77acde2 Mon Sep 17 00:00:00 2001
>From: Matthieu Patou <mat@matws.net>
>Date: Fri, 21 May 2010 11:57:29 +0400
>Subject: [PATCH] s3: Allow previous password to be stored and use it to check tickets
>
> This patch is to fix bug 7099. It stores the current password in the
> previous password key when the password is changed. It also check the
> user ticket against previous password.
>---
> source3/include/proto.h                  |    1 +
> source3/include/secrets.h                |    1 +
> source3/libads/kerberos_verify.c         |   93 ++++++++++++++++++-----------
> source3/passdb/machine_account_secrets.c |   62 ++++++++++++++++++-
> 4 files changed, 118 insertions(+), 39 deletions(-)
>
>diff --git a/source3/include/proto.h b/source3/include/proto.h
>index f7d9f39..63d6129 100644
>--- a/source3/include/proto.h
>+++ b/source3/include/proto.h
>@@ -4634,6 +4634,7 @@ bool secrets_delete_machine_password(const char *domain);
> bool secrets_delete_machine_password_ex(const char *domain);
> bool secrets_delete_domain_sid(const char *domain);
> bool secrets_store_machine_password(const char *pass, const char *domain, enum netr_SchannelType sec_channel);
>+char *secrets_fetch_prev_machine_password(const char *domain);
> char *secrets_fetch_machine_password(const char *domain,
> 				     time_t *pass_last_set_time,
> 				     enum netr_SchannelType *channel);
>diff --git a/source3/include/secrets.h b/source3/include/secrets.h
>index b51fd22..624b146 100644
>--- a/source3/include/secrets.h
>+++ b/source3/include/secrets.h
>@@ -25,6 +25,7 @@
> */
> #define SECRETS_MACHINE_ACCT_PASS "SECRETS/$MACHINE.ACC"
> #define SECRETS_MACHINE_PASSWORD "SECRETS/MACHINE_PASSWORD"
>+#define SECRETS_MACHINE_PASSWORD_PREV "SECRETS/MACHINE_PASSWORD.PREV"
> #define SECRETS_MACHINE_LAST_CHANGE_TIME "SECRETS/MACHINE_LAST_CHANGE_TIME"
> #define SECRETS_MACHINE_SEC_CHANNEL_TYPE "SECRETS/MACHINE_SEC_CHANNEL_TYPE"
> #define SECRETS_MACHINE_TRUST_ACCOUNT_NAME "SECRETS/SECRETS_MACHINE_TRUST_ACCOUNT_NAME"
>diff --git a/source3/libads/kerberos_verify.c b/source3/libads/kerberos_verify.c
>index 4d7bb8d..0afd84e 100644
>--- a/source3/libads/kerberos_verify.c
>+++ b/source3/libads/kerberos_verify.c
>@@ -307,7 +307,10 @@ static krb5_error_code ads_secrets_verify_ticket(krb5_context context,
> {
> 	krb5_error_code ret = 0;
> 	bool auth_ok = False;
>+	bool cont = True;
> 	char *password_s = NULL;
>+	/* Let's make some room for 2 password (old and new)*/
>+	krb5_data passwords[2];
> 	krb5_data password;
> 	krb5_enctype enctypes[] = { 
> #if defined(ENCTYPE_ARCFOUR_HMAC)
>@@ -318,12 +321,13 @@ static krb5_error_code ads_secrets_verify_ticket(krb5_context context,
> 		ENCTYPE_NULL
> 	};
> 	krb5_data packet;
>-	int i;
>+	int i, j;
> 
> 	*pp_tkt = NULL;
> 	*keyblock = NULL;
> 	*perr = 0;
> 
>+	passwords[1].length = 0;
> 
> 	if (!secrets_init()) {
> 		DEBUG(1,("ads_secrets_verify_ticket: secrets_init failed\n"));
>@@ -338,8 +342,17 @@ static krb5_error_code ads_secrets_verify_ticket(krb5_context context,
> 		return False;
> 	}
> 
>-	password.data = password_s;
>-	password.length = strlen(password_s);
>+
>+	passwords[0].data = password_s;
>+	passwords[0].length = strlen(password_s);
>+
>+	password_s = secrets_fetch_prev_machine_password(lp_workgroup());
>+	if (password_s) {
>+		DEBUG(10,("ads_secrets_verify_ticket: found previous password\n"));
>+		password.data = password_s;
>+		password.length = strlen(password_s);
>+		passwords[1] = password;
>+	}
> 
> 	/* CIFS doesn't use addresses in tickets. This would break NAT. JRA */
> 
>@@ -347,46 +360,56 @@ static krb5_error_code ads_secrets_verify_ticket(krb5_context context,
> 	packet.data = (char *)ticket->data;
> 
> 	/* We need to setup a auth context with each possible encoding type in turn. */
>-	for (i=0;enctypes[i];i++) {
>-		krb5_keyblock *key = NULL;
>+	for (j=0; j<2 && passwords[j].length; j++) {
> 
>-		if (!(key = SMB_MALLOC_P(krb5_keyblock))) {
>-			ret = ENOMEM;
>-			goto out;
>-		}
>-	
>-		if (create_kerberos_key_from_string(context, host_princ, &password, key, enctypes[i], false)) {
>-			SAFE_FREE(key);
>-			continue;
>-		}
>+		for (i=0;enctypes[i];i++) {
>+			krb5_keyblock *key = NULL;
> 
>-		krb5_auth_con_setuseruserkey(context, auth_context, key);
>+			if (!(key = SMB_MALLOC_P(krb5_keyblock))) {
>+				ret = ENOMEM;
>+				goto out;
>+			}
> 
>-		if (!(ret = krb5_rd_req(context, &auth_context, &packet, 
>-					NULL,
>-					NULL, NULL, pp_tkt))) {
>-			DEBUG(10,("ads_secrets_verify_ticket: enc type [%u] decrypted message !\n",
>-				(unsigned int)enctypes[i] ));
>-			auth_ok = True;
>-			krb5_copy_keyblock(context, key, keyblock);
>-			krb5_free_keyblock(context, key);
>-			break;
>-		}
>+			if (create_kerberos_key_from_string(context, host_princ, &passwords[j], key, enctypes[i], false)) {
>+				SAFE_FREE(key);
>+				continue;
>+			}
>+
>+			krb5_auth_con_setuseruserkey(context, auth_context, key);
>+
>+			if (!(ret = krb5_rd_req(context, &auth_context, &packet,
>+						NULL,
>+						NULL, NULL, pp_tkt))) {
>+				DEBUG(10,("ads_secrets_verify_ticket: enc type [%u] decrypted message !\n",
>+					(unsigned int)enctypes[i] ));
>+				auth_ok = True;
>+				cont = False;
>+				krb5_copy_keyblock(context, key, keyblock);
>+				krb5_free_keyblock(context, key);
>+				break;
>+			}
> 
>-		DEBUG((ret != KRB5_BAD_ENCTYPE) ? 3 : 10,
>-				("ads_secrets_verify_ticket: enc type [%u] failed to decrypt with error %s\n",
>-				(unsigned int)enctypes[i], error_message(ret)));
>+			DEBUG((ret != KRB5_BAD_ENCTYPE) ? 3 : 10,
>+					("ads_secrets_verify_ticket: enc type [%u] failed to decrypt with error %s\n",
>+					(unsigned int)enctypes[i], error_message(ret)));
>+
>+			/* successfully decrypted but ticket is just not valid at the moment */
>+			if (ret == KRB5KRB_AP_ERR_TKT_NYV ||
>+			    ret == KRB5KRB_AP_ERR_TKT_EXPIRED ||
>+			    ret == KRB5KRB_AP_ERR_SKEW) {
>+				krb5_free_keyblock(context, key);
>+				cont = False;
>+				break;
>+			}
> 
>-		/* successfully decrypted but ticket is just not valid at the moment */
>-		if (ret == KRB5KRB_AP_ERR_TKT_NYV || 
>-		    ret == KRB5KRB_AP_ERR_TKT_EXPIRED ||
>-		    ret == KRB5KRB_AP_ERR_SKEW) {
> 			krb5_free_keyblock(context, key);
>+		}
>+		if (!cont) {
>+			/* If we found a valid pass then no need to try
>+			 * the next one or we have invalid ticket so no need
>+			 * to try next password*/
> 			break;
> 		}
>-
>-		krb5_free_keyblock(context, key);
>-
> 	}
> 
>  out:
>diff --git a/source3/passdb/machine_account_secrets.c b/source3/passdb/machine_account_secrets.c
>index 4a1c3fa..1ee764e 100644
>--- a/source3/passdb/machine_account_secrets.c
>+++ b/source3/passdb/machine_account_secrets.c
>@@ -161,6 +161,23 @@ static const char *machine_last_change_time_keystr(const char *domain)
> 
> 
> /**
>+ * Form a key for fetching the machine previous trust account password
>+ *
>+ * @param domain domain name
>+ *
>+ * @return keystring
>+ **/
>+static const char *machine_prev_password_keystr(const char *domain)
>+{
>+	char *keystr;
>+
>+	keystr = talloc_asprintf_strupper_m(talloc_tos(), "%s/%s",
>+					    SECRETS_MACHINE_PASSWORD_PREV, domain);
>+	SMB_ASSERT(keystr != NULL);
>+	return keystr;
>+}
>+
>+/**
>  * Form a key for fetching the machine trust account password
>  *
>  * @param domain domain name
>@@ -300,21 +317,38 @@ bool secrets_fetch_trust_account_password(const char *domain, uint8 ret_pwd[16],
> }
> 
> /************************************************************************
>- Routine to delete the plaintext machine account password
>+ Routine to delete the plaintext machine account password and old
>+ password if any
> ************************************************************************/
> 
> bool secrets_delete_machine_password(const char *domain)
> {
>+	const char *ret = (char *)secrets_fetch(machine_prev_password_keystr(domain), NULL);
>+	if (ret != NULL) {
>+		if (!secrets_delete(machine_prev_password_keystr(domain))) {
>+			SAFE_FREE(ret);
>+			return false;
>+		}
>+	}
>+	SAFE_FREE(ret);
> 	return secrets_delete(machine_password_keystr(domain));
> }
> 
> /************************************************************************
>- Routine to delete the plaintext machine account password, sec channel type and
>- last change time from secrets database
>+ Routine to delete the plaintext machine account password, old password,
>+ sec channel type and last change time from secrets database
> ************************************************************************/
> 
> bool secrets_delete_machine_password_ex(const char *domain)
> {
>+	const char *ret = (char *)secrets_fetch(machine_prev_password_keystr(domain), NULL);
>+	if (ret != NULL) {
>+		if (!secrets_delete(machine_prev_password_keystr(domain))) {
>+			SAFE_FREE(ret);
>+			return false;
>+		}
>+	}
>+	SAFE_FREE(ret);
> 	if (!secrets_delete(machine_password_keystr(domain))) {
> 		return false;
> 	}
>@@ -335,7 +369,8 @@ bool secrets_delete_domain_sid(const char *domain)
> 
> /************************************************************************
>  Routine to set the plaintext machine account password for a realm
>-the password is assumed to be a null terminated ascii string
>+ the password is assumed to be a null terminated ascii string.
>+ Before storing
> ************************************************************************/
> 
> bool secrets_store_machine_password(const char *pass, const char *domain,
>@@ -344,7 +379,14 @@ bool secrets_store_machine_password(const char *pass, const char *domain,
> 	bool ret;
> 	uint32 last_change_time;
> 	uint32 sec_channel_type;
>+	const char *oldpass;
> 
>+	oldpass = (char *)secrets_fetch(machine_password_keystr(domain), NULL);
>+	if (oldpass!=NULL && strlen(oldpass) > 0) {
>+		if (!secrets_store(machine_prev_password_keystr(domain), oldpass, strlen(oldpass)+1))
>+			return ret;
>+		SAFE_FREE(oldpass);
>+	}
> 	ret = secrets_store(machine_password_keystr(domain), pass, strlen(pass)+1);
> 	if (!ret)
> 		return ret;
>@@ -358,6 +400,18 @@ bool secrets_store_machine_password(const char *pass, const char *domain,
> 	return ret;
> }
> 
>+
>+/************************************************************************
>+ Routine to fetch the previous plaintext machine account password for a realm
>+ the password is assumed to be a null terminated ascii string.
>+************************************************************************/
>+
>+char *secrets_fetch_prev_machine_password(const char *domain)
>+{
>+	char *ret;
>+	ret = (char *)secrets_fetch(machine_prev_password_keystr(domain), NULL);
>+}
>+
> /************************************************************************
>  Routine to fetch the plaintext machine account password for a realm
>  the password is assumed to be a null terminated ascii string.
>-- 
>1.7.0.4
>

Actions: View

Attachments on bug 7099: 5273 | 5728 | 5735 | 5752 | 5758