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