[PATCH] libads: record service ticket endtime for sealed ldap connections

[Ralph Böhme]

Hi!

On Tue, May 12, 2015 at 10:47:20AM -0700, Jeremy Allison wrote:
> On Sat, May 09, 2015 at 10:59:17PM +0300, Uri Simchoni wrote:
> > When a ticket is obtained for binding a signed/sealed ldap connection,
> > its liftime should be recorded in the ads struct, in order to enable
> > reuse of the connection.
> 
> Oh that's a really smart catch - thanks !
> 
> However I think we should also handle the
> gss_context_time returns context_validity == 0
> case where the context has already expired.
> 
> Does the following also work for you ?
> 
> From daa6c09b7a2931aacd531bb49daf6e47e843a059 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <urisimchoni at gmail.com>
> Date: Sat, 9 May 2015 22:59:17 +0300
> Subject: [PATCH] libads: record service ticket endtime for sealed ldap
>  connections
> 
> When a ticket is obtained for binding a signed/sealed ldap connection,
> its liftime should be recorded in the ads struct, in order to enable
> reuse of the connection.
> 
> Signed-off-by: Uri Simchoni <urisimchoni at gmail.com>
> Reviewed-by: Jeremy Allison <jra at samba.org>
> ---
>  source3/libads/sasl.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/source3/libads/sasl.c b/source3/libads/sasl.c
> index ce3740f..af50186 100644
> --- a/source3/libads/sasl.c
> +++ b/source3/libads/sasl.c
> @@ -458,6 +458,8 @@ static ADS_STATUS ads_sasl_spnego_gsskrb5_bind(ADS_STRUCT *ads, const gss_name_t
>  	DATA_BLOB unwrapped;
>  	DATA_BLOB wrapped;
>  	struct berval cred, *scred = NULL;
> +	uint32_t context_validity;
> +	time_t context_endtime = 0;
>  
>  	status = ads_init_gssapi_cred(ads, &gss_cred);
>  	if (!ADS_ERR_OK(status)) {
> @@ -652,6 +654,23 @@ static ADS_STATUS ads_sasl_spnego_gsskrb5_bind(ADS_STRUCT *ads, const gss_name_t
>  		goto failed;
>  	}
>  
> +	gss_rc =
> +	    gss_context_time(&minor_status, context_handle, &context_validity);
> +	if (gss_rc == 0) {

shouldn't we use the proper defines per the doc instead of simply
checking 0? Haven't looked at what the rest of the codes does wrtt,
but this doesn't look right to me.

Cf <http://www.gnu.org/software/gss/manual/html_node/Context_002dLevel-Routines.html>

Return value:

GSS_S_COMPLETE: Successful completion.
GSS_S_CONTEXT_EXPIRED: The context has already expired.
GSS_S_NO_CONTEXT: The context_handle parameter did not identify a valid context

But to me that doesn't make it clear whethe an expired context should
be detected via GSS_S_COMPLETE && (context_validity == 0), or via
GSS_S_CONTEXT_EXPIRED.

Afaict an expired context will return GSS_S_CONTEXT_EXPIRED, not
GSS_S_COMPLETE (== 0?) and context_validity = 0. Go figure... ;)

-Ralph

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de,mailto:kontakt@sernet.de