r12682: This patch finally fixes our kpasswdd implementation to be compatible

with clients compiled against the MIT Kerberos implementation.  (Which
checks for address in KRB-PRIV packets, hence my comments on socket
functions earlier today).

It also fixes the 'set password' operation to behave correctly (it was
previously a no-op).

This allows Samba3 to join Samba4.  Some winbindd operations even work,
which I think is a good step forward.  There is naturally a lot of work
to do, but I wanted at least the very basics of Samba3 domain membership
to be available for the tech preview.

Andrew Bartlett

source/auth/gensec/gensec.c

@@ -377,6 +377,8 @@ static NTSTATUS gensec_start(TALLOC_CTX *mem_ctx,
 	(*gensec_security)->ops = NULL;
 
 	ZERO_STRUCT((*gensec_security)->target);
+	ZERO_STRUCT((*gensec_security)->peer_addr);
+	ZERO_STRUCT((*gensec_security)->my_addr);
 
 	(*gensec_security)->subcontext = False;
 	(*gensec_security)->want_features = 0;
@@ -789,7 +791,7 @@ BOOL gensec_have_feature(struct gensec_security *gensec_security,
 }
 
 /** 
- * Associate a credentails structure with a GENSEC context - talloc_reference()s it to the context 
+ * Associate a credentials structure with a GENSEC context - talloc_reference()s it to the context 
  *
  */
 
@@ -800,7 +802,7 @@ NTSTATUS gensec_set_credentials(struct gensec_security *gensec_security, struct
 }
 
 /** 
- * Return the credentails structure associated with a GENSEC context
+ * Return the credentials structure associated with a GENSEC context
  *
  */
 
@@ -855,10 +857,71 @@ const char *gensec_get_target_hostname(struct gensec_security *gensec_security)
 		return gensec_security->target.hostname;
 	}
 
-	/* TODO: Add a 'set sockaddr' call, and do a reverse lookup */
+	/* We could add use the 'set sockaddr' call, and do a reverse
+	 * lookup, but this would be both insecure (compromising the
+	 * way kerberos works) and add DNS timeouts */
 	return NULL;
 }
 
+/** 
+ * Set local and peer socket addresses onto a socket context on the GENSEC context 
+ *
+ * This is so that kerberos can include these addresses in
+ * cryptographic tokens, to avoid certain attacks.
+ */
+
+NTSTATUS gensec_set_my_addr(struct gensec_security *gensec_security, const char *my_addr, int port) 
+{
+	gensec_security->my_addr.addr = talloc_strdup(gensec_security, my_addr);
+	if (my_addr && !gensec_security->my_addr.addr) {
+		return NT_STATUS_NO_MEMORY;
+	}
+	gensec_security->my_addr.port = port;
+	return NT_STATUS_OK;
+}
+
+NTSTATUS gensec_set_peer_addr(struct gensec_security *gensec_security, const char *peer_addr, int port) 
+{
+	gensec_security->peer_addr.addr = talloc_strdup(gensec_security, peer_addr);
+	if (peer_addr && !gensec_security->peer_addr.addr) {
+		return NT_STATUS_NO_MEMORY;
+	}
+	gensec_security->peer_addr.port = port;
+	return NT_STATUS_OK;
+}
+
+const char *gensec_get_my_addr(struct gensec_security *gensec_security, int *port) 
+{
+	if (gensec_security->my_addr.addr) {
+		if (port) {
+			*port = gensec_security->my_addr.port;
+		}
+		return gensec_security->my_addr.addr;
+	}
+
+	/* We could add a 'set sockaddr' call, and do a lookup.  This
+	 * would avoid needing to do system calls if nothing asks. */
+	return NULL;
+}
+
+const char *gensec_get_peer_addr(struct gensec_security *gensec_security, int *port) 
+{
+	if (gensec_security->peer_addr.addr) {
+		if (port) {
+			*port = gensec_security->peer_addr.port;
+		}
+		return gensec_security->peer_addr.addr;
+	}
+
+	/* We could add a 'set sockaddr' call, and do a lookup.  This
+	 * would avoid needing to do system calls if nothing asks.
+	 * However, this is not appropriate for the peer addres on
+	 * datagram sockets */
+	return NULL;
+}
+
+
+
 /** 
  * Set the target principal (assuming it it known, say from the SPNEGO reply)
  *  - ensures it is talloc()ed 

source/auth/gensec/gensec.h

@@ -31,10 +31,14 @@ struct gensec_security;
 struct gensec_target {
 	const char *principal;
 	const char *hostname;
-	const struct sock_addr *addr;
 	const char *service;
 };
 
+struct gensec_addr {
+	const char *addr;
+	int port;
+};
+
 #define GENSEC_FEATURE_SESSION_KEY	0x00000001
 #define GENSEC_FEATURE_SIGN		0x00000002
 #define GENSEC_FEATURE_SEAL		0x00000004
@@ -114,6 +118,7 @@ struct gensec_security {
 	BOOL subcontext;
 	uint32_t want_features;
 	struct event_context *event_ctx;
+	struct gensec_addr my_addr, peer_addr;
 };
 
 /* this structure is used by backends to determine the size of some critical types */

