[Bug 260017] security/cyrus-sasl2: Review and skim patch-plugins_gssapi.c

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=260017

            Bug ID: 260017
           Summary: security/cyrus-sasl2: Review and skim
                    patch-plugins_gssapi.c
           Product: Ports & Packages
           Version: Latest
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Only Me
          Priority: ---
         Component: Individual Port(s)
          Assignee: ume@FreeBSD.org
          Reporter: michael.osipov@siemens.com
          Assignee: ume@FreeBSD.org
             Flags: maintainer-feedback?(ume@FreeBSD.org)

During an evaluation of a new server we have discovered (severe) issues with
that custom patch:

> +#if 0
>     params->utils->log(params->utils->conn, SASL_LOG_DEBUG,
> 		       "GSSAPI client step %d", text->state);
> +#endif

Why are you taking away this debug output from me? I should have access to this
as a developer.

> +	/*
> +	 * If caller didn't provide creds already.
> +	 *
> +	 * In the case of Kerberos, a client typically wants to use
> +	 * a credential in either a keytab file or the credentials cache
> +	 * of the current process context.  This code path will try to
> +	 * find a credential in the specified keytab file,  then the
> +	 * credentials cache.  The keytab file can be specified by
> +	 * "keytab" option, and it is configured by using
> +	 * gsskrb5_register_acceptor_identity() API when available.
> +	 */
> +	if (client_creds == GSS_C_NO_CREDENTIAL) {
> +	    GSS_LOCK_MUTEX_CTX(params->utils, text);
> +	    maj_stat = gss_acquire_cred(&min_stat,
> +					text->server_name,
> +					GSS_C_INDEFINITE,
> +					GSS_C_NO_OID_SET,
> +					GSS_C_INITIATE,
> +					&text->client_creds, 
> +					NULL, 
> +					NULL);
> +	    GSS_UNLOCK_MUTEX_CTX(params->utils, text);
> +
> +	    /*
> +	     * Ignore the error intentionally.  The credential was
> +	     * not found in the specified keytab file.
> +	     */
> +	    if (GSS_ERROR(maj_stat) == 0) {
> +		client_creds = text->client_creds;
> +	    }
> +	}
> +
> +	/* Try the credentials cache. */

This is problematic as a whole. I understand the reason why it was imported,
but it is broken:

It acquires a client credential (GSS_C_INITIATE), but supplies the server name
(text->server_name) for the target cred. That struct is likely memory garbage
and will lead to a SIGSEGV. It has to be NULL. If you check the log of that
file you see that it has been reverted in
https://github.com/cyrusimap/cyrus-sasl/commit/26835b156ec9b69f22c3f15da9c3ab671b2d22dc
for the obvious reasons also described in:
https://github.com/cyrusimap/cyrus-sasl/pull/593

The next hunks suffer from the same broken logical approach and have been
reverted ih the commit from above.

Ideally, the entire file is removed.

-- 
You are receiving this mail because:
You are the assignee for the bug.