BeamMP / BeamMP-Server

Server for the multiplayer mod BeamMP for BeamNG.drive
https://beammp.com
GNU Affero General Public License v3.0
132 stars 54 forks source link

[Feature Request] Password Protection #163

Closed Starystars67 closed 11 months ago

Starystars67 commented 1 year ago

Is your feature request related to a problem? Please describe. I would like to request password protection for servers. This should be something that occurs before mod downloading to protect the resources on that server. This would prevent the leaking of private resources or enforce limited access.

Describe the solution you'd like Please implement password protection into the server config. Something like server_password would be good. This check needs to happen before the client downloads any mods though.

Describe alternatives you've considered We could implement password protection using the Lua interface but there is presently no way to have this occur before the server mods have been downloaded that I am aware of.

Additional context I have had people come to me in the past requesting this feature but then alternative means were used. I am now getting this request again so I feel it is time to document this and get things moving.

lionkor commented 1 year ago

This could happen as the second step of auth, after player authentication - we need to come up with a packet code for this, maybe as part of writing a spec.

Should this be possible only on private, or also public servers?

20dka commented 1 year ago

Should definitely be written down as part of a spec. I think it should be an option on public servers too, but the serverlist has to reflect it, and should have a filter for password protected servers

OfficialLambdax commented 1 year ago

And with that come changes to the UI. A extra symbol that indicates that the server is pw locked and a enter password here ui. What then also should be considered is if locked servers should shows their mods/players. The playercount is likely ok, but who plays on it or whats hosted on it might not - in the perspective of the hoster.

lionkor commented 1 year ago

Some points about this:

lionkor commented 1 year ago
Anonymous-275 commented 1 year ago

to answer the points since more info is needed

O1LER commented 1 year ago

With a whitelist you have the full control over who can and can not connect to a server.

A password can be sent to others and the server owner doesnt even know who leaked it.

Anonymous-275 commented 1 year ago

you cannot whitelist your entire community easily

Starystars67 commented 1 year ago

True however from the POV of running a server for a small group of friends a password is easier than having to stop what I am doing to whitelist people.

Could we not offer both maybe?

Starystars67 commented 1 year ago

I think if we are going to putting in allowlist then we should also look to add a blocklist.

lionkor commented 1 year ago

doesn't need to be, can be a future thing, trying to minimize the work needed

To me that's a hard requirement - either make it private server only, or heartbeat it and put it on the serverlist

lionkor commented 1 year ago

why does it need to be in plaintext? We have the openssl library that can sha256 the password

because then we're fooling ourselves into thinking we have security, which we don't - sha256 without a salt is not secure in any way

Anonymous-275 commented 1 year ago

sure then just heartbeat a boolean of pass

Starystars67 commented 1 year ago

I would like to see this detail shown in the server list too.

lionkor commented 1 year ago

True however from the POV of running a server for a small group of friends a password is easier than having to stop what I am doing to whitelist people.

Could we not offer both maybe?

The difference is that a whitelist is easy to do as a server resource, whereas a password requires changes in almost all components of beammp, that's why I asked. I'm ok with having a password option

Anonymous-275 commented 1 year ago

why does it need to be in plaintext? We have the openssl library that can sha256 the password

because then we're fooling ourselves into thinking we have security, which we don't - sha256 without a salt is not secure in any way

I mean yea the point is control the players joining to your server not secure your private cat photos on google

Starystars67 commented 1 year ago

The difference is that a whitelist is easy to do as a server resource, whereas a password requires changes in almost all components of beammp, that's why I asked. I'm ok with having a password option.

The issue I found when doing this was that there was no way to get the players ID's before they download the mods. This means that private mods on a server would still be shared.

lionkor commented 1 year ago

Yeah, exactly, I just wanted to make sure we agree it can be plaintext, because its not particularly sensitive.

I'm trying to make sure this is done right, as all features, and not "quick and trying to minimize the work needed", basically. That's why I'm being so pedantic.

Anonymous-275 commented 1 year ago

