Skip to content

Fix self-signed certificate handling without SAN ("Hostname not verified") - #6646

Open
mahibi wants to merge 2 commits into
masterfrom
acceptUserDecisionToTrustCert
Open

Fix self-signed certificate handling without SAN ("Hostname not verified")#6646
mahibi wants to merge 2 commits into
masterfrom
acceptUserDecisionToTrustCert

Conversation

@mahibi

@mahibi mahibi commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

improves the situation for #5509 and #5439

The hostname verifier rejected any certificate lacking a matching Subject Alternative Name (SAN) — e.g. legacy self-signed certificates that only set a Common Name — even after the user had already explicitly trusted that exact certificate via the "untrusted certificate" dialog. The manual trust decision was silently discarded one step later during hostname verification.

Now, when the strict SAN-based check fails, the verifier falls back to checking whether the user already trusted this specific certificate. Certificates that are neither CA-signed nor manually trusted are still rejected as before — this only relaxes the check for certificates the user has explicitly accepted.

Added unit tests covering the hostname verifier's behavior (SAN match, no-SAN rejected/accepted depending on manual trust, missing/unreadable peer certificates).

Test plan

  • Unit tests added and passing (TrustManagerTest)
  • Verified new tests fail against the pre-fix code
  • Manual test against a real self-signed certificate without SAN

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not needed
  • 🔖 Capability is checked or not needed
  • 🔙 Backport requests are created or not needed: /backport to stable-xx.x
  • 📅 Milestone is set
  • 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

…rusted

The hostname verifier rejected any certificate lacking a matching
Subject Alternative Name, even after the user explicitly trusted that
exact certificate via the certificate dialog. Fall back to the manual
trust store when the strict SAN check fails, so legacy self-signed
certificates that only set a Common Name work once accepted.

Assisted-by: Claude Code:claude-sonnet-5

Signed-off-by: Marcel Hibbe <[email protected]>
Covers the hostname verifier fallback: a certificate without a
matching SAN is rejected unless the user already trusted it manually,
a certificate with a matching SAN is accepted, and missing/unreadable
peer certificates are rejected. Verified these tests fail against the
pre-fix TrustManager.

Adds okhttp-tls as a test dependency to generate real self-signed
certificates (with and without a SAN) instead of certificate mocks,
since KeyStore.setCertificateEntry needs an encodable certificate.

Assisted-by: Claude Code:claude-sonnet-5

Signed-off-by: Marcel Hibbe <[email protected]>
@mahibi mahibi added this to the 25.1.0 milestone Sep 4, 2026
@mahibi
mahibi requested a review from rapterjet2004 September 4, 2026 14:45
@mahibi mahibi self-assigned this Sep 4, 2026
@mahibi mahibi added the 3. to review Waiting for reviews label Sep 4, 2026
@mahibi mahibi changed the title Accept user decision to trust cert Fix self-signed certificates without SAN ("Hostname not verified") Sep 4, 2026
@mahibi

mahibi commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

/backport to stable-25.0.x

@mahibi mahibi changed the title Fix self-signed certificates without SAN ("Hostname not verified") Fix self-signed certificate handling without SAN ("Hostname not verified") Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

APK file: https://github.com/nextcloud/talk-android/actions/runs/33885571520/artifacts/9941962206
To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.
qrcode (please click on link to get QR code displayed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants