openhab / openhab-distro

The binary distribution of openHAB
https://www.openhab.org/
Eclipse Public License 2.0
1.3k stars 393 forks source link

Introduce Basic Auth on HTTP server #791

Open kaikreuzer opened 5 years ago

kaikreuzer commented 5 years ago

With https://github.com/eclipse/smarthome/pull/6034, we now have some initial framework feature in place to do authentication and authorisation on HTTP requests.

With https://github.com/openhab/openhab-core/pull/412 and https://github.com/openhab/openhab-distro/pull/782, I enabled those in openHAB without giving it much thought, so it had to be reverted. It turned out that a few pieces are still missing and that we need to carefully plan the activation of this feature.

Here are a few open topics that come to my mind:

@splatch Would you be able to work on (some of) those?

kaikreuzer commented 5 years ago

@splatch & @marziman, as you both were driving the authentication topic and as I would really really like to see it being part of openHAB 2.4: Any suggestion how we should best proceed on it to get it done?

splatch commented 5 years ago

@kaikreuzer Thank you for reminder, I hope to pick some of above points end of this week/beginning of next one.

In terms of existing servlets I've found only one in openhab2 addons AFAIR none in openhab1 addons.

Jochen1980 commented 5 years ago

@all: Are all of the topics / ideas Kai mentioned still opened? Is anyone of you are already working on one of these?

kirantpatil commented 5 years ago

@splatch has done some work on it.

Please find it here https://github.com/eclipse/smarthome/pull/6034

Now it might be merged in to openhab-core.

Skinah commented 4 years ago

In case it helps, I have fully working BASIC auth code in my IpCamera binding and it is opensource and happy for it to be used. I have also got working DIGEST code there as well as it would be good to have both supported and not just basic.

IMHO Digest should be used unless you are using HTTPS in combination with BASIC. It is far too easy to unpack the header and steal a password with basic.

hmerk commented 3 years ago

@kaikreuzer Is this issue still considered to be open or can it be closed since @ghys has implemented authentification in the new UI ?

kaikreuzer commented 3 years ago

What has been implemented is authentication for admin resources, while the rest is still unsecured.I am pretty happy with what @ghys came up, but I what imho is still missing is the possibility to completely secure the instance and only allow authenticated requests, also for user resources. I agree though that the description of this issue here is not really applicable anymore, so I'd also be absolutely fine to close it and have the bounty awarded to @ghys.

splatch commented 3 years ago

I believe that basic authentication is still relevant for simple system to system integration which is far from completing oauth flows. Securing all rest endpoints requires either implementation of additional flows to be consumed by basic clients (direct/password or client credentials) which is probably beyond the scope of what been provided so far.

kaikreuzer commented 3 years ago

I don't really agree. Any REST APIs out there in the wild are using token (e.g. apiKey) based authentication and nobody uses basic auth - this is imho a pretty outdated concept.

splatch commented 3 years ago

API keys are still a fancy name for basic auth. The difference which you draw is just non standard header with random value instead of well known user/password. What really blocks you from using random value as an user and empty password for basic auth? Name of header where you place random md5 hash is secondary issue. Before you say that nobody uses basic auth try forcing curl to make HTTP call with tokens. Soon you will realize that you need additional tools such jq or another bash tinkering just to extract token for desired call. Keep in mind that tokens are fine for online services but might not be such a great idea for locally hosted systems as it will bring nothing but headache to people who try to integrate.

marziman commented 3 years ago

@kaikreuzer we use JWT Token based authentication to protect everything and combined it with RBAC authorization. But you know we did many things differently when it comes to the ESH/OH, as it is based on cloud IdP. This is maybe not the ideal usecase if you want to be completely cloudless. For us the basic auth didn't bring value (as it kept other parts open), thats why in real production we went Token based.

@Jochen1980 we have many of these concepts implemented back than and in production.

ghys commented 3 years ago

I agree with the fact that ultimately, the entire API should end up being put behind authorization; however as it stands now that would mean breaking many things, and the added security isn't worth it - most instances are merely unsecured local network HTTP anyway; IPV6 being widely adopted by ISPs does introduce a change i.e. network devices that used to be put behind NAT are now potentially fully routable so this openHAB server that used to be fine because it wasn't "reachable from the internet" now is - maybe even without the user knowing about it.

