[PATCH] winbind: Return error for failed PAC signature verification

[Christof Schmitt]

This is a follow-up to the discussion from July last year
(https://lists.samba.org/archive/samba-technical/2012-July/thread.html#85283).

While looking at the winbind interface to decode the PAC again, i
started thinking if we need to return the failed PAC signature
verification back to the caller. A client with a valid kerberos ticket
could generate its own PAC and authenticate to an application using the
winbind PAC interface. If winbind does not return the failed signature
verification, then the application could rely on false data. Based on
this, it seems that it is better to return an error instead of untrusted
data.

Christof
-------------- next part --------------
>From 1bcf0bbb1051940b10217bbeaff36465ef6d1beb Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Thu, 1 May 2014 13:54:19 -0700
Subject: [PATCH] winbind: Return error for failed PAC signature verification

An application passing a PAC to wbcAuthenticateUserEx should have a way
to determine if the PAC is valid. Otherwise the application might rely
on data from an invalid PAC. Fix this by returning an error instead of
the user data if the signature verification fails.

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 source3/winbindd/winbindd_pam.c |   20 +++-----------------
 1 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c
index 415dc79..4058ba9 100644
--- a/source3/winbindd/winbindd_pam.c
+++ b/source3/winbindd/winbindd_pam.c
@@ -2381,28 +2381,14 @@ NTSTATUS winbindd_pam_auth_pac_send(struct winbindd_cli_state *state,
 
 	pac_blob = data_blob_const(req->extra_data.data, req->extra_len);
 	result = extract_pac_vrfy_sigs(state->mem_ctx, pac_blob, &logon_info);
-	if (!NT_STATUS_IS_OK(result) &&
-	    !NT_STATUS_EQUAL(result, NT_STATUS_ACCESS_DENIED)) {
+	if (!NT_STATUS_IS_OK(result)) {
 		DEBUG(1, ("Error during PAC signature verification: %s\n",
 			  nt_errstr(result)));
 		return result;
 	}
 
-	if (logon_info) {
-		/* Signature verification succeeded, trust the PAC */
-		netsamlogon_cache_store(NULL, &logon_info->info3);
-
-	} else {
-		/* Try without signature verification */
-		result = kerberos_pac_logon_info(state->mem_ctx, pac_blob, NULL,
-						 NULL, NULL, NULL, 0,
-						 &logon_info);
-		if (!NT_STATUS_IS_OK(result)) {
-			DEBUG(10, ("Could not extract PAC: %s\n",
-				   nt_errstr(result)));
-			return result;
-		}
-	}
+	/* Signature verification succeeded, trust the PAC */
+	netsamlogon_cache_store(NULL, &logon_info->info3);
 
 	*info3 = &logon_info->info3;
 
-- 
1.7.1