source/auth/gensec/gensec_krb5.c

@@ -29,6 +29,8 @@
 #include "auth/kerberos/kerberos.h"
 #include "librpc/gen_ndr/ndr_krb5pac.h"
 #include "auth/auth.h"
+#include "system/network.h"
+#include "lib/socket/socket.h"
 
 enum GENSEC_KRB5_STATE {
 	GENSEC_KRB5_SERVER_START,
@@ -85,7 +87,10 @@ static NTSTATUS gensec_krb5_start(struct gensec_security *gensec_security)
 	krb5_error_code ret;
 	struct gensec_krb5_state *gensec_krb5_state;
 	struct cli_credentials *creds;
-
+	const char *my_addr, *peer_addr;
+	int my_port, peer_port;
+	krb5_address my_krb5_addr, peer_krb5_addr;
+	
 	creds = gensec_get_credentials(gensec_security);
 	if (!creds) {
 		return NT_STATUS_INVALID_PARAMETER;
@@ -133,6 +138,70 @@ static NTSTATUS gensec_krb5_start(struct gensec_security *gensec_security)
 		return NT_STATUS_INTERNAL_ERROR;
 	}
 
+	my_addr = gensec_get_my_addr(gensec_security, &my_port);
+	if (my_addr) {
+		struct sockaddr_in sock_addr;
+		struct ipv4_addr addr;
+
+		/* TODO:  This really should be in a utility function somewhere */
+		ZERO_STRUCT(sock_addr);
+#ifdef HAVE_SOCK_SIN_LEN
+		sock_addr.sin_len		= sizeof(sock_addr);
+#endif
+		addr				= interpret_addr2(my_addr);
+		sock_addr.sin_addr.s_addr	= addr.addr;
+		sock_addr.sin_port		= htons(my_port);
+		sock_addr.sin_family	= PF_INET;
+		
+		ret = krb5_sockaddr2address(gensec_krb5_state->smb_krb5_context->krb5_context, 
+					    (struct sockaddr *)&sock_addr, &my_krb5_addr);
+		if (ret) {
+			DEBUG(1,("gensec_krb5_start: krb5_sockaddr2address (local) failed (%s)\n", 
+				 smb_get_krb5_error_message(gensec_krb5_state->smb_krb5_context->krb5_context, 
+							    ret, gensec_krb5_state)));
+			talloc_free(gensec_krb5_state);
+			return NT_STATUS_INTERNAL_ERROR;
+		}
+	}
+
+	peer_addr = gensec_get_my_addr(gensec_security, &peer_port);
+	if (peer_addr) {
+		struct sockaddr_in sock_addr;
+		struct ipv4_addr addr;
+
+		/* TODO:  This really should be in a utility function somewhere */
+		ZERO_STRUCT(sock_addr);
+#ifdef HAVE_SOCK_SIN_LEN
+		sock_addr.sin_len		= sizeof(sock_addr);
+#endif
+		addr				= interpret_addr2(peer_addr);
+		sock_addr.sin_addr.s_addr	= addr.addr;
+		sock_addr.sin_port		= htons(peer_port);
+		sock_addr.sin_family	= PF_INET;
+		
+		ret = krb5_sockaddr2address(gensec_krb5_state->smb_krb5_context->krb5_context, 
+					    (struct sockaddr *)&sock_addr, &peer_krb5_addr);
+		if (ret) {
+			DEBUG(1,("gensec_krb5_start: krb5_sockaddr2address (local) failed (%s)\n", 
+				 smb_get_krb5_error_message(gensec_krb5_state->smb_krb5_context->krb5_context, 
+							    ret, gensec_krb5_state)));
+			talloc_free(gensec_krb5_state);
+			return NT_STATUS_INTERNAL_ERROR;
+		}
+	}
+
+	ret = krb5_auth_con_setaddrs(gensec_krb5_state->smb_krb5_context->krb5_context, 
+				     gensec_krb5_state->auth_context,
+				     my_addr ? &my_krb5_addr : NULL, 
+				     peer_addr ? &peer_krb5_addr : NULL);
+	if (ret) {
+		DEBUG(1,("gensec_krb5_start: krb5_auth_con_setaddrs failed (%s)\n", 
+			 smb_get_krb5_error_message(gensec_krb5_state->smb_krb5_context->krb5_context, 
+						    ret, gensec_krb5_state)));
+		talloc_free(gensec_krb5_state);
+		return NT_STATUS_INTERNAL_ERROR;
+	}
+
 	return NT_STATUS_OK;
 }
 