Tthe focus was put on the admin operations as a first step - it was a low-hanging fruit and yes, it's not enough, you can do many nasty things by merely sending commands to items but it's a start! At least editing things, installing add-ons and the like are now made somewhat harder, or actually a lot harder if you care about putting HTTPS in place even on your local network.

wborn commented 3 years ago

Keep in mind that tokens are fine for online services but might not be such a great idea for locally hosted systems as it will bring nothing but headache to people who try to integrate.

See: https://github.com/openhab/openhab-core/issues/1699

I'm all for adding multiple authentication options including none. But it comes ofcourse with the price that someone has to make and maintain it. The more options available, the more work has to be done to support them all and if there is no standard this will cause lots of issues, documentation and community posts. :neutral_face:

bwosborne2 commented 3 years ago

permitting basic auth at this point would weaken the security of OAuth2 already introduces in OH3.

The current flaw, IMO is that the authorization (what you are permitted( is currently accessible from the RESTAPI but not authentication (username password, who you are).

spacemanspiff2007 commented 3 years ago

Isn't the whole point of OAuth2 to separate the instance granting authorization, the authorization server and the resource server? Since everything is not only running on the same machine but even in the same program the benefit of using OAuth2 is still unclear to me.

@bwosborne2 : Can you elaborate how OAuth2 is more secure than basic auth in this context?

bwosborne2 commented 3 years ago

Basic Auth is just username & password. It needs can easily impact security of an API, especially if access is not easily revokable.

https://nordicapis.com/3-common-methods-api-authentication-explained/

spacemanspiff2007 commented 3 years ago

An in the case of openhab OAuth2 is just a really long username without a password. There is no security benefit using it.

If I may quote the "in depth" article you linked (in about the middle).

grafik

As I have been trying to point out - there is no 3rd instance which does grant access so there is no point in using OAuth. I know token is a fancy word and sounds better than username but in the case of openhab it is just the same!

So we get all the complexity and no benefits.

kaikreuzer commented 3 years ago

An in the case of openhab OAuth2 is just a really long username without a password.

That's why I suggested to go for api keys in https://github.com/openhab/openhab-core/issues/1699#issuecomment-705793108 for local system-to-system integrations. For systems, user names do not make sense. All they need is an access key (=token). Unless we want systems to allow to authenticate on behalf of (different) users, but that's another complexity level on top (and OAuth2 would make sense for that again).

bwosborne2 commented 3 years ago

For LOCAL, I agree, but the REST API is also accessible externally from the OH server. I think that is why OAuth2 was chosen. Perhaps a different grant type needs to be added for local system to system use.

spacemanspiff2007 commented 3 years ago

@kaikreuzer : If you want to put the token in the header, then it's the equivalent of using basic auth because it just does that. If you want to pass it in the url it is a different concept.

Basic Auth:

Header example: Authorization: Basic dXNlcm5hbWU6cGFzc3dvcmQ=\n

Token in url:

Url example: http://openhab_instance:port/rest/items&token=asfdgklrtjhqwehqwenfdvbajklfwerwekl

@bwosborne2 : I still don't understand to which entity you want to delegate to with OAuth. There is none, because you do authentication and authorization in openhab.

J-N-K commented 3 years ago

I fully agree with both comments from @spacemanspiff2007 above.

bwosborne2 commented 3 years ago

"I still don't understand to which entity you want to delegate to with OAuth. There is none, because you do authentication and authorization in openhab." openHAB should provide a way for the REST API to delegate access ( in this case, the Admin access) to an app outside of openHAB, such as HABApp. Whether the app is on the local server or on a different server should not matter.

OpenHAB has chosen to use a username/password combo that does not appear to have been integrated into the API. Unless it had changed for OH3, the UIs only accessed OH through the API. If the new UI does that for authentication, it is hidden & not well documented.

spacemanspiff2007 commented 3 years ago

openHAB should provide a way for the REST API to delegate access ( in this case, the Admin access) to an app outside of openHAB, such as HABApp. Whether the app is on the local server or on a different server should not matter.

But what you are talking about is authorization and not delegation. Delegation would be "Facebook has said it's okay that @bwosborne2 uses the openhab rest api and here is a token to prove it".