[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: KDE/kdebase/workspace/kdm/backend
From:       Oswald Buddenhagen <ossi () kde ! org>
Date:       2009-01-03 9:49:37
Message-ID: 20090103094937.GA32349 () kegelstatt ! local
[Download RAW message or body]

hi,

On Tue, Dec 16, 2008 at 02:50:04PM +0000, Lubo?? Lu????k wrote:
> After successful authorization always ask PAM for the login name,
> instead of using whatever the user typed, this helps e.g. with LDAP,
> which ignores leading and trailing whitespace, so such incorrectly
> typed login can succeed but the session will be broken.
> 
> --- trunk/KDE/kdebase/workspace/kdm/backend/client.c #897661:897662
> -	if (!curuser) {
> -		debug( " asking PAM for user ...\n" );
> -		pam_get_item( pamh, PAM_USER, &pitem );
> -		reInitErrorLog();
> -		strDup( &curuser, (const char *)pitem );
> -		gSendInt( V_PUT_USER );
> -		gSendStr( curuser );
> -	}
> +	// normalize name (e.g. ldap removes whitespace)
> +	debug( " asking PAM for user ...\n" );
> +	pam_get_item( pamh, PAM_USER, &pitem );
> +	reInitErrorLog();
> +	if( curuser )
> +		free( curuser );
> +	strDup( &curuser, (const char *)pitem );
> +	gSendInt( V_PUT_USER );
> +	gSendStr( curuser );
>
this patch is wrong for multiple reasons:

- conversation plugins in the frontend which cause curuser to be set do
  not expect V_PUT_USER being used. disrespecting that might lead to
  deadlocks, crashes, children being eaten, etc.

  in particular, you broke autologin.

- the retrieval of the previous session type selection is bound to that
  variable. this is usually done after the user clicked his name
  (uncritical case) or typed it in (critical case) without actually
  starting an authentication cycle, i.e., before pam has a chance to
  normalize the name. while kdm itself handles that gracefully (to be
  able to cope with home dirs inaccessible prior to authentication), the
  behavior will be different (and thus confusing) depending on whether
  the name was normalized in the first place.

  therefore the get-username-from-pam case is reserved for the cases
  where the user did not manually enter a username in the first place,
  i.e., for alternative authentication schemes.

  kdm already tried to deal with ldap's sneaky normalization,
  specifically the case-changing - see the strcpy() near the top of
  startClient() in client.c. of course that works only when getpwnam()
  works with the denormalized name in the first place. the whitespace
  normalization (only leading and trailing, i assume?) should be handled
  already a the login prompt; it should not affect non-ldap schemes in a
  bad way.
[prev in list] [next in list] [prev in thread] [next in thread]