WLCG-AuthZ-WG / bearer-token-discovery

0 stars 2 forks source link

Add instructions for protection against vulnerabilities #3

Open maarten-litmaath opened 4 years ago

maarten-litmaath commented 4 years ago

Hi, I think we need implementations to take steps to protect users and others against accidental or deliberate abuses via world-writable directories, to the extent possible. If someone else e.g. creates /tmp/bt_u$MY_UID, then the code must not consider that file, but rather fail with a warning...

DrDaveD commented 4 years ago

It's easy to understand why one needs to make sure that writing doesn't follow symlinks created by someone else, but what is the vulnerability if someone creates a token for someone else? What kind of harm could be done, for example?

maarten-litmaath commented 4 years ago

Hi Dave, at the very least we should try to avoid having to debug perplexities like: "how can John Doe's token end up being used for my request?!" Better fail much earlier in the chain. I do not think there is a valid use case for someone preparing a token for another (in such a way). I suspect that in principle worse abuses would be possible, but I do not have something realistic beyond a DoS yet.

DrDaveD commented 4 years ago

In that case maybe the MUSTs should be changed to SHOULDs.

msalle commented 4 years ago

Note that if the file isn't owned by the user it typically also means it needs to be group or even world-readable. I think something like that would only be acceptable on servers where users don't have access. For any shared system (UIs, WNs), I think a MUST is the correct one. Note that we have the same for proxy files: they need to be readable only for the owner. As an example of an attack scenario: if a user would upload private data with an attackers credentials, the attacker then has access to that private data. In short I think a proper ownership check + file permissions (incl. the standard paths checks to prevent symlink attacks) is necessary on any shared system.

DrDaveD commented 4 years ago

Well then if we're getting into requiring security checks in every token reader, it ought to be complete and require that the file is only accessible by the current user.