SpongePowered / Ore

Repository software for Sponge plugins and Forge mods
https://ore.spongepowered.org/
MIT License
79 stars 25 forks source link

Various security requirements #151

Closed Zidane closed 8 years ago

Zidane commented 8 years ago

After discussing in IRC, I want the following implemented. Priority wise, consider this a blocker from rolling out Ore:

Requirements:

Optional:

No one is exempt from this (even Sponge staff, from Leaders to Contributors). Feel free to discuss further below.

DDoS commented 8 years ago

I also suggest having a minimum 3 day delay between key replacement and being allowed to submit files again. This gives more time for a victim to notice an attack. This still includes a check from the staff.

Also if the key is removed all of the files signed with it need to be removed too.

Zidane commented 8 years ago

Updated two-factor Authentication to be more forgiving. You are now given three attempts.

DenWav commented 8 years ago

I wonder if a badge on someone's profile if they use 2FA would be a decent incentive for people use use it (people love badges). This would also give users more peace of mind for developers of large plugins (the most likely to be targeted by an attack) if they could see that user uses 2FA.

I don't agree with how harsh the 2FA failing is, I think it should be treated the same as a failed login. Two attempts seems a little harsh, considering if that fails you need to contact Ore staff. I think after a certain number, maybe 3 failed login attempts, the user should be locked out from logging in for several hours. This would prevent anyone from "brute-forcing" through a 2FA check, if it's implemented properly, without making the system as a whole too much of a pain to use.

DDoS commented 8 years ago

Also here's the reason behind this discussion: https://www.reddit.com/r/feedthebeast/comments/56gq12/delete_tic_254_if_you_downloaded_it/

Someone uploaded a fake TiC 2.5.4 to Curse Forge, which is a Trojan.

Zidane commented 8 years ago

Updated main post with more information.

DDoS commented 8 years ago

Another suggestion: if the key is lost, there's no point in making it invalid. Instead make it no-longer usable for uploads, but keep it as accepted.

So add a button for "archive key", which works like "remove key" excepts the key stays trusted and the files are not deleted.

progwml6 commented 8 years ago

additional suggestion: site staff/admins should have 2fa required

windy1 commented 8 years ago

For the key change thing -- methinks maybe an email notification might be sufficient instead of a requiring staff attention. This could get frustrating quickly if someone makes an error or something. Thoughts @Zidane?

windy1 commented 8 years ago

Also, did we have any specific kind of 2fa in mind? I personally favor SMS verification but if we want to go that route we'll probably have to front some cash for a service (i.e. twilio)

DenWav commented 8 years ago

SMS has it's issues, but it's probably the best option. Building a custom app for Ore just wouldn't be practical. Email should probably be an option just for people who want 2FA but don't want to use SMS.

Zidane commented 8 years ago

@windy1

One thing I want to drive home with these changes is the human element of Ore's overall processes is never removed. So while I am open to have slight automation, it is for only the mundane things. For example, Ore will never have an automated jar review process.

lukegb commented 8 years ago

I mean, getting rid of human review of plugins would be...

JARring...

I'll see myself out.

Zidane commented 8 years ago

@lukegb ....

@windy1 I don't know anything about 2FA to be honest. Could someone write up a list with links?

lukegb commented 8 years ago

@windy1 Can we just use U2F and/or OTPs? I'd like support for the former, but it's not really doable without support for the latter since not enough people have U2F keys yet. It's really not worthwhile bothering to set up an SMS stack.

windy1 commented 8 years ago

@Zidane Conceptually the idea is that you use a method that's

at least two of the following categories: knowledge (something they know); possession (something they have), and inherence (something they are).

https://en.wikipedia.org/wiki/Multi-factor_authentication

This means just having two passwords is not sufficient.