@@ -425,11 +494,12 @@ static NTSTATUS gensec_krb5_update(struct gensec_security *gensec_security,
 		}
 		return nt_status;
 	}
+
 	case GENSEC_KRB5_DONE:
-		return NT_STATUS_OK;
+	default:
+		/* Asking too many times... */
+		return NT_STATUS_INVALID_PARAMETER;
 	}
-	
-	return NT_STATUS_INVALID_PARAMETER;
 }
 
 static NTSTATUS gensec_krb5_session_key(struct gensec_security *gensec_security, 

source/kdc/kdc.c

@@ -45,8 +45,8 @@ typedef BOOL (*kdc_process_fn_t)(struct kdc_server *kdc,
 				 TALLOC_CTX *mem_ctx, 
 				 DATA_BLOB *input, 
 				 DATA_BLOB *reply,
-				 const char *src_addr,
-				 int src_port);
+				 const char *peer_address, int peer_port,
+				 const char *my_address, int my_port);
 
 /* hold information about one kdc socket */
 struct kdc_socket {
@@ -116,6 +116,8 @@ static void kdc_recv_handler(struct kdc_socket *kdc_socket)
 	size_t nread, dsize;
 	const char *src_addr;
 	int src_port;
+	const char *my_addr;
+	int my_port;
 	int ret;
 
 	status = socket_pending(kdc_socket->sock, &dsize);
@@ -140,15 +142,24 @@ static void kdc_recv_handler(struct kdc_socket *kdc_socket)
 	talloc_steal(tmp_ctx, src_addr);
 	blob.length = nread;
 	
-	DEBUG(2,("Received krb5 UDP packet of length %lu from %s:%u\n", 
+	DEBUG(10,("Received krb5 UDP packet of length %lu from %s:%u\n", 
 		 (long)blob.length, src_addr, (uint16_t)src_port));
 	
+	my_addr = socket_get_my_addr(kdc_socket->sock, tmp_ctx);
+	if (!my_addr) {
+		talloc_free(tmp_ctx);
+		return;
+	}
+	my_port = socket_get_my_port(kdc_socket->sock);
+
+
 	/* Call krb5 */
 	ret = kdc_socket->process(kdc_socket->kdc, 
 				  tmp_ctx, 
 				  &blob,  
 				  &reply,
-				  src_addr, src_port);
+				  src_addr, src_port,
+				  my_addr, my_port);
 	if (!ret) {
 		talloc_free(tmp_ctx);
 		return;
@@ -205,19 +216,28 @@ static NTSTATUS kdc_tcp_recv(void *private, DATA_BLOB blob)
 							     struct kdc_tcp_connection);
 	NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
 	TALLOC_CTX *tmp_ctx = talloc_new(kdcconn);
-	const char *src_addr;
-	int src_port;
 	int ret;
 	DATA_BLOB input, reply;
+	const char *src_addr;
+	int src_port;
+	const char *my_addr;
+	int my_port;
 
 	talloc_steal(tmp_ctx, blob.data);
 
 	src_addr = socket_get_peer_addr(kdcconn->conn->socket, tmp_ctx);
-	if (!src_addr) goto nomem;
-	src_port = socket_get_peer_port(kdcconn->conn->socket);
+	if (!src_addr) {
+		talloc_free(tmp_ctx);
+		return NT_STATUS_NO_MEMORY;
+	}
+	src_port = socket_get_my_port(kdcconn->conn->socket);
 
-	DEBUG(2,("Received krb5 TCP packet of length %lu from %s:%u\n", 
-		 (long)blob.length - 4, src_addr, src_port));
+	my_addr = socket_get_my_addr(kdcconn->conn->socket, tmp_ctx);
+	if (!my_addr) {
+		talloc_free(tmp_ctx);
+		return NT_STATUS_NO_MEMORY;
+	}
+	my_port = socket_get_my_port(kdcconn->conn->socket);
 
 	/* Call krb5 */
 	input = data_blob_const(blob.data + 4, blob.length - 4); 
@@ -226,16 +246,18 @@ static NTSTATUS kdc_tcp_recv(void *private, DATA_BLOB blob)
 			       tmp_ctx,
 			       &input,
 			       &reply,
-			       src_addr, src_port);
+			       src_addr, src_port,
+			       my_addr, my_port);
 	if (!ret) {
-		status = NT_STATUS_INTERNAL_ERROR;
-		goto failed;
+		talloc_free(tmp_ctx);
+		return NT_STATUS_INTERNAL_ERROR;
 	}
 
 	/* and now encode the reply */
 	blob = data_blob_talloc(kdcconn, NULL, reply.length + 4);
 	if (!blob.data) {
-		goto nomem;
+		talloc_free(tmp_ctx);
+		return NT_STATUS_NO_MEMORY;
 	}
 
 	RSIVAL(blob.data, 0, reply.length);
