Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,12 @@ internal class ConnectionViewModel @VisibleForTesting constructor(

init {
viewModelScope.launch {
// Pre-set the mTLS flag before emitting the auth URL. If the phone is currently
// connected to an mTLS-protected instance, the private key is already loaded in
// memory. The onboarding WebView will reuse the live TLS session (session resumption)
// so onReceivedClientCertRequest never fires — pre-setting the flag ensures the
// Wear OS cert-selection screen is not silently skipped.
webViewClient.preInitializeTLSClientAuthState()
buildAuthUrl(rawUrl)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,36 @@ open class TLSWebViewClient(private var keyChainRepository: KeyChainRepository)
private var key: PrivateKey? = null
private var chain: Array<X509Certificate>? = null

/**
* Pre-initializes [isTLSClientAuthNeeded] from the current in-memory key state to handle
* TLS session resumption.
*
* Normally [isTLSClientAuthNeeded] is set when [onReceivedClientCertRequest] fires during
* a full TLS handshake. However, when TLS session resumption occurs (the WebView reuses an
* existing session from the same process), the server does not issue a new
* `CertificateRequest`, so [onReceivedClientCertRequest] is never called — even if the
* server requires a client certificate.
*
* This is the root cause of the Wear OS onboarding mTLS failure: the main app WebView
* establishes a TLS session while the user is connected; the onboarding WebView immediately
* resumes it, bypassing the callback that would reveal the mTLS requirement to the
* navigation layer.
*
* The fix checks whether the repository already holds a loaded private key in memory. A
* non-null key means the phone is currently connected to an instance that required a client
* certificate, so [isTLSClientAuthNeeded] is pre-set to `true`. If the app was force-stopped
* first (clearing in-memory state) no TLS session can be resumed either, so
* [onReceivedClientCertRequest] will fire naturally on the fresh handshake.
*
* Must be called **before** the WebView starts loading (i.e. before the URL is emitted).
* Idempotent: if the flag is already `true` (set by a real handshake) this is a no-op.
*/
fun preInitializeTLSClientAuthState() {
if (!isTLSClientAuthNeeded) {
isTLSClientAuthNeeded = keyChainRepository.getPrivateKey() != null
}
Comment on lines +67 to +69
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A more precise fix would store the host from request.host inside onReceivedClientCertRequest in the shared KeyChainRepository singleton and compare it against rawUrl.host in preInitializeTLSClientAuthState. Happy to add that if reviewers feel it is warranted.

Indeed, if I'm not wrong it means that if I onboard a new server that is not using mTLS but one of my server has mTLS I'm going to be forced to pick a cert for the wear where I might have none. It shouldn't be an issue to give a "fake" cert but still a pretty bad experience.

We could try to store the host information indeed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had assumed the cert picker in the UI was optional even if the cert was considered "mandatory". I must admit I didn't check or I was remembering an old implementation.

It might want it to be optional anyway, in the event the server specifies it is optional. Although I believe at the moment it only presents this screen if it's mandatory (and if you don't hit this bug).

We could alternatively set a flag stating that it is "probably/optionally required" and let the user decide whether to provide one or not.

but one of my server has mTLS

I understand this will only be the case if the server you are currently logged into in the phone app requires mTLS. Note prior to this change it wouldn't work at all in this scenario if the server you are onboarding the watch to also requires mTLS (most common case). You get a "could not register watch" toast and no further info.

I can add the host matching - but am a bit concerned about matching domain suffixes, http vs https and so on.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add the host matching - but am a bit concerned about matching domain suffixes, http vs https and so on.

What is your concern?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just meant matching myhainstance vs mhainstance.lan vs http://127.0.01 vs https://localhost etc, it's not a simple strcmp. I guess the certificate CNAME has to match anyway.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have some utils already for that in the repo like

fun URL.baseIsEqual(other: URL?): Boolean = if (other == null) {
or
fun HttpUrl.hasSameOrigin(other: HttpUrl): Boolean {

}

private fun getActivity(context: Context?): Activity? {
if (context == null) {
return null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -417,4 +417,32 @@ class ConnectionViewModelTest {
assertEquals("class java.lang.UnsatisfiedLinkError", error.rawErrorType)
verify(exactly = 1) { connectivityCheckRepository.runChecks(rawUrl) }
}

@Test
fun `Given a private key already loaded in memory when initializing then isTLSClientAuthNeeded is pre-set to true`() = runTest {
every { keyChainRepository.getPrivateKey() } returns mockk()

val viewModel = ConnectionViewModel(
"http://homeassistant.local:8123",
webViewClientFactory,
connectivityCheckRepository,
)
advanceUntilIdle()

assertTrue(viewModel.webViewClient.isTLSClientAuthNeeded)
}

@Test
fun `Given no private key in memory when initializing then isTLSClientAuthNeeded remains false`() = runTest {
every { keyChainRepository.getPrivateKey() } returns null

val viewModel = ConnectionViewModel(
"http://homeassistant.local:8123",
webViewClientFactory,
connectivityCheckRepository,
)
advanceUntilIdle()

assertFalse(viewModel.webViewClient.isTLSClientAuthNeeded)
}
}