basic auth - potential security issue if wrongly configured

[violoncelloCH]

During reviewing #54 (and experimenting with it), I just remarked that a wrong configuration of OC_User_BasicAuth could lead to a potential security issue:

If an admin configures an URL to authenticate against, where no basic authentication happens at all but the server responds with a 200 response immediately on each request, everyone can login with any username/password combinations. It's even possible to login with usernames registered under other backends (e.g. a username registered as a regular Nextcloud user; you can just use anything as password and get access to this user account as well).

It could even be the case, that something changes on the side of the requested web server over time and the admin wouldn't remark it at all (that his Nextcloud is open to anyone) until he/she explicitly tests wrong username/password combinations

I wonder, if we could somehow check, if the requested web server does an authentication at all and otherwise not allow access. If not, we should at least add a warning to the documentation. What do you think @nerdmaennchen ?

[nerdmaennchen]

I do absolutely agree that there should be at least a warning. But in general there is only so much you can do to prevent admins from hurting themselves... I'm not entirely sure if adding a mechanism to check the backend for basic authentication (e.g., a negative test) would be an actual solution, because it might give a false sense of security. But maybe my pessimism is exaggerated.

However, if a coded "solution" is necessary I'd suggest adding a configuration parameter containing an invalid username/password combination and prior to any authentication this combination is used to check if the backend actually does basic auth. The following positive test would be the actual authentication with the provided credentials.

Do you think adding a negative test might be necessary?

Greetings from Guatemala, Lutz

[violoncelloCH]

I'm not entirely sure if adding a mechanism to check the backend for basic authentication (e.g., a negative test) would be an actual solution, because it might give a false sense of security.

we should probably add a warning anyway and not propagate a check (if we have one)

I'd suggest adding a configuration parameter containing an invalid username/password combination and prior to any authentication this combination is used to check if the backend actually does basic auth.

hmm, not sure if this would make sense, because it would generate a huge overhead on every authentication request (user_external is already quite slow because of it's nature of each time requesting (over a network connection) the original backend)

I rather thought about if we could somehow know from the response header if an authentication happened or not? That would not generate extra request and would therefore be much simpler and more lightweight. However I'm not familiar with HTTP Basic Auth and the response headers, that's why I'm asking you ;)

Thank you very much for your help!

[nerdmaennchen]

I'm afraid a positive response header would look the same in the case of an incorrectly configured server as in the case of a valid login. That's why I suggested doing a negative test as well.

Anyways, as usual: I'm no expert on this matter and I don't know a better solution than doing a negative test. If you think that's too expensive then we should not do that.

all the best, Lutz

[violoncelloCH]

then I'm actually not sure what to do (except the warning of course) maybe @rullzer can you give us some advice here?

[nerdmaennchen]

Sorry for my late review. From what I (now) know: You are absolutely correct. Testing for that header should prevent arbitrary user/pw combinations from logging in when authenticating agains a misconfigured URL.

Im not sure if a second request is actually necessary though. Correct me if I'm wrong, but wouldn't it be enough to test the response of the authentication request for the presence of the www-authenticate header? If so, a single request would suffice and the performance penalty (I agree with you here, I'd say it is miniscule) would be even further reduced.

Anyways, thank you very much for you contribution!

All the best, Lutz

[violoncelloCH]

@nerdmaennchen shouldn't your last comment here have gone to the PR #82 ? could you post it there?