Interix Port

[Peter Stuge]

Just a quick review..

Markus Duft wrote:
> +++ openssh-5.5p1/auth.c	2010-10-22 14:11:48 +0200
> @@ -541,6 +541,25 @@
>  
>  	pw = getpwnam(user);
>  
> +#ifdef __INTERIX
> +    /* on windows, is there is no such user in the principal domain

Typo: if there is


> +	 * (which is checked by default), we also have a look at the
> +	 * local accounts by prefixing the username with the hostname
> +	 */
> +	if (pw == NULL) {
> +		char tmp[MAXHOSTNAMELEN];
> +		if(gethostname(tmp, MAXHOSTNAMELEN) == 0) {
> +			strcat(tmp, "+");
> +			strcat(tmp, user);

Buffer overflow. Do not create bugs like this!! Consider what it says
about the rest of your code.


> +++ openssh-5.5p1/openbsd-compat/bsd-misc.h	2010-10-22 14:11:48 +0200
..
> +#ifndef __INTERIX

> +++ openssh-5.5p1/openbsd-compat/getrrsetbyname.c	2010-10-22 14:11:48 +0200
..
> +#if !defined(__INTERIX)

Why mix the two styles?


>  int
>  getrrsetbyname(const char *hostname, unsigned int rdclass,
>      unsigned int rdtype, unsigned int flags,
>      struct rrsetinfo **res)
>  {
> +#if defined(__INTERIX)
> +	return (ERRSET_FAIL);
> +#else

Maybe create functionality flags for this, rather than test system
type everywhere in the code.


> +++ openssh-5.5p1/session.c	2010-10-22 14:11:48 +0200
> @@ -91,6 +91,27 @@
>  #include "monitor_wrap.h"
>  #include "sftp.h"
>  
> +#ifdef __INTERIX
> +# include <interix/env.h>
> +# include <interix/security.h>
> +char* InterixPwdToken = NULL;
> +
> +#define INTERIX_PWD_WARNING \
> +		fprintf(stderr,"@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@\n" \
> +					   "@ WARNING: Due to limitations in the POSIX Subsystem and Win32  @\n" \
> +					   "@     a Password is required to acquire a full authentication.  @\n" \
> +					   "@     Without such an authentication token, certain things will @\n" \
> +					   "@     only be available in a very limited way (Visual Studio's  @\n" \
> +					   "@     link.exe can only link without debug information, network @\n" \
> +					   "@     shares that require user authentication don't fully work, @\n" \
> +					   "@     etc.). However if you don't require those things to work, @\n" \
> +					   "@     you may be just fine without password (public-key, etc.). @\n" \
> +					   "@ To obtain a full authentication you need to use password      @\n" \
> +					   "@ authentication at the moment. To do so, remove your public    @\n" \
> +					   "@ key from your ~/.ssh/authorized_keys[2] file(s).              @\n" \
> +					   "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@\n");
> +#endif

Strongly suggest that you make the define contain only the string and
nothing else.


..
> +#ifdef __INTERIX
> +		/* on interix, to get a full env, we _need_ the plain text
> +		 * password during this! */
> +		if(InterixPwdToken) {
> +			debug2("re-setting user password");
> +			strcpy(pw->pw_passwd, InterixPwdToken);
> +		} else {
> +			INTERIX_PWD_WARNING
> +		}
> +#endif

So that it is more obvious what this code actually does in the
warning case.


> @@ -1607,6 +1662,13 @@
>  launch_login(struct passwd *pw, const char *hostname)
>  {
>  	/* Launch login(1). */
> +	#ifdef __INTERIX
> +	/* -f only works if the user is already autheticated as the requested user */
> +	if (!InterixPwdToken)
> +		INTERIX_PWD_WARNING

And this will of course fail miserably if the define gets expanded to
have more than a single statement. You may have seen the use of do {}
while(0) in macros with code.


> +	if(getuid() == 0) {
> +		struct passwd* _admin = getpwuid(getuid());
> +
> +		if(!_admin) {
> +			fprintf(stderr, "Cannot retrieve user information for current user!\n");
> +			exit(2);
> +		}

I think it's a no-no to use stderr directly. Why don't you
consistently use the log infrastructure like the rest of the code.


> +++ openssh-5.5p1/uidswap.c	2010-10-22 14:13:47 +0200
..
> @@ -220,6 +228,20 @@
>  	debug("permanently_set_uid: %u/%u", (u_int)pw->pw_uid,
>  	    (u_int)pw->pw_gid);
>  
> +#ifdef __INTERIX
> +	if (0) {
> +		fprintf(stderr, "WARNING: Your password will expire soon. This will make remote\n"
> +						"logins via ssh impossible without notice of the occured error!\n");
> +	}

What is this about? Don't produce dead code!!


//Peter