[Freeciv-Dev] Re: client/server authentication (PR#1767)

["Mike Kaufman" <kaufman@xxxxxxxxxxxxxxxxxxxxxx>]

On Fri, May 23, 2003 at 02:14:12AM -0700, Raimar Falke wrote:
> > and...? we don't have a connectdlg_common.c yet, and 
> 
> > I'm not going to make one for this triviality
> 
> Why not?

well, for one these are not _required_ for all clients. Two, we're talking
about _one_ enum. I mean come on.

> > > IMHO should a connection have a pointer to a struct user. This field
> > > should then go.
> > 
> > no. I thought about this. This would require that user.h be in common and
> > that is not right as the client has absolutely nothing to do with that
> > struct. The client should not have access to it.
> 
> Not at all. You don't have to expose the content of struct user if you
> use a pointer.

this is six or one-half dozen the other. If you put in a struct user, then
what's the point of having a pconn->username? and then the client needs to
know what's in pconn->user, etc., so I think this change is more trouble 
than it's worth.

>             number of possible passwords
>  time =    ----------------------------------------------------------
>             guessing speed (how many passwords can be tried per sec)
> 
> You increased the numerator with is_good_password. But this doesn't
> help if we also don't limit the denominator to a low value.

well, you convinced me to look into it, but I will not venture into the
territory of limiting _connections_, or keeping track of IP addresses. That
is definitely beyond the scope of the patch.
 
> > > > +#define NEW_USERS_ALLOWED  /* if defined, new users may log in. */
> > > > +#define GUESTS_ALLOWED     /* if defined, users can log in as guests */
> > > 
> > > These settings are hidden quite nice in this file. I think that these
> > > two settings and the settings below should be server variables.
> > 
> > sure, but there is a perpetual argument over new server options are you are
> > advocating five (or at least 3) new ones in a single stroke. Until a better
> > system in stdinhand is worked out, the answer will be: not in this patch.
> 
> I agree that we need some improvement. But look at this problem from
> another angle: if we add now the auth options and some other options
> in the future we may increase the pressure to a range where people
> really start working on this area. So far everybody agreed that it
> needs fixing but is low priority. At least this is my impression.

this is not an adequate argument for me to change it at this time. There
are several things that need doing once this patch is in; perhaps I'll take
the time to do this as well.
 
> > > I would just let the server answer "error, please try guest".
> > 
> > uh, this won't work if guest logins aren't allowed...
> 
> And the server will answer "you lost ;)" if if guest logins aren't
> allowed.

I don't understand you.
 
 > +/**************************************************************************
> > > > +  Receives a password from a client and verifies it.
> > > > +**************************************************************************/
> > > > +bool handle_authentication_reply(struct connection *pconn,
> > > 
> > > Since the check for
> > >   if (pconn->server.authentication_tries >= MAX_AUTHENTICATION_TRIES) {
> > > is quite universal I would move it at the top.
> > 
> > Are you actually reading the patch Raimar? There are five or six paths
> > through this function are only two of them require that check...
> 
> I know. But does it harm if we add this check to all code paths? IMHO
> not.

as written it does. a good login might be rejected. I'll look at it
however.
 
> fileno() will yield this. I'm not sure if this is more portable than
> S_IRUSR. Just choose the one which has less configure problems.

doing some searching, it appears that fileno() is POSIX.1 plus a bunch of
other crap, so it should be as portable as S_IRUSR, so I'll use it.
 
-mike