Re: [Freeciv-Dev] (PR#39957) multiple bugs in HACK handling

2007-12-13 Thread Jason Dorje Short

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

Jason Dorje Short wrote:
> http://bugs.freeciv.org/Ticket/Display.html?id=39957 >
> 
> William Allen Simpson wrote:
> 
>> Madeline, where is your code?  The AUTH code here is cryptologically
>> unsound.  Did the AUTH code come from someplace special?  Is there any
>> reason to be backward compatible with anything?
> 
> The auth code was written by mike or per as a quick fix to get the 
> client to auto-launch the server.  There is little need for it to be 
> backward-compatible even among the same version of freeciv I think.
> 
> And, how is it cryptologically unsound?

My mistake, I misread - the above reply is about the HACK test code, not 
the auth code.

About the auth code, I don't see why it's needed outside of pubserver. 
If pubserver is replaced universally with GGZ then it could be dropped 
completely.

-jason



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


Re: [Freeciv-Dev] (PR#39957) multiple bugs in HACK handling

2007-12-13 Thread Jason Dorje Short

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

William Allen Simpson wrote:

> Madeline, where is your code?  The AUTH code here is cryptologically
> unsound.  Did the AUTH code come from someplace special?  Is there any
> reason to be backward compatible with anything?

The auth code was written by mike or per as a quick fix to get the 
client to auto-launch the server.  There is little need for it to be 
backward-compatible even among the same version of freeciv I think.

And, how is it cryptologically unsound?

-jason



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


Re: [Freeciv-Dev] (PR#39957) multiple bugs in HACK handling

2007-12-12 Thread William Allen Simpson

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

Madeline Book wrote:
> 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.
> 
No, I was looking at the auth code.


> ...  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".
> 
That would be better in some ways, but SSL/TLS involves a different
problem space.  I'm doubtful that we need encrypted communications,
with a complete Diffie-Hellman key agreement protocol.

If we did, I'd add Photuris instead  In this case, CHAP will be OK.


> 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. ...
> 
Since 2.1 clients will never be able to access 2.2 servers (they will
die() on the unrecognized terrain land and water), that's a good time
to introduce a replacement.

Thanks you for your background information.



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


[Freeciv-Dev] (PR#39957) multiple bugs in HACK handling

2007-12-12 Thread Madeline Book

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


Re: [Freeciv-Dev] (PR#39957) multiple bugs in HACK handling

2007-12-12 Thread William Allen Simpson

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

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?  The AUTH code here is cryptologically
unsound.  Did the AUTH code come from someplace special?  Is there any
reason to be backward compatible with anything?




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


Re: [Freeciv-Dev] (PR#39957) multiple bugs in HACK handling

2007-12-11 Thread Jason Dorje Short

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

William Allen Simpson wrote:
> http://bugs.freeciv.org/Ticket/Display.html?id=39957 >
> 
> Jason Dorje Short wrote:
>> Also the whole point of the HACK check is that the client SHOULD have 
>> permission to delete the file created.  If the client cannot write to 
>> that file then it should not be granted hack access.
> 
> Speaking as an Internet security expert, that's just plain wrong.  It's
> merely a token, used as a shared-secret.  It's bad enough that it's used
> as a plaintext password.
> 
> For security, the files should be controlled and updated by the server.
> The client should have no more than read access, especially as the
> current scheme is designed for multiple clients accessing the server
> installed in a common directory.
> 
> Moreover, in a properly designed protocol, the client should be able to
> access the server at various control levels remotely.  For 2.2 or 2.3
> 
> For 2.1, I'm just fixing the wrongly sent packets!

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.

-jason



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


[Freeciv-Dev] (PR#39957) multiple bugs in HACK handling

2007-12-11 Thread Madeline Book

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

> [jdorje - Tue Dec 11 06:55:12 2007]:
> 
> > [book - Mon Dec 10 16:22:28 2007]:
> 
> > We ameliorated this situation somewhat for warserver by
> > introducing/modifying the cmdlevels to:
> 
> [...]
> 
> What's the difference between ADMIN and CTRL access levels?
> 
> -jason
> 
>

Pepeto Already answered this in general, but if you want excessive
detail :), here is a grep from server/commands.c [if you recall,
the first cmdlevel is the in-game level, whereas the second is
pregame]:

  {"cut",   ALLOW_CTRL, ALLOW_CTRL,
  {"ban",   ALLOW_ADMIN, ALLOW_ADMIN,
  {"unban", ALLOW_ADMIN, ALLOW_ADMIN,
  {"score", ALLOW_CTRL, ALLOW_NEVER,
  {"wall",  ALLOW_ADMIN, ALLOW_ADMIN,
  {"set",   ALLOW_CTRL, ALLOW_BASIC,
  {"metaconnection",ALLOW_ADMIN, ALLOW_ADMIN,
  {"metaserver",ALLOW_ADMIN, ALLOW_ADMIN,
  {"aitoggle",  ALLOW_CTRL, ALLOW_CTRL,
  {"create",ALLOW_NEVER, ALLOW_CTRL,
  {"novice",ALLOW_CTRL, ALLOW_BASIC,
  {"easy",  ALLOW_CTRL, ALLOW_BASIC,
  {"normal",ALLOW_CTRL, ALLOW_BASIC,
  {"hard",  ALLOW_CTRL, ALLOW_BASIC,
  {"experimental",  ALLOW_CTRL, ALLOW_BASIC,
  {"cmdlevel",  ALLOW_ADMIN, ALLOW_ADMIN, /* confusing at ALLOW_CTRL */
  {"timeoutincrease", ALLOW_CTRL, ALLOW_BASIC,
  {"autoteam", ALLOW_NEVER, ALLOW_CTRL, /* require vote in pregame */
  {"mute", ALLOW_CTRL, ALLOW_CTRL,
  {"unmute", ALLOW_CTRL, ALLOW_CTRL,
  {"endgame",   ALLOW_CTRL, ALLOW_NEVER,
  {"remove",ALLOW_ADMIN, ALLOW_CTRL,
  {"save",  ALLOW_ADMIN, ALLOW_ADMIN,
  {"load",  ALLOW_NEVER, ALLOW_ADMIN,
  {"loadmap",  ALLOW_NEVER, ALLOW_CTRL,
  {"unloadmap",  ALLOW_NEVER, ALLOW_CTRL,
  {"loadscenario",  ALLOW_NEVER, ALLOW_CTRL,
  {"reset", ALLOW_NEVER, ALLOW_CTRL,
  {"dnslookup", ALLOW_ADMIN, ALLOW_ADMIN,
  {"addaction", ALLOW_ADMIN, ALLOW_ADMIN,
  {"delaction", ALLOW_ADMIN, ALLOW_ADMIN,


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


[Freeciv-Dev] (PR#39957) multiple bugs in HACK handling

2007-12-11 Thread Pepeto _

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

> [jdorje - Mar. Déc. 11 06:55:12 2007]:
> 
> > [book - Mon Dec 10 16:22:28 2007]:
> 
> > We ameliorated this situation somewhat for warserver by
> > introducing/modifying the cmdlevels to:
> 
> [...]
> 
> What's the difference between ADMIN and CTRL access levels?
> 
> -jason
> 
> 

ADMIN allows all commands which are not dangerous for the server
(doesn't have file support) but which cannot be obtain by vote. One of
the main possibility of ADMIN is the user support (action list).

However, by experience, people who host warservers doesn't give ADMIN to
anyone, but HACK directly. In a classical action list, you have only:
version=1
basic username=pepeto # your own nick, to don't get hack, which can be
annoying while the game is running
hack address=82.236.117.22 # your ip to get hack otherwise
basic address=* # basic for all others connection


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


Re: [Freeciv-Dev] (PR#39957) multiple bugs in HACK handling

2007-12-11 Thread William Allen Simpson

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

Jason Dorje Short wrote:
> Also the whole point of the HACK check is that the client SHOULD have 
> permission to delete the file created.  If the client cannot write to 
> that file then it should not be granted hack access.

Speaking as an Internet security expert, that's just plain wrong.  It's
merely a token, used as a shared-secret.  It's bad enough that it's used
as a plaintext password.

For security, the files should be controlled and updated by the server.
The client should have no more than read access, especially as the
current scheme is designed for multiple clients accessing the server
installed in a common directory.

Moreover, in a properly designed protocol, the client should be able to
access the server at various control levels remotely.  For 2.2 or 2.3

For 2.1, I'm just fixing the wrongly sent packets!



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


[Freeciv-Dev] (PR#39957) multiple bugs in HACK handling

2007-12-10 Thread Jason Short

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

> [book - Mon Dec 10 16:22:28 2007]:

> We ameliorated this situation somewhat for warserver by
> introducing/modifying the cmdlevels to:

[...]

What's the difference between ADMIN and CTRL access levels?

-jason


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


Re: [Freeciv-Dev] (PR#39957) multiple bugs in HACK handling

2007-12-10 Thread Jason Dorje Short

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

William Allen Simpson wrote:
> http://bugs.freeciv.org/Ticket/Display.html?id=39957 >
> 
> server/gamehand.c
>handle_single_want_hack_req()
>  Sent send_ruleset_choices() when HACK is not successful.
>  Redundant send_conn_info() when HACK is not successful.
> 
> client/connectdlg_common.c
>handle_single_want_hack_reply()
>  File should be deleted by server, client doesn't always have permission.
> 
> Implies every client request should have different file?
> 
> Really need different paradigm for 2.2, perhaps the AUTH system?

The purpose of a file-driven HACK check was always to make sure that the 
local connection was given hack access when the client launches the 
server.  It will also give you hack access when you launch client and 
server separately from the same computer (and account); whether this is 
needed I'm not sure.

The former case could be handled more cleanly using environment 
variables.  The client sets the FREECIV_HACK_PASSWORD environment 
variable to some large bit of pseudo-random garbage.  On connecting it 
passes this bit of garbage to the server which is then used to verify 
the connection and provide HACK access.  The only problem is that this 
will ONLY work if the client controls the server's environment - i.e., 
when the client launches the server.

Also the whole point of the HACK check is that the client SHOULD have 
permission to delete the file created.  If the client cannot write to 
that file then it should not be granted hack access.  And yes, a 
separate file is needed for each connection.

-jason




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


[Freeciv-Dev] (PR#39957) multiple bugs in HACK handling

2007-12-10 Thread Madeline Book

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

> [wsimpson - Sun Dec 09 22:15:45 2007]:
> 
> server/gamehand.c
>handle_single_want_hack_req()
>  Sent send_ruleset_choices() when HACK is not successful.
>  Redundant send_conn_info() when HACK is not successful.
> 
> client/connectdlg_common.c
>handle_single_want_hack_reply()
>  File should be deleted by server, client doesn't always have
> permission.
> 
> Implies every client request should have different file?
> 
> Really need different paradigm for 2.2, perhaps the AUTH system?
> 

We ameliorated this situation somewhat for warserver by
introducing/modifying the cmdlevels to:

  ALLOW_NONE = 0, /* user may issue no commands at all */
  ALLOW_OBSERVER, /* user may issue observer commands */
  ALLOW_BASIC,/* user may issue basic commands - default */
  ALLOW_CTRL, /* user may issue commands that affect game & 
users */
  ALLOW_ADMIN,/* admin user */
  ALLOW_HACK, /* user may issue *all* commands - dangerous! */

and implementing an ACL like system for assigning cmdlevels to
new connections based on action results:

  ACTION_BAN = 0,
  ACTION_GIVE_NONE,
  ACTION_GIVE_OBSERVER,
  ACTION_GIVE_BASIC,
  ACTION_GIVE_CTRL,
  ACTION_GIVE_ADMIN,
  ACTION_GIVE_HACK,

The ACL is a text file that gives the rules for computing the above
results per connection, e.g.:

ban   *.foo.com  # same as hostname=*.foo.com
ban   address=34.67.123.234
ctrl  username=book
admin address=127.0.0.1
basic *  # everyone else gets basic 

Obviously before the ACL can be applied, AUTH must be enabled and
working and the hostname lookup must be completed. Now that I
think of it, it would probably better to make an ACL table in the
mysql database and load/save the rules from there.

I would not copy our implementation wholesale; there is lots of
cruft from older "experimental" versions bloating the ACL code, and
probably some bugs lurking around. But in the interests of sharing
our ideas and preventing you from re-inventing the wheel, I hope
this is of some help to you. :)

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


[Freeciv-Dev] (PR#39957) multiple bugs in HACK handling

2007-12-09 Thread William Allen Simpson

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

server/gamehand.c
   handle_single_want_hack_req()
 Sent send_ruleset_choices() when HACK is not successful.
 Redundant send_conn_info() when HACK is not successful.

client/connectdlg_common.c
   handle_single_want_hack_reply()
 File should be deleted by server, client doesn't always have permission.

Implies every client request should have different file?

Really need different paradigm for 2.2, perhaps the AUTH system?



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