missing basic auth for basicui results in user errors & secvuln: information leakage

[rdslw]

• [X] Basic UI

There is a dual problem related to basic auth implementation within basic ui:

• low security vuln: information leakage
• missing http Authorization header in basicui results inability to use basic auth for basic ui

Detailed description:

1. Even if 'Implicit User Role' is disabled, basicui is able to retrieve and show to unathenticated user all relevant items, state etc. It's not possible (in such case) to write to items, but reading is possible. This is contrary to what documentation states, and contrary to user expecting what it shall do upon blocking it.

2. if user disables option 'Implicit User Role' and enables option 'Allow Basic Auth', after going into http://openhab/basicui/app there is no Authorization header served by openhab, resulting in inability to use basicui to steer. Just showing this header would solve this problem.

ad 3. combination of 1&2 results in many people misunderstanding how authentication works (while in fact it does not) for basicui, resulting in numerous answers on community.openhab.org that 'enabling both implicit user role and basic auth' is important and proper for basicui authenticated usage. This is not the case. Just implicit user role enabled is enough and gives ANYBODY (basic auth having no effect) to full access to basicui.

Expected behavior/fix proposed

a. If option 'basicauth' is enabled, all urls http://openhab/basicui/ should present http Authorization header and use this information for authentication b. if option 'Implicit user role' is disabled, basicui shall NOT have ability to read and present items state

Your environment

openhabian 1.7 openhab 3.3 tested on chrome/firefox@linux and safari@ios

[lolodomo]

BasicUI is very old {OH2 - maybe even OH1) and never implemented authentication.

Authentication was added later in OH3 when MainUI became the main UI (and supported authentication) .

Just using default authentication settings is OK to use BasicUI.

If you or someone else wants to add support for authentication in BasicUI, he is welcome.

[rdslw]

Thank you kindly for the insight about OH1/OH2/OH3 history.

I agree that full authentication support would be an enhancement. Also needing more work, not sure if reasonable given MainUI introduction.

That being said, as of now case 1 described above is a result of bug in current code, and is a security vulnerability, allowing basicui to circumvent restapi security, and read values EVEN if implicit role is disabled. This scenario is currently implemented while shall stop fully basicui, which is not the case, as now it only stops write, but not reads.

May I suggest, change this issue to the bug itself, as it's shorter to fix, and actually important, and does not need addingauthentication support to basicui.

[florian-h05]

@rdslw I‘ve just had a look at this on my system, I can reproduce the behaviour that BasicUI displays all Sitemaps but is unable to control anything if „Implicit User Role“ is disabled.

Regarding a: I think the current implementation is not completely wrong, because the option is „Allow Basic Auth in addition to API tokens for authentication“. You are talking about a „Require Basic Auth“ option. FYI: If you disable the „Implicit User Role“, MainUI asks for authentication. BasicUI should do the same.

Regarding b: This is a bug in core, because the REST API allows requests to the /rest/sitemaps endpoint.

[florian-h05]

Regarding b: This is a bug in core, because the REST API allows requests to the /rest/sitemaps endpoint.

I've just checked, core works as expected. The problem is BasicUI that does not perform any security checks. But I can't contribute because I have no experience with BasicUI development.

[rdslw]

I will try to respond to it with more informaiton tomorrow. Thank you @florian-h05 for your checks and help here!

[rdslw]

Upon additional checks some more information.

With 'Implicit User Role' disabled, basing UI is able to both show all sitemaps, AND is able to show (read access) all items state, so this is not purely 'sitemap access', but all items read unauth access. This shall not be the case. There is no write access though.

My thinking here is that we're looking at different strategies to fix this bug:

• make basicui to always use restapi to access items, not some internal api. That (basicui uses restapi always) was my thinking (confirmed on forums byopenhab member). This bug shows that this is not true, and basicui uses some under-the-hood access method, EITHER restapi has some security bug (allowing access even with option disabled)
• make basicui respect (actually USE, because it is not using http basic auth now) 'Allow Basic Authentication' setting. This is prefered IMO (although slightly more work needed). BasicUI shall emit http Authorization header and use this information for authentication in case this option is enabled.