@@ -243,17 +265,13 @@ static NTSTATUS kdc_tcp_recv(void *private, DATA_BLOB blob)
 
 	status = packet_send(kdcconn->packet, blob);
 	if (!NT_STATUS_IS_OK(status)) {
-		goto failed;
+		talloc_free(tmp_ctx);
+		return status;
 	}
 
 	/* the call isn't needed any more */
 	talloc_free(tmp_ctx);
 	return NT_STATUS_OK;
-nomem:
-	status = NT_STATUS_NO_MEMORY;
-
-failed:
-	return status;
 }
 
 /*
@@ -294,31 +312,35 @@ static BOOL kdc_process(struct kdc_server *kdc,
 			TALLOC_CTX *mem_ctx, 
 			DATA_BLOB *input, 
 			DATA_BLOB *reply,
-			const char *src_addr,
-			int src_port)
+			const char *peer_addr,
+			int peer_port,
+			const char *my_addr,
+			int my_port)
 {
 	int ret;	
 	krb5_data k5_reply;
 	struct ipv4_addr addr;
-	struct sockaddr_in src_sock_addr;
+	struct sockaddr_in peer_sock_addr;
 
 	/* TODO:  This really should be in a utility function somewhere */
-	ZERO_STRUCT(src_sock_addr);
+	ZERO_STRUCT(peer_sock_addr);
 #ifdef HAVE_SOCK_SIN_LEN
-	src_sock_addr.sin_len		= sizeof(src_sock_addr);
+	peer_sock_addr.sin_len		= sizeof(peer_sock_addr);
 #endif
-	addr				= interpret_addr2(src_addr);
-	src_sock_addr.sin_addr.s_addr	= addr.addr;
-	src_sock_addr.sin_port		= htons(src_port);
-	src_sock_addr.sin_family	= PF_INET;
+	addr				= interpret_addr2(peer_addr);
+	peer_sock_addr.sin_addr.s_addr	= addr.addr;
+	peer_sock_addr.sin_port		= htons(peer_port);
+	peer_sock_addr.sin_family	= PF_INET;
+
+	DEBUG(10,("Received KDC packet of length %lu from %s\n", 
+		 (long)input->length - 4, peer_addr));
 