Yeah, exactly, I just wanted to make sure we agree it can be plaintext, because its not particularly sensitive.

I'm trying to make sure this is done right, as all features, and not "quick and trying to minimize the work needed", basically. That's why I'm being so pedantic.

sure but a hash is better since that is a one way transformation me knowing the hash is better than me knowing the password

lionkor commented 1 year ago

The difference is that a whitelist is easy to do as a server resource, whereas a password requires changes in almost all components of beammp, that's why I asked. I'm ok with having a password option.

The issue I found when doing this was that there was no way to get the players ID's before they download the mods. This means that private mods on a server would still be shared.

The player ID is available on authentication event "onPlayerAuth" (all identifiers are), as of v3.1.0 (see the changelog), so that's not an issue.

lionkor commented 1 year ago

Yeah, exactly, I just wanted to make sure we agree it can be plaintext, because its not particularly sensitive. I'm trying to make sure this is done right, as all features, and not "quick and trying to minimize the work needed", basically. That's why I'm being so pedantic.

sure but a hash is better since that is a one way transformation me knowing the hash is better than me knowing the password

I'm against this because it's not like that, as there are dictionary attacks (rainbow LUT attacks) for most passwords people will use in this kind of context (short and/or common words)

lionkor commented 1 year ago

I'm saying at that point the hash may as well not exist. We can just make a TLS connection for authentication, but that's more work.

Anonymous-275 commented 1 year ago

Yeah, exactly, I just wanted to make sure we agree it can be plaintext, because its not particularly sensitive. I'm trying to make sure this is done right, as all features, and not "quick and trying to minimize the work needed", basically. That's why I'm being so pedantic.

sure but a hash is better since that is a one way transformation me knowing the hash is better than me knowing the password

I'm against this because it's not like that, as there are dictionary attacks (rainbow LUT attacks) for most passwords people will use in this kind of context (short and/or common words)

great then for an attacker it's more work than the plain password, also good luck finding a 30 character complex custom password on a rainbow list

1eob commented 1 year ago

True however from the POV of running a server for a small group of friends a password is easier than having to stop what I am doing to whitelist people. Could we not offer both maybe?

The difference is that a whitelist is easy to do as a server resource, whereas a password requires changes in almost all components of beammp, that's why I asked. I'm ok with having a password option

It would be nice to have basic moderation abilities on the base server rather then having to download/find a plugin to have that

lionkor commented 1 year ago

30 complex custom password

I think we can just do plaintext, i doubt people use this kind of password for a server, especially in beamng where copy+paste is horribly broken (at least for me)

Anonymous-275 commented 1 year ago

30 complex custom password

I think we can just do plaintext, i doubt people use this kind of password for a server, especially in beamng where copy+paste is horribly broken (at least for me)

why not slap in 3 lines of code to hash it anyway?

OfficialLambdax commented 1 year ago

Yeah, exactly, I just wanted to make sure we agree it can be plaintext, because its not particularly sensitive. I'm trying to make sure this is done right, as all features, and not "quick and trying to minimize the work needed", basically. That's why I'm being so pedantic.

sure but a hash is better since that is a one way transformation me knowing the hash is better than me knowing the password

As in sending the hash to the server? i mean as a attacker i will just grab and send the hash myself. I was thinking of having the server create a random string, aes encrypt it with the actual key, send the encrypted string to the client, have it decryt it and send the decrypted text back to the server, so if both strings match then go forward. But then i thought, those who can sniff can also perform mitm. So if we would wanna go full blown on this then integrate tls, but i think this is to much hassle for something so simple as beammp.

Anonymous-275 commented 1 year ago

yeah ofc you can just send the hash but a hash leak is tamer than a password leak IMO + minimal work required

OfficialLambdax commented 1 year ago

i mean the hash becomes the password then. that will just protect the password but not prevent login

Starystars67 commented 1 year ago

True however from the POV of running a server for a small group of friends a password is easier than having to stop what I am doing to whitelist people. Could we not offer both maybe?