So summarizing, what this bug is asking for: a. If option 'basicauth' is enabled, all urls http://openhab/basicui/ should present http Authorization header and use this information for authentication b. if option 'Implicit user role' is disabled, basicui shall NOT have ability to read and present items state

NB: Current situation (with this bug) is: if someone is using basicui (a lot of people do), it forces them to enable 'Implicit User Role' which is having big security implications, because this options means: all access R&W to anybody to restapi (sic!), while ignoring 'Allow Basic Authentication' because it's not needed.

[lolodomo]

BasicUI uses the sitemap REST API + a SSE connection to the server to get immediate updates.

[rdslw]

AFAIR in my instance SSE is not working (basicUI is complaining about it), hence meaning that even with 'Implicit user role' option being disabled, REST API gives BasicUI access to sitemaps (and readonly for all items exported through sitemaps , probably through sitemaps, but I'm unaware how it works, so there more be problems inside)

[lolodomo]

I see allowed roles set on the global REST class: https://github.com/openhab/openhab-core/blob/8d64ecfd8db99844d79a776fef37cbe527c1f0c6/bundles/org.openhab.core.io.rest.sitemap/src/main/java/org/openhab/core/io/rest/sitemap/internal/SitemapResource.java#L133

but not on all individual REST calls in this class, while it looks that it is set in other REST classes. I am not expert at all with these annotations. Maybe @ghys can advice in case something is missing in class SitemapResource ?

[lolodomo]

I could make a try to add @RolesAllowed({ Role.USER }) on each REST API call (sitemap REST API). @ghys : what is the expected result if Role.USER is not there ?

[lolodomo]

By the way, the problem could be rather (or also) the WEB app servlet which was never updated to support security/role stuff ? https://github.com/openhab/openhab-webui/blob/main/bundles/org.openhab.ui.basic/src/main/java/org/openhab/ui/basic/internal/servlet/WebAppServlet.java#L67

[lolodomo]

I could probably simply update the WEB app servlet to consider the "Implicit User Role" setting. If disabled, I will return a specific page telling that BasicUI cannot work if "Implicit User Role" is disabled.

The 3 existing servlets should be considered, that is the manifest servlet and the CMD servlet in addition to the main servlet.

[ghys]

Basic UI renders sitemaps server-side using a servlet, and also uses the REST API to send commands, and subscribe to SSE streams to monitor for updates. The "API Security" settings only refer to the REST API, in theory. They're not supposed to influence servlet-based UIs.

However, there could be 2 approaches for enforcing authorization in Basic UI:

1) Use the Main UI sessions and authorization flow: a. The servlet should look for a X-OPENHAB-SESSIONID cookie and verify whether a session with this ID can be found, if not, redirect with a 301 redirect so the user can login using the logic implemented in the Main UI. b. The client JS should check for the presence of a refresh token in localStorage (key: openhab.ui:refreshToken) set by the main UI after a successful sign in, and use that to get an access token using a /rest/auth/token call. The access token should also be checked for expiration (it only lasts one hour) and renewed when necessary. All calls to the API should be made with the access token. If there was a problem retrieving the access token then we can assume there's a need to authorize again, so redirect as in 1.a)

2) open its own sessions: a. In this case the OAuth2 authorization flow should be implemented: when a refresh token wasn't found (for instance in its own dedicated key in localStorage) the JS client should begin the authorization flow like the main UI does, to get an authorization code and with that, get the first access token along with the refresh token (which is to store). If the refresh token already exists then do the same as in 1.b) b. The problem that could arise is that there is no current way of using the useCookie=true parameter during the authorization flow to set the X-OPENHAB-SESSIONID cookie as it would conflict with an existing one set for the main UI's session. Without a session cookie intercepted by the servlet, it probably can't determine whether the user is already authorized.

[florian-h05]

