[PATCH] Improve debugging in gensec_gssapi

[Jeremy Allison]

On Wed, Sep 02, 2015 at 05:01:40PM +1200, Andrew Bartlett wrote:
> This should give a little less confusion when debugging gensec_gssapi
> in the future. 
> 
> Please review/push

Andrew, just one question here.

In the KRB5KRB_AP_ERR_TKT_NYV case, this changes
the return from NT_STATUS_TIME_DIFFERENCE_AT_DC to
NT_STATUS_INVALID_PARAMETER in the case GENSEC_SERVER
case.

That's a behavior change - is it intentional ?

Jeremy.

> Andrew Bartlett
> -- 
> Andrew Bartlett
> https://samba.org/~abartlet/
> Authentication Developer, Samba Team         https://samba.org
> Samba Development and Support, Catalyst IT   
> https://catalyst.net.nz/services/samba
> 
> 
> 
> 

> From 7e5f89cb0eb641d6a12fb9dc4eca45f4f267c284 Mon Sep 17 00:00:00 2001
> From: Andrew Bartlett <abartlet at samba.org>
> Date: Wed, 2 Sep 2015 15:02:01 +1200
> Subject: [PATCH] gensec_gssapi: Improve debug messages
> 
> In particular, the KRB5KRB_AP_ERR_TKT_EXPIRED can happen on the server as well as the client
> 
> Signed-off-by: Andrew Bartlett <abartlet at samba.org>
> ---
>  source4/auth/gensec/gensec_gssapi.c | 65 +++++++++++++++++++++++++++++--------
>  1 file changed, 52 insertions(+), 13 deletions(-)
> 
> diff --git a/source4/auth/gensec/gensec_gssapi.c b/source4/auth/gensec/gensec_gssapi.c
> index ed6d08a..46bd32b 100644
> --- a/source4/auth/gensec/gensec_gssapi.c
> +++ b/source4/auth/gensec/gensec_gssapi.c
> @@ -533,11 +533,6 @@ static NTSTATUS gensec_gssapi_update(struct gensec_security *gensec_security,
>  			OM_uint32 lifetime = 0;
>  			gss_cred_usage_t usage;
>  			const char *role = NULL;
> -			DEBUG(0, ("GSS %s Update(krb5)(%d) Update failed, credentials expired during GSSAPI handshake!\n",
> -				  role,
> -				  gensec_gssapi_state->gss_exchange_count));
> -
> -			
>  			switch (gensec_security->gensec_role) {
>  			case GENSEC_CLIENT:
>  				creds = gensec_gssapi_state->client_cred->creds;
> @@ -549,6 +544,10 @@ static NTSTATUS gensec_gssapi_update(struct gensec_security *gensec_security,
>  				break;
>  			}
>  
> +			DEBUG(0, ("GSS %s Update(krb5)(%d) Update failed, credentials expired during GSSAPI handshake!\n",
> +				  role,
> +				  gensec_gssapi_state->gss_exchange_count));
> +
>  			maj_stat = gss_inquire_cred(&min_stat, 
>  						    creds,
>  						    &name, &lifetime, &usage, NULL);
> @@ -591,15 +590,55 @@ static NTSTATUS gensec_gssapi_update(struct gensec_security *gensec_security,
>  					     gss_mech_krb5)) {
>  			switch (min_stat) {
>  			case KRB5KRB_AP_ERR_TKT_NYV:
> -				DEBUG(1, ("Error with ticket to contact %s: possible clock skew between us and the KDC or target server: %s\n",
> -					  gensec_gssapi_state->target_principal,
> -					  gssapi_error_string(out_mem_ctx, maj_stat, min_stat, gensec_gssapi_state->gss_oid)));
> -				return NT_STATUS_TIME_DIFFERENCE_AT_DC; /* Make SPNEGO ignore us, we can't go any further here */
> +				switch (gensec_security->gensec_role) {
> +				case GENSEC_CLIENT:
> +					DEBUG(1, ("Error with our ticket used to contact %s: "
> +						  "possible clock skew between us and the "
> +						  "KDC or target server: %s\n",
> +						  gensec_gssapi_state->target_principal,
> +						  gssapi_error_string(out_mem_ctx, maj_stat,
> +								      min_stat,
> +								      gensec_gssapi_state->gss_oid)));
> +					/* Make SPNEGO ignore us, we can't go any further here */
> +					return NT_STATUS_TIME_DIFFERENCE_AT_DC;
> +					break;
> +				case GENSEC_SERVER:
> +					DEBUG(1, ("Error with ticket used by client: "
> +						  "possible clock skew between us and the "
> +						  "KDC or client: %s\n",
> +						  gssapi_error_string(out_mem_ctx, maj_stat,
> +								      min_stat,
> +								      gensec_gssapi_state->gss_oid)));
> +					/* Make SPNEGO ignore us, we can't go any further here */
> +					return NT_STATUS_INVALID_PARAMETER;
> +					break;
> +				}
>  			case KRB5KRB_AP_ERR_TKT_EXPIRED:
> -				DEBUG(1, ("Error with ticket to contact %s: ticket is expired, possible clock skew between us and the KDC or target server: %s\n",
> -					  gensec_gssapi_state->target_principal,
> -					  gssapi_error_string(out_mem_ctx, maj_stat, min_stat, gensec_gssapi_state->gss_oid)));
> -				return NT_STATUS_INVALID_PARAMETER; /* Make SPNEGO ignore us, we can't go any further here */
> +				switch (gensec_security->gensec_role) {
> +				case GENSEC_CLIENT:
> +					DEBUG(1, ("Error with ticket to contact %s: "
> +						  "ticket is expired, possible "
> +						  "clock skew between us and "
> +						  "the KDC or target server: %s\n",
> +						  gensec_gssapi_state->target_principal,
> +						  gssapi_error_string(out_mem_ctx,
> +								      maj_stat,
> +								      min_stat,
> +								      gensec_gssapi_state->gss_oid)));
> +					/* Make SPNEGO ignore us, we can't go any further here */
> +					return NT_STATUS_INVALID_PARAMETER;
> +				case GENSEC_SERVER:
> +					DEBUG(1, ("Error with ticket used by client: "
> +						  "ticket is expired, possible "
> +						  "clock skew between us and the "
> +						  "KDC or client: %s\n",
> +						  gssapi_error_string(out_mem_ctx,
> +								      maj_stat,
> +								      min_stat,
> +								      gensec_gssapi_state->gss_oid)));
> +					/* Make SPNEGO ignore us, we can't go any further here */
> +					return NT_STATUS_INVALID_PARAMETER;
> +				}
>  			case KRB5_KDC_UNREACH:
>  				DEBUG(3, ("Cannot reach a KDC we require in order to obtain a ticket to %s: %s\n",
>  					  gensec_gssapi_state->target_principal,
> -- 
> 2.5.0
>