The difference is that a whitelist is easy to do as a server resource, whereas a password requires changes in almost all components of beammp, that's why I asked. I'm ok with having a password option

It would be nice to have basic moderation abilities on the base server rather then having to download/find a plugin to have that

Agreed and with v3.1.0 Allow/Block list are now possible through the Lua scripting system.

So that would just leave the password option. From what I know it is not possible to do this using the Lua interface of the server alone. As such we will need to do work to other aspects of the project (Heartbeat, Lua+UI, Client).

With regards to the Hashing/No Hashing I am not sure I see a reason or real need for this. The only reason this can be of concern would be for MITM. This is a games server not a dedicated messaging platform. There is no confidential data being sent here (or shouldn't, who knows what people choose to send).

Anonymous-275 commented 1 year ago

i mean the hash becomes the password then. that will just protect the password but not prevent login

exactly my point Literally I am just protecting the password

lionkor commented 1 year ago

As long as you put a documentation comment that clarifies that it doesnt add any security, yes. As I said, that, or actual security, the in-between sounds a lot like something we have to deprecatre later.

I'm going to call this discussion about the hashing here, as it's not really the point.

I think all points have been made about the encryption/obfuscation/etc. so we can leave it at that.

lionkor commented 1 year ago

About the heartbeat, I think a bool is good for that as you said, I agree.

Anonymous-275 commented 1 year ago

As long as you put a documentation comment that clarifies that it doesnt add any security, yes. As I said, that, or actual security, the in-between sounds a lot like something we have to deprecatre later.

I'm going to call this discussion about the hashing here, as it's not really the point.

  • send it as plaintext, or
  • hash it and add a big comment explaining that it makes no difference for common passwords, short passwords, or passwords that consist of common words (im not going to let us fool ourselves into thinking its secure in any way), or
  • make the connection use TLS

I think all points have been made about the encryption/obfuscation/etc. so we can leave it at that.

sure we can add a text in all caps about that when the server is ran with a password

lionkor commented 1 year ago

I think I was clear about the options here, let's leave it at that

Anonymous-275 commented 1 year ago

I think I was clear about the options here, let's leave it at that

sorry if my reply was not clear, yes yes hash it and add a big comment explaining that it makes no difference for common passwords, short passwords, or passwords that consist of common words.

OfficialLambdax commented 1 year ago

id say we go the hased pw way and only change that system if its at some point required and then only to tls as thats the ultima ratio, not that i think it will ever be required

lionkor commented 1 year ago

As for when, I'd say it can be a packet right before the P packet, so before the launcher's SR.

Anonymous-275 commented 1 year ago

yeah right before SR

lionkor commented 1 year ago

What code do we use?

Anonymous-275 commented 1 year ago

or we can actually use SR's response for this

Anonymous-275 commented 1 year ago

What code do we use?

what do you mean?

lionkor commented 1 year ago

SR happens after P, so that doesn't work, it needs to be before P

lionkor commented 1 year ago

We don't want to assign a pid before the password is right

lionkor commented 1 year ago

What code do we use?

what do you mean?

Which character, like, which byte are we thinking for the password challenge packet

Anonymous-275 commented 1 year ago

SR happens after P, so that doesn't work, it needs to be before P

we can have it that it returns P if there is no password or the reply there would be the password flag

lionkor commented 1 year ago

Yeah, it should be

  1. server challenges client for a password
  2. client sends password
  3. server either closes connection with the "invalid password" message, or sends P with the pid
Anonymous-275 commented 1 year ago

we could use S for Security

Anonymous-275 commented 1 year ago

Yeah, it should be

  1. server challenges client for a password
  2. client sends password
  3. server either closes connection with the "invalid password" message, or sends P with the pid

yeah these are the steps obviously yes

lionkor commented 1 year ago

An effort needs to be made to specify exactly what we're doing, otherwise the PR review gets ugly, we don't want that. I'd rather clear everything up here, first, no matter how obvious it is