-	
 	ret = krb5_kdc_process_krb5_request(kdc->smb_krb5_context->krb5_context, 
 					    kdc->config,
 					    input->data, input->length,
 					    &k5_reply,
-					    src_addr,
-					    (struct sockaddr *)&src_sock_addr);
+					    peer_addr,
+					    (struct sockaddr *)&peer_sock_addr);
 	if (ret == -1) {
 		*reply = data_blob(NULL, 0);
 		return False;

source/kdc/kdc.h

@@ -33,8 +33,10 @@ BOOL kpasswdd_process(struct kdc_server *kdc,
 		      TALLOC_CTX *mem_ctx, 
 		      DATA_BLOB *input, 
 		      DATA_BLOB *reply,
-		      const char *from,
-		      int src_port);
+		      const char *peer_addr,
+		      int peer_port,
+		      const char *my_addr,
+		      int my_port);
 
 /*
   top level context structure for the kdc server

source/kdc/kpasswdd.c

@@ -178,7 +178,10 @@ static BOOL kpasswdd_change_password(struct kdc_server *kdc,
 						reply);
 	}
 	
-	DEBUG(3, ("Changing password of %s\n", dom_sid_string(mem_ctx, session_info->security_token->user_sid)));
+	DEBUG(3, ("Changing password of %s\\%s (%s)\n", 
+		  session_info->server_info->domain_name,
+		  session_info->server_info->account_name,
+		  dom_sid_string(mem_ctx, session_info->security_token->user_sid)));
 
 	/* User password change */
 	status = samdb_set_password_sid(samdb, mem_ctx, 
@@ -203,14 +206,7 @@ static BOOL kpasswd_process_request(struct kdc_server *kdc,
 				    DATA_BLOB *input, 
 				    DATA_BLOB *reply)
 {
-	NTSTATUS status;
-	enum samr_RejectReason reject_reason;
-	struct samr_DomInfo1 *dominfo;
-	struct ldb_context *samdb;
 	struct auth_session_info *session_info;
-	struct ldb_message *msg = ldb_msg_new(gensec_security);
-	krb5_context context = kdc->smb_krb5_context->krb5_context;
-	int ret;
 	if (!msg) {
 		return False;
 	}
@@ -236,14 +232,22 @@ static BOOL kpasswd_process_request(struct kdc_server *kdc,
 	}
 	case KRB5_KPASSWD_VERS_SETPW:
 	{
-		size_t len;
+		NTSTATUS status;
+		enum samr_RejectReason reject_reason;
+		struct samr_DomInfo1 *dominfo;
+		struct ldb_context *samdb;
+		struct ldb_message *msg = ldb_msg_new(mem_ctx);
+		krb5_context context = kdc->smb_krb5_context->krb5_context;
+
 		ChangePasswdDataMS chpw;
 		char *password;
+
 		krb5_principal principal;
 		char *set_password_on_princ;
 		struct ldb_dn *set_password_on_dn;
 
-		samdb = samdb_connect(gensec_security, session_info);
+		size_t len;
+		int ret;
 
 		ret = decode_ChangePasswdDataMS(input->data, input->length,
 						&chpw, &len);
@@ -294,11 +298,22 @@ static BOOL kpasswd_process_request(struct kdc_server *kdc,
 		
 		krb5_free_principal(context, principal);
 		
-		status = crack_user_principal_name(samdb, mem_ctx, 
-						   set_password_on_princ, 
-						   &set_password_on_dn, NULL);
-		free(set_password_on_princ);
-		if (!NT_STATUS_IS_OK(status)) {
+		samdb = samdb_connect(mem_ctx, session_info);
+		if (!samdb) {
+			return kpasswdd_make_error_reply(kdc, mem_ctx, 
+							 KRB5_KPASSWD_HARDERROR,
+							 "Unable to open database!",
+							 reply);
+		}
+
+		DEBUG(3, ("%s\\%s (%s) is changing password of %s\n", 
+			  session_info->server_info->domain_name,
+			  session_info->server_info->account_name,
+			  dom_sid_string(mem_ctx, session_info->security_token->user_sid), 
+			  set_password_on_princ));
+		ret = ldb_transaction_start(samdb);
+		if (ret) {
+			status = NT_STATUS_TRANSACTION_ABORTED;
 			return kpasswd_make_pwchange_reply(kdc, mem_ctx, 
 							   status,
 							   reject_reason, 
@@ -306,14 +321,61 @@ static BOOL kpasswd_process_request(struct kdc_server *kdc,
 							   reply);
 		}
 
-		/* Admin password set */
-		status = samdb_set_password(samdb, mem_ctx,
-					    set_password_on_dn, NULL,
-					    msg, password, NULL, NULL, 
-					    False, /* this is not a user password change */
-					    True, /* run restriction tests */
-					    &reject_reason, &dominfo);
+		status = crack_user_principal_name(samdb, mem_ctx, 
+						   set_password_on_princ, 
+						   &set_password_on_dn, NULL);
+		free(set_password_on_princ);
+		if (!NT_STATUS_IS_OK(status)) {
+			ldb_transaction_cancel(samdb);
+			return kpasswd_make_pwchange_reply(kdc, mem_ctx, 
+							   status,
+							   reject_reason, 
+							   dominfo, 
+							   reply);
+		}
 
+		msg = ldb_msg_new(mem_ctx);
+		if (msg == NULL) {
+			ldb_transaction_cancel(samdb);
+			status = NT_STATUS_NO_MEMORY;
+		} else {
+			msg->dn = ldb_dn_copy(msg, set_password_on_dn);
+			if (!msg->dn) {
+				status = NT_STATUS_NO_MEMORY;
+			}
+		}
+
+		if (NT_STATUS_IS_OK(status)) {
+			/* Admin password set */
+			status = samdb_set_password(samdb, mem_ctx,
+						    set_password_on_dn, NULL,
+						    msg, password, NULL, NULL, 
+						    False, /* this is not a user password change */
+						    True, /* run restriction tests */
+						    &reject_reason, &dominfo);
+		}
+
+		if (NT_STATUS_IS_OK(status)) {
+			/* modify the samdb record */
+			ret = samdb_replace(samdb, mem_ctx, msg);
+			if (ret != 0) {
+				DEBUG(2,("Failed to modify record to set password on %s: %s\n",
+					 ldb_dn_linearize(mem_ctx, msg->dn),
+					 ldb_errstring(samdb)));
+				status = NT_STATUS_ACCESS_DENIED;
+			}
+		}
+		if (NT_STATUS_IS_OK(status)) {
+			ret = ldb_transaction_commit(samdb);
+			if (ret != 0) {
+				DEBUG(1,("Failed to commit transaction to set password on %s: %s\n",
+					 ldb_dn_linearize(mem_ctx, msg->dn),
+					 ldb_errstring(samdb)));
+				status = NT_STATUS_TRANSACTION_ABORTED;
+			}
+		} else {
+			ldb_transaction_cancel(samdb);
+		}
 		return kpasswd_make_pwchange_reply(kdc, mem_ctx, 
 						   status,
 						   reject_reason, 
@@ -322,11 +384,11 @@ static BOOL kpasswd_process_request(struct kdc_server *kdc,
 	}
 	default:
 		return kpasswdd_make_error_reply(kdc, mem_ctx, 
-						KRB5_KPASSWD_BAD_VERSION,
-						talloc_asprintf(mem_ctx, 
-								"Protocol version %u not supported", 
-								version),
-						reply);
+						 KRB5_KPASSWD_BAD_VERSION,
+						 talloc_asprintf(mem_ctx, 
+								 "Protocol version %u not supported", 
+								 version),
+						 reply);
 	}
 	return True;
 }
@@ -335,8 +397,10 @@ BOOL kpasswdd_process(struct kdc_server *kdc,
 		      TALLOC_CTX *mem_ctx, 
 		      DATA_BLOB *input, 
 		      DATA_BLOB *reply,
-		      const char *from,
-		      int src_port)
+		      const char *peer_addr,
+		      int peer_port,
+		      const char *my_addr,
+		      int my_port)
 {
 	BOOL ret;
 	const uint16_t header_len = 6;
@@ -355,6 +419,8 @@ BOOL kpasswdd_process(struct kdc_server *kdc,
 		return False;
 	}
 
+	/* Be parinoid.  We need to ensure we don't just let the
+	 * caller lead us into a buffer overflow */
 	if (input->length <= header_len) {
 		talloc_free(tmp_ctx);
 		return False;
@@ -366,6 +432,9 @@ BOOL kpasswdd_process(struct kdc_server *kdc,
 		return False;
 	}
 
+	/* There are two different versions of this protocol so far,
+	 * plus others in the standards pipe.  Fortunetly they all
+	 * take a very similar framing */
 	version = RSVAL(input->data, 2);
 	ap_req_len = RSVAL(input->data, 4);
 	if ((ap_req_len >= len) || (ap_req_len + header_len) >= len) {
@@ -407,7 +476,26 @@ BOOL kpasswdd_process(struct kdc_server *kdc,
 		return ret;
 	}
 	
-	gensec_set_credentials(gensec_security, server_credentials);
+	nt_status = gensec_set_credentials(gensec_security, server_credentials);
+	if (!NT_STATUS_IS_OK(nt_status)) {
+		talloc_free(tmp_ctx);
+		return False;
+	}
+
+	/* The kerberos PRIV packets include these addresses.  MIT
+	 * clients check that they are present */
+	nt_status = gensec_set_peer_addr(gensec_security, peer_addr, peer_port);
+	if (!NT_STATUS_IS_OK(nt_status)) {
+		talloc_free(tmp_ctx);
+		return False;
+	}
+	nt_status = gensec_set_my_addr(gensec_security, my_addr, my_port);
+	if (!NT_STATUS_IS_OK(nt_status)) {
+		talloc_free(tmp_ctx);
+		return False;
+	}
+
+	/* We want the GENSEC wrap calls to generate PRIV tokens */
 	gensec_want_feature(gensec_security, GENSEC_FEATURE_SEAL);
 
 	nt_status = gensec_start_mech_by_name(gensec_security, "krb5");
@@ -416,6 +504,7 @@ BOOL kpasswdd_process(struct kdc_server *kdc,
 		return False;
 	}
 
+	/* Accept the AP-REQ and generate teh AP-REP we need for the reply */
 	nt_status = gensec_update(gensec_security, tmp_ctx, ap_req, &ap_rep);
 	if (!NT_STATUS_IS_OK(nt_status) && !NT_STATUS_EQUAL(nt_status, NT_STATUS_MORE_PROCESSING_REQUIRED)) {
 		
@@ -433,6 +522,7 @@ BOOL kpasswdd_process(struct kdc_server *kdc,
 		return ret;
 	}
 
+	/* Extract the data from the KRB-PRIV half of the message */
 	nt_status = gensec_unwrap(gensec_security, tmp_ctx, &krb_priv_req, &kpasswd_req);
 	if (!NT_STATUS_IS_OK(nt_status)) {
 		ret = kpasswdd_make_unauth_error_reply(kdc, mem_ctx, 
@@ -449,6 +539,7 @@ BOOL kpasswdd_process(struct kdc_server *kdc,
 		return ret;
 	}
 
+	/* Figure out something to do with it (probably changing a password...) */
 	ret = kpasswd_process_request(kdc, tmp_ctx, 
 				      gensec_security, 
 				      version, 
@@ -457,7 +548,9 @@ BOOL kpasswdd_process(struct kdc_server *kdc,
 		/* Argh! */
 		return False;
 	}
-	
+
+	/* And wrap up the reply: This ensures that the error message
+	 * or success can be verified by the client */
 	nt_status = gensec_wrap(gensec_security, tmp_ctx, 
 				&kpasswd_rep, &krb_priv_rep);
 	if (!NT_STATUS_IS_OK(nt_status)) {