Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Server does not enforce login #363

Closed
jacwah opened this issue Aug 19, 2015 · 4 comments
Closed

Server does not enforce login #363

jacwah opened this issue Aug 19, 2015 · 4 comments

Comments

@jacwah
Copy link
Contributor

jacwah commented Aug 19, 2015

Currently, a login message from the client is no more than a way to set their user name. It's possible to run any command on the server without first authenticating. Below is a proof of concept I made by connecting to the server with telnet and running a query and a start game command without any prior login.

$ telnet localhost 4242
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
{"command":"query","request":"USERS","message":""}
{"command":"userstatus","userId":2,"status":"ONLINE","name":"AI Loser"}{"command":"userstatus","userId":3,"status":"ONLINE","name":"AI Idiot"}{"command":"userstatus","userId":4,"status":"ONLINE","name":"AI Medium"}{"command":"userstatus","userId":5,"status":"ONLINE","name":"AI Fighter"}{"command":"userstatus","userId":7,"status":"ONLINE","name":""}{"command":"userstatus","userId":8,"status":"ONLINE","name":"jacwah"}{"command":"userstatus","userId":8,"status":"OFFLINE","name":"jacwah"}
{"command":"startgame","opponent":2,"gameType":"Cyborg-Chronicles"}
{"command":"wait","message":"Waiting for opponent..."}{"command":"newgame","gameId":2,"playerIndex":1}{"command":"playerconfig","configs":{"Deck":{"_type":"DeckConfig","cardData":{"3":{"command":"card","id":3,"properties":{"SCRAP":3,"TAUNT":1,"flavor":"Cobbled together from whatever was lying around at the time.","MAX_HEALTH":1,"SICKNESS":0,"MANA_COST":0,"name":"Spareparts","ATTACK":0,"creatureType":"Mech","HEALTH":1,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"4":{"command":"card","id":4,"properties":{"SCRAP":1,"MAX_HEALTH":1,"DENY_COUNTERATTACK":1,"MANA_COST":1,"creatureType":"Mech","TAUNT":1,"flavor":"A flying, spherical droid that shoots weak laser beams at nearby targets.","SICKNESS":1,"effect":"Change HEALTH by -1 on creatures owned by opponent on Battlefield\n\n","name":"Gyrodroid","ATTACK":1,"HEALTH":1,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"5":{"command":"card","id":5,"properties":{"SCRAP":1,"TAUNT":1,"flavor":"Looks like a flying circular blade with a sphere in the middle.","MAX_HEALTH":1,"SICKNESS":0,"effect":"Choose 1 at random: Deal 2 damage to opponent\n or Deal 1 damage to you\n","MANA_COST":2,"name":"The Chopper","ATTACK":2,"creatureType":"Mech","HEALTH":1,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"6":{"command":"card","id":6,"properties":{"SCRAP":1,"TAUNT":1,"flavor":"A small, flying shield generator droid.","MAX_HEALTH":3,"SICKNESS":1,"effect":"Give creatures owned by you on Battlefield 1 MAX_HEALTH\n","MANA_COST":3,"name":"Shieldmech","ATTACK":1,"creatureType":"Mech","HEALTH":3,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"7":{"command":"card","id":7,"properties":{"SCRAP":1,"TAUNT":1,"flavor":"Common and inexpensive robot often use for personal protection.","MAX_HEALTH":2,"SICKNESS":1,"MANA_COST":2,"name":"Robot Guard","ATTACK":2,"creatureType":"Mech","HEALTH":2,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"8":{"command":"card","id":8,"properties":{"SCRAP":2,"TAUNT":1,"flavor":"You might mistake it for a human, but it won’t mistake you for a Mech.","MAX_HEALTH":3,"SICKNESS":1,"effect":"Change HEALTH by -2 on 1 random creatures of type Bio owned by opponent on Battlefield\n\n at start of your turn","MANA_COST":4,"name":"Humadroid","ATTACK":3,"creatureType":"Mech","HEALTH":3,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"9":{"command":"card","id":9,"properties":{"SCRAP":2,"TAUNT":1,"flavor":"Humanoid in form, except for two massive cannons in place of arms.","MAX_HEALTH":2,"DENY_COUNTERATTACK":1,"SICKNESS":1,"MANA_COST":4,"name":"Assassinatrix","ATTACK":4,"creatureType":"Mech","HEALTH":2,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"10":{"command":"card","id":10,"properties":{"SCRAP":2,"TAUNT":1,"flavor":"About the only place that a person is safe during a firefight is inside one of these.","MAX_HEALTH":4,"SICKNESS":1,"effect":"Change HEALTH by 2 on 1 random creatures of type Bio owned by you on Battlefield\n\n at start of your turn","MANA_COST":3,"name":"Fortimech","ATTACK":2,"creatureType":"Mech","HEALTH":4,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"11":{"command":"card","id":11,"properties":{"SCRAP":2,"TAUNT":1,"flavor":"The fastest mech on two legs. You don’t want to see the ones with four.","MAX_HEALTH":1,"SICKNESS":0,"MANA_COST":3,"name":"Scout Mech","ATTACK":5,"creatureType":"Mech","HEALTH":1,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"12":{"command":"card","id":12,"properties":{"SCRAP":3,"TAUNT":1,"flavor":"Worth more than its weight in scrap, and it is pretty heavy.","MAX_HEALTH":5,"SICKNESS":0,"MANA_COST":3,"name":"Supply Mech","ATTACK":0,"creatureType":"Mech","HEALTH":5,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"13":{"command":"card","id":13,"properties":{"SCRAP":2,"TAUNT":1,"flavor":"The Field Medical Unit is equipped with modern laser surgical tools and a variety of remedy shots.","MAX_HEALTH":4,"SICKNESS":1,"effect":"Heal 1 to you\n\n at end of your turn","MANA_COST":4,"name":"F.M.U.","ATTACK":0,"creatureType":"Mech","HEALTH":4,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"14":{"command":"card","id":14,"properties":{"SCRAP":3,"TAUNT":1,"flavor":"Uses the legs of other bots to enhance its own speed.","MAX_HEALTH":3,"SICKNESS":0,"MANA_COST":6,"name":"Modleg Ambusher","ATTACK":5,"creatureType":"Mech","HEALTH":3,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"15":{"command":"card","id":15,"properties":{"SCRAP":3,"TAUNT":1,"flavor":"The bigger they are, the harder they fall. Eventually.","MAX_HEALTH":6,"SICKNESS":1,"MANA_COST":5,"name":"Heavy Mech","ATTACK":3,"creatureType":"Mech","HEALTH":6,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"16":{"command":"card","id":16,"properties":{"SCRAP":3,"TAUNT":1,"flavor":"Armored and armed with superior arms.","MAX_HEALTH":4,"SICKNESS":1,"MANA_COST":5,"name":"Waste Runner","ATTACK":4,"creatureType":"Mech","HEALTH":4,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"17":{"command":"card","id":17,"properties":{"SCRAP":1,"TAUNT":1,"flavor":"Upgrades itself on each new turn up to Mk V.","MAX_HEALTH":3,"SICKNESS":0,"effect":"Summon 1 Upgrado Mk II to your Battlefield\n at start of your turn\nSet HEALTH to 0 on this card\n\n at start of your turn","MANA_COST":3,"name":"Upgrado Mk I","ATTACK":1,"creatureType":"Mech","HEALTH":3,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"22":{"command":"card","id":22,"properties":{"TAUNT":1,"flavor":"He just signed up last week and he is very excited to fight.","MAX_HEALTH":2,"SICKNESS":0,"effect":"Deal 1 damage to opponent\n","MANA_COST":2,"name":"Conscript","ATTACK":2,"creatureType":"Bio","HEALTH":2,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"23":{"command":"card","id":23,"properties":{"TAUNT":1,"flavor":"Eyes and reflexes augmented for maximum deadliness.","MAX_HEALTH":1,"DENY_COUNTERATTACK":1,"SICKNESS":1,"MANA_COST":3,"name":"Longshot","ATTACK":3,"creatureType":"Bio","HEALTH":1,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"24":{"command":"card","id":24,"properties":{"TAUNT":1,"flavor":"Strength augmented with mechanical musculature.","MAX_HEALTH":3,"SICKNESS":1,"MANA_COST":3,"name":"Bodyman","ATTACK":2,"creatureType":"Bio","HEALTH":3,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"25":{"command":"card","id":25,"properties":{"TAUNT":1,"flavor":"A retired conscript with a desire to jack and make some quick creds.","MAX_HEALTH":3,"SICKNESS":1,"effect":"Change SCRAP by 1 on you\n\n","MANA_COST":5,"name":"Vetter","ATTACK":3,"creatureType":"Bio","HEALTH":3,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"26":{"command":"card","id":26,"properties":{"TAUNT":1,"flavor":"Unsung hero responsible for keeping countless troops alive.","MAX_HEALTH":5,"SICKNESS":1,"effect":"Heal 1 to you\n\n at end of your turn","MANA_COST":5,"name":"Field Medic","ATTACK":1,"creatureType":"Bio","HEALTH":5,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"27":{"command":"card","id":27,"properties":{"TAUNT":1,"flavor":"Spent his life learning the lessons of the wastelands.","MAX_HEALTH":4,"SICKNESS":1,"effect":"Give creatures of type Bio owned by opponent on Battlefield -1 ATTACK\n","MANA_COST":6,"name":"Wastelander","ATTACK":4,"creatureType":"Bio","HEALTH":4,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"28":{"command":"card","id":28,"properties":{"TAUNT":1,"flavor":"A professional soldier for the government.","MAX_HEALTH":3,"SICKNESS":0,"effect":"Change HEALTH by -1 on creatures owned by opponent on Battlefield\n\n","MANA_COST":6,"name":"Commander","ATTACK":5,"creatureType":"Bio","HEALTH":3,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"29":{"command":"card","id":29,"properties":{"TAUNT":1,"flavor":"Supersized and heavily augmented.","MAX_HEALTH":5,"SICKNESS":1,"MANA_COST":6,"name":"Cyberpimp","ATTACK":3,"creatureType":"Bio","HEALTH":5,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"30":{"command":"card","id":30,"properties":{"TAUNT":1,"flavor":"He’s more machine than human now.","MAX_HEALTH":5,"SICKNESS":1,"MANA_COST":7,"name":"Cyborg","ATTACK":5,"creatureType":"Bio","HEALTH":5,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"31":{"command":"card","id":31,"properties":{"TAUNT":1,"flavor":"Leader of a gang that primarily operates on the web.","MAX_HEALTH":6,"SICKNESS":1,"effect":"Give creatures of type Mech owned by you on Battlefield 1 ATTACK and HEALTH\n","MANA_COST":8,"name":"Web Boss","ATTACK":6,"creatureType":"Bio","HEALTH":6,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"32":{"command":"card","id":32,"properties":{"TAUNT":1,"flavor":"A government official with wider web control. Usually brings friends.","MAX_HEALTH":6,"SICKNESS":1,"effect":"Choose 2 at random: Summon 1 Bodyman to your Battlefield\n or Summon 1 Conscript to your Battlefield\n or Summon 1 Longshot to your Battlefield\n","MANA_COST":9,"name":"Inside Man","ATTACK":2,"creatureType":"Bio","HEALTH":6,"ATTACK_AVAILABLE":1},"command":"card","zone":0},"33":{"command":"card","id":33,"properties":{"flavor":"These arms will give strength to even the most puny individual.","name":"Bionic Arms","ATTACK":2,"SCRAP_COST":1},"command":"card","zone":0},"34":{"command":"card","id":34,"properties":{"flavor":"Steel-reinforced armor to absord damage from blows and shots.","name":"Body Armor","MAX_HEALTH":2,"HEALTH":2,"SCRAP_COST":1},"command":"card","zone":0},"35":{"command":"card","id":35,"properties":{"flavor":"An injection to increase speed and body function.","MAX_HEALTH":1,"SCRAP_COST":1,"effect":"Set SICKNESS to 0","name":"Adrenalin Injection","ATTACK":1,"HEALTH":1},"command":"card","zone":0},"36":{"command":"card","id":36,"properties":{"flavor":"Intravenous implants that feed the body for increased strength.","name":"Steroid Implants","MAX_HEALTH":1,"ATTACK":2,"HEALTH":1,"SCRAP_COST":2},"command":"card","zone":0},"37":{"command":"card","id":37,"properties":{"flavor":"Offers head protection as well as a slight increase in brain activity.","name":"Reinforced Cranial Implants","MAX_HEALTH":2,"ATTACK":1,"HEALTH":2,"SCRAP_COST":2},"command":"card","zone":0},"38":{"command":"card","id":38,"properties":{"flavor":"Replaces the forearm with a powerful firearm for massive damage.","MAX_HEALTH":0,"SCRAP_COST":2,"effect":"Set DENY_COUNTERATTACK to 1","name":"Cybernetic Arm Cannon","ATTACK":3,"HEALTH":0},"command":"card","zone":0},"39":{"command":"card","id":39,"properties":{"flavor":"This very invasive operation reinforces bone tissue with titanium.","name":"Exoskeleton","MAX_HEALTH":3,"HEALTH":3,"SCRAP_COST":2},"command":"card","zone":0},"40":{"command":"card","id":40,"properties":{"flavor":"An advanced processor is connected to the subject's brain, replacing personality with extreme intelligence and reflexes.","name":"Artificial Intelligence Implants","MAX_HEALTH":3,"ATTACK":2,"HEALTH":3,"SCRAP_COST":3},"command":"card","zone":0},"41":{"command":"card","id":41,"properties":{"flavor":"Most of the subject's body is converted to cybernetics, increasing strength and resilience substantially.","name":"Full-body Cybernetics Upgrade","MAX_HEALTH":3,"ATTACK":3,"HEALTH":3,"SCRAP_COST":5},"command":"card","zone":0},"42":{"command":"card","id":42,"properties":{"MANA_COST":8,"name":"E.M.P.","effect":"Deal 2 damage to targets\n"},"command":"card","zone":0}},"chosen":{},"max":{"18":0,"19":0,"20":0,"21":0},"minSize":30,"maxSize":30,"maxPerCard":3}},"gameId":2,"modName":"Cyborg-Chronicles"}

Tested on ed3a80b (current develop).

This related to #347, I ran into it when trying to fix that issue. Closing the socket would raise socket IO exceptions because the server immediately after sending an error back would start to process any other messages received from that client.

The server should keep track of which clients are authenticated and limit most commands to only those clients. The only command that should be run by an unauthenticated client that I know of is the login command.

@acostarelli
Copy link

Could you provide the server log, too? I think this could be helpful because it would show how the server is treating this client, like what ID's and indexes are being given to it.

@jacwah
Copy link
Contributor Author

jacwah commented Aug 19, 2015

I reproduced the same thing as above, here is the relevant portion of the server log.

[2015-08-19 22:38:12,503]  INFO MainServer [      main] (       MainServer.java: 93) - Started
[2015-08-19 22:38:29,388]  INFO ServerSock [  Thread-0] (       ServerSock.java: 41) - Incoming connection from /127.0.0.1:44278
[2015-08-19 22:38:29,388]  INFO     Server [  Thread-0] (           Server.java:186) - New client: 6:  @ /127.0.0.1:44278
[2015-08-19 22:38:29,392]  INFO ServerSock [  Thread-0] (       ServerSock.java: 38) - Waiting for client nr: 2...
[2015-08-19 22:38:29,392]  INFO ClientSocketHandler [    Conn-0] (ClientSocketHandler.java: 66) - Listening for messages using com.cardshifter.server.clients.JsonSerialization@779df34f
[2015-08-19 22:38:43,371]  INFO ClientSocketHandler [    Conn-0] (ClientSocketHandler.java: 89) - Received from 6:  @ /127.0.0.1:44278: ServerQueryMessage: Request USERS message: 
[2015-08-19 22:38:43,371]  INFO Log4jAdapter [    Conn-0] (     Log4jAdapter.java: 16) - Send to : UserStatusMessage [userId=2, status=ONLINE, name=AI Loser]
[2015-08-19 22:38:43,397]  INFO Log4jAdapter [    Conn-0] (     Log4jAdapter.java: 16) - Send to : UserStatusMessage [userId=3, status=ONLINE, name=AI Idiot]
[2015-08-19 22:38:43,398]  INFO Log4jAdapter [    Conn-0] (     Log4jAdapter.java: 16) - Send to : UserStatusMessage [userId=4, status=ONLINE, name=AI Medium]
[2015-08-19 22:38:43,398]  INFO Log4jAdapter [    Conn-0] (     Log4jAdapter.java: 16) - Send to : UserStatusMessage [userId=5, status=ONLINE, name=AI Fighter]
[2015-08-19 22:38:43,398]  INFO Log4jAdapter [    Conn-0] (     Log4jAdapter.java: 16) - Send to : UserStatusMessage [userId=6, status=ONLINE, name=]
[2015-08-19 22:38:59,437]  INFO ClientSocketHandler [    Conn-0] (ClientSocketHandler.java: 89) - Received from 6:  @ /127.0.0.1:44278: com.cardshifter.api.incoming.StartGameRequest@45c7f98b
[2015-08-19 22:38:59,697]  INFO Log4jAdapter [    Conn-0] (     Log4jAdapter.java: 16) - Send to : com.cardshifter.api.outgoing.WaitMessage@9804811
[2015-08-19 22:38:59,698]  INFO Log4jAdapter [    Conn-0] (     Log4jAdapter.java: 16) - Send to AI Loser: com.cardshifter.api.both.InviteRequest@6f60737e
[2015-08-19 22:38:59,698]  INFO GameInvite [    Conn-0] (       GameInvite.java: 58) - com.cardshifter.server.model.GameInvite@3c94d01c Invite Accept: 2: AI Loser @ FakeAI: com.cardshifter.ai.ScoringAI@6895a785 contains? true
[2015-08-19 22:38:59,698]  INFO GameInvite [    Conn-0] (       GameInvite.java: 79) - com.cardshifter.server.model.GameInvite@3c94d01c Game Start! [6:  @ /127.0.0.1:44278, 2: AI Loser @ FakeAI: com.cardshifter.ai.ScoringAI@6895a785]
[2015-08-19 22:38:59,699]  INFO Log4jAdapter [    Conn-0] (     Log4jAdapter.java: 16) - Send to AI Loser: com.cardshifter.api.outgoing.NewGameMessage@3a32d638
[2015-08-19 22:38:59,699]  INFO Log4jAdapter [    Conn-0] (     Log4jAdapter.java: 16) - Send to : com.cardshifter.api.outgoing.NewGameMessage@203ce552
[2015-08-19 22:38:59,724]  INFO    ECSGame [    Conn-0] (          ECSGame.java:104) - Add system: com.cardshifter.core.replays.ReplayRecordSystem@24a34489
Included creatures
Included enchantment
Rules
Added rule closure to list
Included scrap
Included noAttack
Included spells
Config Game$_run_closure3@1bb41db5
Rules
Added rule closure to list
Known resources is [ATTACK:ATTACK, HEALTH:HEALTH, MAX_HEALTH:MAX_HEALTH, MANA_COST:MANA_COST, ATTACK_AVAILABLE:ATTACK_AVAILABLE, DENY_COUNTERATTACK:DENY_COUNTERATTACK, SICKNESS:SICKNESS, TAUNT:TAUNT, TRAMPLE:TRAMPLE, MANA:MANA, MANA_MAX:MANA_MAX, SCRAP:SCRAP, SCRAP_COST:SCRAP_COST]
Game Closure!
Include cardset mechs: Included 19 cards
Include cardset bios: Included 11 cards
Include cardset enchantments: Included 9 cards
Include cardset spellcards: Included 1 cards
[2015-08-19 22:39:02,774]  INFO    TCGGame [    Conn-0] (          TCGGame.java:403) - AI is configured for Entity #43
[2015-08-19 22:39:02,775]  INFO Log4jAdapter [    Conn-0] (     Log4jAdapter.java: 16) - Send to AI Loser: PlayerConfigMessage{configs={Deck=DeckConfig [chosen={}]}, gameId=1, modName='Cyborg-Chronicles'}
[2015-08-19 22:39:02,776]  INFO Log4jAdapter [    Conn-0] (     Log4jAdapter.java: 16) - Send to : PlayerConfigMessage{configs={Deck=DeckConfig [chosen={}]}, gameId=1, modName='Cyborg-Chronicles'}
[2015-08-19 22:39:02,790]  INFO   ChatArea [    Conn-0] (         ChatArea.java: 31) - ChatArea:1Main broadcast: ChatMessage [chatId=1, message=AI Loser and  are now playing game 1, from=Server]
[2015-08-19 22:39:02,791]  INFO Log4jAdapter [    Conn-0] (     Log4jAdapter.java: 16) - Send to AI Loser: ChatMessage [chatId=1, message=AI Loser and  are now playing game 1, from=Server]
[2015-08-19 22:39:02,791]  INFO Log4jAdapter [    Conn-0] (     Log4jAdapter.java: 16) - Send to AI Medium: ChatMessage [chatId=1, message=AI Loser and  are now playing game 1, from=Server]
[2015-08-19 22:39:02,791]  INFO Log4jAdapter [    Conn-0] (     Log4jAdapter.java: 16) - Send to AI Fighter: ChatMessage [chatId=1, message=AI Loser and  are now playing game 1, from=Server]
[2015-08-19 22:39:02,791]  INFO Log4jAdapter [    Conn-0] (     Log4jAdapter.java: 16) - Send to AI Idiot: ChatMessage [chatId=1, message=AI Loser and  are now playing game 1, from=Server]

The client is assigned a new ID and treated as any other client, albeit with an empty name field.

@Zomis
Copy link
Member

Zomis commented Aug 19, 2015

This can indeed be a problem. It should definitely be fixed. There might be some message types in the future that may be allowed to be sent without logging in, but most should be forbidden. (I think the only message type that may be allowed is ServerQueryMessage)

@Zomis
Copy link
Member

Zomis commented Aug 19, 2015

After some discussion and Rubberducking in chat:

I would prefer using two separate methods: addHandler and addUnauthorizedHandler (in lack of a better name at the moment)

http://chat.stackexchange.com/transcript/message/23538713#23538713

@Zomis Zomis added this to the 0.6.3 milestone Aug 21, 2015
@jacwah jacwah closed this as completed Aug 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants