Re: [Freeciv-Dev] (PR#39957) multiple bugs in HACK handling
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
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
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
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
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
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
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
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
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
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
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
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
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