Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable http/2 for https & Set Keep-Alive header for http #4466

Open
florian-h05 opened this issue Dec 1, 2024 · 30 comments
Open

Enable http/2 for https & Set Keep-Alive header for http #4466

florian-h05 opened this issue Dec 1, 2024 · 30 comments
Labels
enhancement An enhancement or new feature of the Core

Comments

@florian-h05
Copy link
Contributor

When having multiple tabs of Main UI opened, the UI often fails to send HTTP requests to the server because the browser does not execute the requests.
This is because we are running into the max parallel HTTP connections limit.

To fix this issue, we need to keep alive HTTP connections for http and/or enable http/2 for https.
Enabling http/2 for https also brings other benefits, and http/2 is supported by all major browser for several years now (see https://caniuse.com/http2).
This fix seems to work as I have never experienced the above problems with my production instance running behing nginx where I use http/2 and https.

See https://community.openhab.org/t/openhab-4-3-milestone-discussion/158139/75?u=florian-h05, https://www.geeksforgeeks.org/what-are-max-parallel-http-connections-in-a-browser/ and https://www.baeldung.com/jetty-http-2 for more details.

Unfortunately I do not really know how Jetty dependencies are added to openHAB and I am not really confident in configuring Jetty through the XML in openhab-distro, so I need some help.

@J-N-K Since you already added WebSocket and Gzip to Jetty, may I ask for your help here?

@florian-h05 florian-h05 added the enhancement An enhancement or new feature of the Core label Dec 1, 2024
@openhab-bot
Copy link
Collaborator

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-4-3-milestone-discussion/158139/75

@wborn
Copy link
Member

wborn commented Dec 1, 2024

I did some testing with openHAB and HTTP/2 a while ago but it will only work with TLS so you also need a valid (self) signed certificate. I also got h2c (http/2 without TLS) working, however it's not supported by browsers 🙃 .

@florian-h05
Copy link
Contributor Author

I am aware of the fact that HTTP/2 needs TLS, openHAB already ships with a valid self-signed certificate. Browsers are putting more and more restrictions on unencrypted connections, take microphone access on the SIP client widget as another example.

The described issue can be easily fixed for HTTPS by enabling HTTP/2, for HTTP only it should be fixable or at least improveable by setting the Keep-Alive header.
As you already had HTTP/2 running, can you please make it running again?

@ghys
Copy link
Member

ghys commented Dec 1, 2024

I've often wondered if it could be a service offered by the OH Foundation through myopenhab.org like this:

You would have a way to configure a DNS name that would resolve to your private instance IP like:

<user>-<hardtoguesssuffix>.private.myopenhab.org A <localip>

and also have a way to retrieve a certificate and private key for this name and set it for your instance instead of the self-signed certificate
(or the openHAB Cloud add-on could perhaps even do it for you)

Then you could use that DNS name instead of your local IP to access your instance over HTTPS and it would work - a secure origin for a local instance.

Let's Encrypt do easy wildcard certificates now (others too) so potentially we could issue and sign every certificate for the private.myopenhab.org subdomain.

This would mean the foundation becoming a certificate authority (so having to deal with certificate expirations/revocations, renewals, CRLs/OCSP and so on) as well as maintaining a custom DNS server, so, quite the project.

@kaikreuzer
Copy link
Member

@ghys Why would this be necessary? If I get @florian-h05 correctly, a self-signed certificate (as we have it in place) is sufficient for HTTP/2?

@ghys
Copy link
Member

ghys commented Dec 1, 2024

@kaikreuzer a self-signed certificate will never be sufficient until it's trusted by every client that ever accesses the site...
Trusting an unknown certificate is a manual operation that's pretty involved and depends on the OS.

@kaikreuzer
Copy link
Member

@ghys So far the statements here were only that HTTP/2 requires TLS, but not that it requires a trusted certificate.
Checking StackOverflow, it does not seem to require trusted certs either, that's why I was asking.

@andrewfg
Copy link
Contributor

andrewfg commented Dec 2, 2024

FWIW when I wrote the Hue API V2 binding, I had to get the Jetty dependencies for HTTP2 specifically pulled in, since previously OH Core was only pulling in the Jetty HTTP1 modules.

@ghys
Copy link
Member

ghys commented Dec 2, 2024

@kaikreuzer yes you're right.

Please understand that I wasn't trying to coerce the foundation into doing anything, at this point it was only me and my thoughts 🙂

@kaikreuzer
Copy link
Member

If there's an important use case, I wouldn't mind to the foundation to help on it, but I just wasn't sure whether this discussion belongs to this issue here. 😄

@openhab-bot
Copy link
Collaborator

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-4-3-release-discussion/160888/136

@openhab-bot
Copy link
Collaborator

@openhab-bot
Copy link
Collaborator

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-ui-hangs-until-i-close-another-openhab-browser-tab/161145/5

@andrewfg
Copy link
Contributor

andrewfg commented Dec 27, 2024

I think setting Keep-Alive will probably not solve the issue. HTTP/1 sessions without Keep-Alive are auto-closed after the GET (which auto-frees the session and allows a new one to be created). In other words Keep-Alive won't increase the maximum number of sessions. (Although it will increase the bandwidth and reduce the latency of such sessions).

Also I have another doubt: We would only expect to see benefits from HTTP/1 sessions (with Keep-Alive), or HTTP/2 sessions, if the browser can share a single server session across multiple client tabs. To be honest I don't know how browsers handle inter-tab security, but I can imagine that they might sandbox each tab to prevent session sharing across tabs, and thus prevent cross-tab code injection exploits. Or??

@florian-h05
Copy link
Contributor Author

I think setting Keep-Alive will probably not solve the issue. HTTP/1 sessions without Keep-Alive are auto-closed after the GET (which auto-frees the session and allows a new one to be created). In other words Keep-Alive won't increase the maximum number of sessions.

That might be true.
Looking a bit closer it seems that the SSE connections hold by the UI are the problem ... you can easily get the UI have 3 SSE connections in a single tab. With a limit of 6 max parallel connections (Chrome default) per server you can roughly open two tabs and not do anything inside them.

Also I have another doubt:

Using HTTP/2 has fixed the issue for me, and others confirmed that as well. So we at least need to get HTTP/2 running.

@florian-h05
Copy link
Contributor Author

@wborn Since you said you already had HTTP/2 running. Can you please try to get it running again? I failed doing so.

@andrewfg
Copy link
Contributor

you can easily get the UI have 3 SSE connections in a single tab

It seems to me that OH WebUI code could be improved so that instead of opening multiple SSE pipelines, it could cache and recycle a single pipeline instead.

This is not a suggestion instead of HTTP/2 but rather a suggestion for an additional improvement.

@florian-h05
Copy link
Contributor Author

The UI needs at least two SSE connections in parallel.
One to listen to the event bus and one to listen to the Item states, so the room for improvement is small there.
Only improvement we can do is not create an additional SSE connection when a page (e.g. Items page) subscribes to the event bus, but this would require a pretty big refactoring of the UI's SSE logic.

I think we need a different approach there ... what if the UI kills its SSE connections when it is not the active tab anymore? @ghys WDYT as well?

@andrewfg
Copy link
Contributor

PS this article may help with the jetty.xml config for enabling HTTP2

https://www.baeldung.com/jetty-http-2#configureHttp2JettyXml

@ghys
Copy link
Member

ghys commented Dec 28, 2024

I think we need a different approach there ... what if the UI kills its SSE connections when it is not the active tab anymore? @ghys WDYT as well?

This could work at least for the "state updates" SSE (used in pages and widgets) where the work flow is such that you would ultimately get an initial message with the current states of the items you specified to track.

For the general "events" SSE you would (currently) need to update the objects with the regular REST API when the tab is brought into the foreground again, and then get updates through SSE.

@J-N-K
Copy link
Member

J-N-K commented Dec 28, 2024

@florian-h05 I can get HTTP/2 working, but not on the same port as HTTPS, only on 8444 (instead of 8443). What issues do you encounter? I added two sections to jetty.xml:

        <New id="http2Config" class="org.eclipse.jetty.server.HttpConfiguration">
		<Set name="secureScheme">https</Set>
		<Set name="securePort">
			<Property name="org.osgi.service.http2.port.secure" default="8444" />
		</Set>
		<Set name="outputBufferSize">32768</Set>
		<Set name="requestHeaderSize">8192</Set>
		<Set name="responseHeaderSize">8192</Set>
		<Set name="sendServerVersion">true</Set>
		<Set name="sendDateHeader">false</Set>
		<Set name="headerCacheSize">512</Set>

		<Call name="addCustomizer">
			<Arg>
				<New class="org.eclipse.jetty.server.SecureRequestCustomizer" />
			</Arg>
		</Call>
	</New>
	<Call name="addConnector">
		<Arg>
			<New class="org.eclipse.jetty.server.ServerConnector" id="http2ConnectorId">
				<Arg name="server">
					<Ref refid="Server" />
				</Arg>
				<Arg name="factories">
					<Array type="org.eclipse.jetty.server.ConnectionFactory">
						<Item>
							<New class="org.eclipse.jetty.server.SslConnectionFactory">
								<Arg name="sslContextFactory">
									<Ref refid="sslContextFactory" />
								</Arg>
								<Arg name="next">alpn</Arg>
							</New>
						</Item>
						<Item>
							<New class="org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory">
								<Arg>h2</Arg>
							</New>
						</Item>
						<Item>   
							<New class="org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory">
								<Arg name="config">
									<Ref refid="http2Config" />
								</Arg>
							</New>
						</Item>
					</Array>
				</Arg>
				<Set name="name">
					<SystemProperty name="jetty.host" default="0.0.0.0" />:<SystemProperty name="org.osgi.service.http2.port.secure" default="8444" />
				</Set>
				<Set name="host">
					<SystemProperty name="jetty.host" />
				</Set>
				<Set name="port">
					<SystemProperty name="org.osgi.service.http2.port.secure" default="8444" />
				</Set>
				<Set name="idleTimeout">
					<SystemProperty name="https.timeout" default="30000" />
				</Set>
			</New>
		</Arg>
	</Call>

and lowered the start-level of org.apache.aries.spifly.dynamic.bundle:

bundle:start-level org.apache.aries.spifly.dynamic.bundle 30

@florian-h05
Copy link
Contributor Author

and lowered the start-level of org.apache.aries.spifly.dynamic.bundle:

That did the trick, now HTTP/2 works for me, thanks!
I will open a distro PR with my changes, and I think we should add jetty-alpn-java-server as compile dependency for core's runtime BOM.

@florian-h05
Copy link
Contributor Author

How can I configure the start-level of that bundle so that I don't have to set it through the shell?

@jlaur
Copy link
Contributor

jlaur commented Dec 28, 2024

Perhaps unrelated to the ongoing discussion/solution of using TLS with HTTP/2, but do we know why this issue seems to have gotten worse in 4.3? In 4.2 I always had three active tabs running without issues, but I immediately ran into issues after upgrading to 4.3. I'm wondering if there is something else at play that also might need to be looked into.

@florian-h05
Copy link
Contributor Author

florian-h05 commented Dec 28, 2024

4.2 normally hold 1-2 SSE connections: 1 SSE connection for tracking Item states, and for listening to the event bus when relevant (e.g. when viewing the rules list).
A third SSE connection was only established when web audio was enabled or the UI command Item was enabled.

4.3 always enables that last SSE connection to listen to Item changes to reload the semantic model if required, so you end up with 2-3 SSE connections.
We can revert that last change, because in most cases it might not even be useful (the model reload is only triggered if an Item changes while Main UI is active, and the model is reloaded anyway when opening the app).
That should bring us back to the problem "level" of 4.2.

See openhab/openhab-webui#2949.

florian-h05 added a commit to openhab/openhab-webui that referenced this issue Dec 28, 2024
Related to #2584 and
openhab/openhab-core#4466.

This avoids that the global SSE connection is always established, and
therefore reduces problems with the max parallel HTTP connections
limitation. The global SSE connection is now only established if the web
audio sink or the command Item are enabled.

Whilst this change potentially makes the UI not reload the model
automatically on change, practically the now removed mechanism might
haven't been really useful at all because the model is already reloaded
on Main UI start, and if SSE was not connected the moment an Item
changed the mechanism did not trigger.

Signed-off-by: Florian Hotze <[email protected]>
florian-h05 added a commit to openhab/openhab-webui that referenced this issue Dec 28, 2024
Related to #2584 and
openhab/openhab-core#4466.

This avoids that the global SSE connection is always established, and
therefore reduces problems with the max parallel HTTP connections
limitation. The global SSE connection is now only established if the web
audio sink or the command Item are enabled.

Whilst this change potentially makes the UI not reload the model
automatically on change, practically the now removed mechanism might
haven't been really useful at all because the model is already reloaded
on Main UI start, and if SSE was not connected the moment an Item
changed the mechanism did not trigger.

Signed-off-by: Florian Hotze <[email protected]>
(cherry picked from commit 5d7f35a)
@J-N-K
Copy link
Member

J-N-K commented Dec 28, 2024

How can I configure the start-level of that bundle so that I don't have to set it through the shell?

You can add this to the top of the openhab-tp feature.xml:

        <!-- TODO: workaround for KARAF-7843, remove after upgrade to 4.4.7 or higher -->
	<feature name="spifly">
		<feature>asm</feature>
		<bundle start-level="20">mvn:org.apache.aries.spifly/org.apache.aries.spifly.dynamic.bundle/1.3.7</bundle>
	</feature>

@florian-h05
Copy link
Contributor Author

florian-h05 commented Dec 28, 2024

It either does not work or I am not able to correctly build distro with my core changes ...
I have now created #4526 and openhab/openhab-distro#1711.

I am still getting:

15:58:56.634 [WARN ] [ice.jetty.internal.JettyServerWrapper] - Problem parsing file:///home/florianh/gitrepos/openhab-distro/distributions/openhab/target/openhab-4.3.0-SNAPSHOT/runtime/etc/jetty.xml: null
java.lang.reflect.InvocationTargetException: null
        at jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:74) ~[?:?]
        ...