I could make a try to add @RolesAllowed({ Role.USER }) on each REST API call (sitemap REST API).

I tried calling /rest/sitemaps with Implicit User Role disabled and got a 401, so the REST API/core works correctly.

By the way, the problem could be rather (or also) the WEB app servlet which was never updated to support security/role stuff ?

When I checked BasicUI in the Chrome DevTools, the network tab showed me that the Sitemap was actually loaded from /basicui/app?sitemap=test, so it’s not a core REST problem but a BasicUI servlet problem.

[lolodomo]

Clearly I will not implement myself any full authorization flow in Basic UI. I have certainly not the skills to do that (and I don't have this need myself).

But if we can, in a very first step, resolve the security issue (if there is really one) and avoid displaying a sitemap when "Implicit User Role" is disabled, I am ok to try that if this is simple. Do I need to create a simple OSGi service connected to these core settings that will be referenced by any BasicUI servlet in order to retrieve the value of the "Implicit User Role" setting ?

[lolodomo]

When I checked BasicUI in the Chrome DevTools, the network tab showed me that the Sitemap was actually loaded from /basicui/app?sitemap=test, so it’s not a core REST problem but a BasicUI servlet problem.

Yes, that is a call to the main servlet of Basic UI.

[lolodomo]

We can't implement a solution that will depend on MainUI in case a user only uses BasicUI.

[lolodomo]

I have now something working, I will submit a PR later today.

[ghys]

We can't implement a solution that will depend on MainUI in case a user only uses BasicUI.

That's true but in that non-standard case we can also assume the auth feature could be removed altogether as well, so you'll basically have OH2: no main UI, no auth (you can add your own auth layer using a reverse proxy).

Note that if you don' t need the openHAB auth at all (for instance on trusted networks or if it's handled externally) you can remove it and the Main UI will still behave right - it will just give access to everything by default and remove the access to the "profile" page - but I haven't tested that scenario recently.

[lolodomo]

Rereading your first approach with more attention, it looks no so bad 😊

[lolodomo]

I come back here to summarize the discussion with @wborn in the PR. @wborn had a great idea and proposed to add a HTTP request handler that will return HTTP code 403 (forbidden) when any BasicUI servlet is called while the "implicit user role" setting is disabled.

The day authentication is implemented for a servlet, we will remove the servlet from this check.

In fact, I think this should be applied also to other servlets provided by the core framework, at least the chart servlet (used by sitemap UIs) and probably also the icon servlet (even if less critical). So the new class should be embedded in the core framework.

Work in progress.

[lolodomo]

@ghys @wborn : could you explain what is o.o.core.io.http.auth and if we are using it, in particular in MainUI? Apparently, there is a setting org.openhab.auth:enabled which is not set and this setting is not exposed to users in MainUI. It includes two request handlers that allows to check for authentication and redirect to a login URL. I am just trying to understand if this is something that could be used for Basic UI.

[ghys]

It contained IIRC an implementation of an auth system implementation by @splatch that AFAIK wasn't used, at least in the vanilla OH distribution. I reused that package to add stuff related to user authorization et al (servlets to serve the pages for authorizing, creating an API token, or changing a password).

[splatch]

AFAIR basic UI rely on servlet which can be hooked up into security (see SmartHomeServlet base class). Issue is how to return to basicui after successful authentication attempt confirmed by login endpoint. I made some work in own fork to to detect and validate session cookies which work fine as long as user session is active (almost forever ;-)).

There was discussion on this topic earlier in community forums: https://community.openhab.org/t/rbac-model-in-openhab-and-potential-security-vulnerability-found/136419

[lolodomo]

AFAIR basic UI rely on servlet which can be hooked up into security (see SmartHomeServlet base class). Issue is how to return to basicui after successful authentication attempt confirmed by login endpoint. I made some work in own fork to to detect and validate session cookies which work fine as long as user session is active (almost forever ;-)).

Ok, I will let you work on that and close my own PR. I will probably be able to review your work.