<URL: http://bugs.freeciv.org/Ticket/Display.html?id=39957 >

> [wsimpson - Wed Dec 12 14:25:21 2007]:
> 
> Jason Dorje Short wrote:
> > But the point is that having HACK access allows you to write 
directly to 
> > the filesystem, through the /save command among others.  HACK 
access 
> > should only be given when you do not mind the user having write 
access. 
> >   That is why the hack check is done the way it is now and the 
client is 
> > supposed to be able to write to the file to get it.
> > 
> AFAICT, there is no check in the server code that the client has write
> access, nor that the file was properly deleted.  And the mask isn't
> properly set, either.  There is no security reason for the process.
> 
> Madeline, where is your code?

I assume you mean the ACL code I mentioned previously; that is in
the warclient sources (svn://svn.gna.org/svn/freeciv-warclient/trunk).
Some points of interest:
common/connection.h        -- ALLOW_* enums and conn_pattern 
declarations
common/connection.c        -- conn_pattern creation/application
server/connecthand.h       -- user_action declarations
server/connecthand.c       -- the "ACL" implementation (viz. 
grant_access_level)
server/stdinhand.c         -- commands for saving/loading/modifying the 
ACL (i.e. "actionlist")

But there are some issues if you wish to incorporate this code into
2.1.x:
- Just looking at the code right now there are lots of obvious places
  that could be optimized and improved, not to mention that the style
  does not always follow the freeciv coding guidelines.
- Some of the code is dependent on other warclient "additions" (e.g.
  utility/wildcard.[ch]) or assumes that certain features exist
  (e.g. server async DNS).
- As I have been made aware, there is some issue with hostname= type
  patterns and async DNS resolution - some bugs due to the sequence
  of operations.
- The actual ACL definition file is a plain text file with it's own
  unique (although simple) format. Perhaps a .serverrc (a secfile) or
  something else would be better.
- username= type patterns are pretty useless without server AUTH support
  enabled and working (dependence on mysql).

Hence I am not sure our "solution" is, without some extensive
modifications, acceptable or appropriate to replace the hack mechanism
(e.g. as used to take control of a server that the client launches).

> The AUTH code here is cryptologically
> unsound.

I am slightly confused here in that you change subject to the database
auth code (i.e. server/auth.[ch]) - I assume you meant the hack
mechanism.

As an off-topic comment, no part of the freeciv communication
protocol is really cryptographically sound, but for the most part I
do not think it has to adhere to such stringent measures. Truth
be told, I have on more than one occasion been tempted to add a
dependence on libssl, e.g. so that even the server operator could
be considered an "untrusted party".

> Did the AUTH [i.e. our "ACL"] code come from someplace special? 

No, yaro and I coded it up from scratch more than a year ago. Well, 
except wildcard.[ch], which I grabbed off the internet somewhere.

> Is there any reason to be backward compatible with anything?

With respect to preserving the hack mechanism, it is unlikely that
someone would only upgrade their client and not also their server,
so it would be safe to remove for future versions. That said, it
would seem unnecessary (except perhaps as an excessive measure to
improve security), since there is a capability (new_hack) that
controls its use (e.g. if a new mechanism is created, a new
capability can be introduced and the server can remain compatible with
older clients).



_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

Reply via email to