Caused by: java.lang.IllegalStateException: No Server ALPNProcessors!
        at org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory.<init>(ALPNServerConnectionFactory.java:52) ~[bundleFile:9.4.54.v20240208]
        at org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory.<init>(ALPNServerConnectionFactory.java:45) ~[bundleFile:9.4.54.v20240208]
        at jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62) ~[?:?]
        ... 36 more

@florian-h05
Copy link
Contributor Author

Okay, got it working now - my mistake, wasn't able to build distro properly local.

@florian-h05
Copy link
Contributor Author

An update:

With HTTP/2 enabled now, remaining issue is plain HTTP.

Unfortunately will the Keep-Alice not help, after some investigation I noticed we are running into the max parallel connections limit because of the UI‘s SSE subscriptions.
With a recent UI change, that is part of 4.3.1, I have reverted the UI‘s SSE behaviour to 4.2.x state, which means we have one SSE connections for tracking Item states, one SSE connection if UI Command Item or Web Audio is enabled, and one SSE connection for settings pages like the Items list.
The best solution in the long term is to switch entirely to WebSocket, as the max parallel HTTP connections limitation does not apply to WS.

Unfortunately openHAB Cloud doesn’t support WS yet, but as I anyway would start slowly with switching to WS and provide a option to switch manually, I could imagine using WS for local connections only at the moment.
openHAB Cloud can enable HTTP/2 in nginx to fix the limitation there (ping @digitaldan).

@digitaldan
Copy link
Contributor

Unfortunately openHAB Cloud doesn’t support WS yet,

There is a PR that i have long neglected to review (its rather large) to implement this, so that should help quite a bit.

HTTP/2 in nginx to fix the limitation there

Yeah, hopefully our version includes this and its a one liner to enable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature of the Core
Projects
None yet
Development

No branches or pull requests

9 participants