@lukegb U2F + OTP would probably work, the problem comes is when people use the same password for their email (presumably how we'd give them the OTP) which is very common.

Additionally, as far as I can see, there is no means of verifying a password against an already authenticated Discourse user (only SSO). I'm looking into this but this could mean that we need to implement a centralized authentication solution for this to be implemented properly.

lukegb commented 8 years ago

Sorry, I meant Google Authenticator OTPs, specifically TOTPs, not email. Sorry for the confusion.

windy1 commented 8 years ago

Hm yeah I think the problem with that is the assumption that everyone has a smartphone.

lol768 commented 8 years ago

SMS auth is not really ideal and NIST recommend you don't use it.

[Out of band verification] using SMS is deprecated, and will no longer be allowed in future releases of this guidance.

I would recommend going with HOTP/TOTP or U2F as Luke suggested. U2F dongles are pretty cheap and there's support in Chrome and Firefox (via an extension) for these - they wouldn't require a smartphone.

Users will be more used to TOTP, but it's more work on the server (ntp to configure etc). HOTP is fine until the user presses it repeatedly past the window size and breaks it by making it out of sync. It's fun to do but you can't login again afterwards.

lol768 commented 8 years ago

Oh and another thing, consider disallowing 1024 bit RSA keys.

windy1 commented 8 years ago

@lol768

Due to the risk that SMS messages or voice calls may be intercepted or redirected

Why does this seem so much more unlikely than an OTP being intercepted...

You also misquoted the document. It says it's being considered for removal.

DenWav commented 8 years ago

SMS was recently proved unreliable when phone providers were willingly giving out SIM cards to people who claimed they were someone else, therefore giving them access to SMS 2FA.

OTP, when done right like with Google Authenticator, can't be intercepted, as there is no communication between the server and the phone after the initial setup.

windy1 commented 8 years ago

Okay, I'm willing to go that route. However we have another blocker before we can proceed. Like I said before AFAIK it is impossible to verify a password against Discourse for an already authenticated user, and since Ore doesn't store your Discourse password, it makes the first step of 2fa a bit of a pain.

Here are some options I could come up with:

kashike commented 8 years ago

central authentication service

I personally like having SSO login with Ore - and for any future projects, SSO seems like a good choice.

windy1 commented 8 years ago

@kashike The thing is if we had a central auth solution we could also outsource Discourse's authentication to it. So the accounts could remain linked.

https://meta.discourse.org/t/official-single-sign-on-for-discourse/13045

e: oh that's what you're saying isn't it. sorry.

kashike commented 8 years ago

Yeah, that's what I'm saying @windy1. 😸

lukegb commented 8 years ago

For what it's worth, having a central auth solution has been my plan for a while for another reason: combined Ore/Discourse notifications, which would be absolutely epic.

Also because it's less hacky to reserve usernames that way, and then I don't have to have a separate system for managing server logins...

edit: Not that I actually have time to implement it.

JBYoshi commented 8 years ago

@lukegb I'd be willing to code something like that up.

lol768 commented 8 years ago

@windy1

You also misquoted the document. It says it's being considered for removal.

This was not deliberare, the document was changed multiple times from underneath me and I happened to use a secondary source which used an outdated version of the document. Apologies for that.

We're nitpicking here though, it's clear NSIT have reservations about its use and I stand by my original point that it's an inferior method to a TOTP/HOTP method.

Why does this seem so much more unlikely than an OTP being intercepted...

Apologies, I don't really understand this point. I imagine client -> server will be TLS'd anyway so the TOTP will not be recoverable that way. Assuming it was somehow intercepted (i.e. the attacker managed to get a valid certificate) we'd still have some protection due to the replay attack prevention (by storing used codes and not accepting them twice). In any case, if the attacker is intercepting connections secured with TLS, all bets are off because they can just grab the session cookie and call it a day.

windy1 commented 8 years ago

@lukegb @Zidane

Update: I started implementing a central authentication service that follows the protocol that both Discourse and Ore uses. It's still rather simple right now but has basic functionality like remember me tokens and working SSO redirects. I've tested it on Ore and it's working as expected. I have not yet tested it on Discourse but it should work in theory.

Here are some screenshots on what it looks like at the moment. Feel free to pitch any design ideas you might have.

http://imgur.com/a/AxvB4

As you can see it's pretty simple and follows the same design as Ore.

The repo: https://github.com/windy1/SpongeSSO

lukegb commented 8 years ago

@windy1 For SpongeSSO it's absolutely critical that everything is tested. Can you make sure that there are unit and functional tests for it?

Other than that, looks awesome! Can you set up Dockerfiles for it too :P

windy1 commented 8 years ago

@lukegb Yeah most definitely. I'll do my best with the Dockerfile, can't promise anything. :P

windy1 commented 8 years ago

Update: Here's some screenshots of 2FA

http://imgur.com/a/wK29n

windy1 commented 8 years ago

@lukegb The repo now has a working Dockerfile.

windy1 commented 8 years ago

Everything listed but keybase.io integration has been implemented in the security branch. I'm not really sure how using keybase.io would be helpful tbh since we need to host the public keys ourselves to verify uploaded files if I'm not mistaken.

windy1 commented 8 years ago

@SpongePowered/developers

Posted these in IRC but heres a couple demos of how the new authentication system I am proposing would work (as well as how migration to this new system would work)

Authentication from Discourse + Ore Two-factor authentication

JBYoshi commented 8 years ago

@windy1 Would users have to reconfirm their email after the Discourse reimport?

windy1 commented 8 years ago

@JBYoshi I can tweak the import script to not require that.

JBYoshi commented 8 years ago

@windy1 If you do that, could you only confirm it if the account's email is already confirmed? Just in case the migration starts after someone has created the account but hasn't confirmed the email yet. Based on my testing, this should occur when the active field is true for the user and the email token for the user (table email_tokens where the user_id field matches the user's id field) is confirmed (Discourse code).

discourse_development=# SELECT id, email, username, active FROM users;
 id |        email        |  username  | active 
----+---------------------+------------+--------
  1 | jbyoshi@example.com | jbyoshi    | t
 -1 | no_email            | system     | t
  3 | green@yoshi.com     | GreenYoshi | f
(3 rows)

discourse_development=# SELECT id, user_id, email, confirmed FROM email_tokens;
 id | user_id |        email        | confirmed 
----+---------+---------------------+-----------
  1 |      -1 | no_email            | t
  2 |       1 | jbyoshi@example.com | t
  4 |       3 | green@yoshi.com     | f
(3 rows)
windy1 commented 8 years ago

Yes that's correct. I'll add this along with adding an additional join_date in our database that will map to the Discourse created_at so that we can keep join dates accurate.

Aaron1011 commented 8 years ago

@windy1: The only thing we wanted for keybase.io integration would be to automatically try and fetch the public key for their username. For example, if I were to start adding a new GPG key on my account, Ore would automatically check https://keybase.io/Aaron1011/key.asc, and present it to me as an optio (If available).

Aaron1011 commented 8 years ago

@windy1: Would it be possible to allow uploading jars signed with Oracle's jarsigner tool? The downside to the current approach is that the signed file must be decrypted to be usable. Users will have to either download a regular, unsigned jar, or download a separate signature file.

kashike commented 8 years ago

@Aaron1011 I suggested that a while back:

2016-10-11 04:16:02     @kashike        Zidane, windy: I'm personally curious why use GPG when uploading, rather than signing the actual JARs like java supports - https://docs.oracle.com/javase/tutorial/security/toolsign/index.html
2016-10-11 04:16:04     @kashike        https://gist.github.com/matthewprenger/9b2da059b89433a01c1c
2016-10-11 04:24:13     @windy  kashike: overkill, all we need is to verify the identify of the uploader, we don't need anything like user specific policies and i think gpg is more accessible to the average user
2016-10-11 04:25:10     @windy  but i mean either would work
2016-10-11 04:25:12     @kashike        ...how is it overkill? jar signing is something tons of people do - anything on maven central REQUIRES it. Forge mods do it quite often as well - forge has support for signature verification as well
2016-10-11 04:26:13     @windy  maven central requires PGP AFAIK
2016-10-11 04:29:01     @windy  also it's much less important that the downloaded file is signed than the uploaded file
2016-10-11 04:29:11     +phroa  if anything, PGP is overkill.  jarsigner would be much simpler to integrate, I think.  built in to java, embedded - doesn't require uploading an additional .gpg/.asc
2016-10-11 04:29:17     @kashike        how so? ever heard of servers becoming compromised? :)
2016-10-11 04:29:37     @windy  "less"
2016-10-11 04:30:03     @windy  its much more likely for an account to be compromised
2016-10-11 04:30:08     +phroa  that said, I think I would also prefer PGP.  web of trust and all that, not RSA (don't need location, company/organization information)
2016-10-11 04:30:53     @windy  there's also the fact that I spent like half the day working on the pgp system so im not too inclined to switch now :P
2016-10-11 04:32:03     @windy  kashike: I mean you could potentially upload a pgp signed jar that is also java signed
2016-10-11 04:32:07     @windy  not to mention we support zips
2016-10-11 04:32:13     +phroa  additionally, people could/should/are sign their code commits and release tags with PGP
windy1 commented 8 years ago

@Aaron1011 possible yeah. Right now uploaded files have the signature verified and then are stored unencrypted so that the user doesnt need to decrypt it themselves. The downside of course is that the user need to trust that the file wasnt modified since upload. A solution would be to make the encrypted version available but not the default.

windy1 commented 8 years ago

Let me just add that the purpose of doing something like this is to verify the identity of the uploader, not to encrypt the file -- thats just a side effect of aigned pgp files as im sure youre aware.

windy1 commented 8 years ago

@Aaron1011 @Zidane Is it really necessary that we integrate with keybase.io if all we're doing is reading a raw text file when the user can literally just copy and paste it from their profile...I would like to consider this issue completed if there are no objections.

Aaron1011 commented 8 years ago

@windy1: It was just a convenience thing, really. We can always add it in later if we feel like it.

progwml6 commented 8 years ago

can we move the keybase stuff to a separate issue?