Make cert_expiry work with self-signed / private CA signed certificates #137452

cromefire · 2025-02-05T18:57:49Z

Breaking change

Not 100% sure. If people relied on this for checking the cert chain it'd break, but that wasn't the mission of this extension, so IMO this should be fine.

Proposed change

Removes the validation of the certifivate

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes Certificate Expiry integration does not handle self signed or personal cert authorities #91170
This PR is related to issue:
Link to documentation pull request:
Link to developer documentation pull request:
Link to frontend pull request:

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass - Some tests (in other integrations) are not working, but nothing was changed, dependencies are missing from the requirement files it seems.
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works. - Not really possible in a good way here

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

home-assistant

Hi @cromefire

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

home-assistant · 2025-02-05T18:57:57Z

Hey there @jjlawren, mind taking a look at this pull request as it has been labeled with an integration (cert_expiry) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of cert_expiry can trigger bot actions by commenting:

@home-assistant close Closes the pull request.
@home-assistant rename Awesome new title Renames the pull request.
@home-assistant reopen Reopen the pull request.
@home-assistant unassign cert_expiry Removes the current integration label and assignees on the pull request, add the integration domain after the command.
@home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
@home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

cromefire · 2025-02-05T19:55:32Z

So it doesn't seem like the async way can be used, because it just doesn't properly provide the peer certificate, so the stdlib way it is.

Also also the docs on setting everything up properly seem to be somewhat trash, it seems to miss many of the packages required for some tests and so a bunch of tests just fail all the time.

frenck · 2025-03-02T20:45:12Z

homeassistant/components/cert_expiry/config_flow.py

@@ -67,7 +66,7 @@ async def async_step_user(
            await self.async_set_unique_id(f"{host}:{port}")
            self._abort_if_unique_id_configured()

-            if await self._test_connection(user_input):
+            if self._test_connection(user_input):


You can't just do sync operation in async context; this is blocking the loop now.

The only option I can think of is spawning a background thread and awaiting the result. IIRC from my python days many years ago there is a way in the multiprocessing module to do that with a thread pool of some sort. Would that be acceptable?

I don't suppose I've got any other options here, as the async way it was done before only seems to provide the parsed cert, which is only populated if certificate validation is disabled, which it has to be to fix the issue.

I left a different comment with the hope of helping keep everything pure async (which would be my own preference), but the homeassistant-standard way of achieving this safely would be to follow this:
https://developers.home-assistant.io/docs/asyncio_working_with_async/#calling-sync-functions-from-async

example from linked page:

# hub.update() is a sync function. result = await hass.async_add_executor_job(hub.update)

@frenck I wrapped the socket call into hass.async_add_executor_job now, is that acceptable? I will still have to retest, didn't get to that yet, but functionality should be the same.

home-assistant · 2025-03-02T20:45:16Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

homeassistant/components/cert_expiry/helper.py

pergolafabio · 2025-03-13T06:58:43Z

Guys, while you are working on the PR, can you also change to :

add 2 lines:


        ctx=get_default_context()
        ctx.verify_mode=ssl.CERT_REQUIRED

async def async_get_cert(
    hass: HomeAssistant,
    host: str,
    port: int,
) -> dict[str, Any]:
    """Get the certificate for the host and port combination."""
    async with asyncio.timeout(TIMEOUT):
        ctx=get_default_context()
        ctx.verify_mode=ssl.CERT_REQUIRED
        transport, _ = await hass.loop.create_connection(
            asyncio.Protocol,
            host,
            port,
            ssl=ctx,
            happy_eyeballs_delay=0.25,
            server_hostname=host,
        )
    try:
        return transport.get_extra_info("peercert")  # type: ignore[no-any-return]
    finally:
        transport.close()

this will fix/close issue:
#136634

cromefire · 2025-03-13T20:45:45Z

Guys, while you are working on the PR, can you also change to :

No completely sure what you're trying to achieve, but the whole call has changed to:

context = get_default_context()
context.check_hostname = False
context.verify_mode = ssl.CERT_NONE
context.verify_flags = context.verify_flags | ssl.VERIFY_CRL_CHECK_CHAIN

conn = socket.create_connection((host, port), timeout=TIMEOUT)
sock = context.wrap_socket(conn, server_hostname=host)

so I'd ask you to retest once this has landed and see if your issue persists at all.

cromefire · 2025-03-13T21:05:37Z

It seems indeed everything is working correctly still, even with the background thread changes:

The config flow works as well.

Stale

cromefire · 2025-03-13T21:22:42Z

Okay found one issue, the is valid tag was not working properly because it relied on python to validate the cert before, but now it works again:

Maes152 · 2025-05-19T08:36:24Z

ssl.CERT_REQUIRED

Not sure where the merging of this PR stands, but the code change request specifically targets the following:

context.verify_mode = ssl.CERT_NONE

change to

context.verify_mode = ssl.CERT_REQUIRED

cromefire · 2025-05-19T08:39:04Z

ssl.CERT_REQUIRED

Not sure where the merging of this PR stands, but the code change request specifically targets the following:
context.verify_mode = ssl.CERT_NONE
change to
context.verify_mode = ssl.CERT_REQUIRED

It does, otherwise self-signed certs don't work. I'm currently waiting on a review with the changes I made.

cromefire added 2 commits February 5, 2025 19:48

[cert_expiry] Disable certificate verification in context

17eef63

[cert_expiry] Retrieve the certificate from an SSL socket

d5d4158

home-assistant bot previously requested changes Feb 5, 2025

View reviewed changes

home-assistant bot added bugfix cla-needed integration: cert_expiry small-pr PRs with less than 30 lines. labels Feb 5, 2025

home-assistant bot assigned jjlawren Feb 5, 2025

home-assistant bot added Quality Scale: No score cla-recheck cla-signed and removed cla-recheck cla-needed labels Feb 5, 2025

Merge branch 'dev' into patch-1

5c38dc7

cromefire mentioned this pull request Feb 5, 2025

Certificate Expiry integration does not handle self signed or personal cert authorities #91170

Open

cromefire changed the title ~~Make certificate_expiry work with self-signed / private CA signed certificates.~~ Make cert_expiry work with self-signed / private CA signed certificates. Feb 5, 2025

cromefire added 3 commits February 5, 2025 20:09

[cert_expiry] Remove unused hass helper

498e8b3

Removed async where it's not needed

fd044fe

Try pre commit again

711021f

cromefire marked this pull request as ready for review February 5, 2025 21:29

cromefire requested a review from jjlawren as a code owner February 5, 2025 21:29

Merge branch 'dev' into patch-1

21508c3

MartinHjelmare changed the title ~~Make cert_expiry work with self-signed / private CA signed certificates.~~ Make cert_expiry work with self-signed / private CA signed certificates Feb 23, 2025

frenck requested changes Mar 2, 2025

View reviewed changes

home-assistant bot marked this pull request as draft March 2, 2025 20:45

PaarthShah reviewed Mar 13, 2025

View reviewed changes

homeassistant/components/cert_expiry/helper.py Show resolved Hide resolved

Scheduling the checks on background threads

89c4aa4

Merge branch 'dev' into patch-1

f8bbdab

cromefire marked this pull request as ready for review March 13, 2025 21:05

home-assistant bot requested a review from frenck March 13, 2025 21:05

Set certificate as invalid if expired

25fba7e

cromefire added 3 commits March 14, 2025 00:33

Merge branch 'dev' into patch-1

7701f5f

Merge branch 'dev' into patch-1

d835ec3

Merge branch 'dev' into patch-1

2dcc576

Merge branch 'dev' into patch-1

fe0552b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Make cert_expiry work with self-signed / private CA signed certificates #137452

Make cert_expiry work with self-signed / private CA signed certificates #137452

cromefire commented Feb 5, 2025 •

edited

Loading

Uh oh!

home-assistant bot left a comment

Uh oh!

home-assistant bot commented Feb 5, 2025

Uh oh!

cromefire commented Feb 5, 2025 •

edited

Loading

Uh oh!

frenck Mar 2, 2025

Uh oh!

cromefire Mar 2, 2025

Uh oh!

PaarthShah Mar 13, 2025 •

edited

Loading

Uh oh!

cromefire Mar 13, 2025

Uh oh!

home-assistant bot commented Mar 2, 2025

Uh oh!

Uh oh!

pergolafabio commented Mar 13, 2025

Uh oh!

cromefire commented Mar 13, 2025

Uh oh!

cromefire commented Mar 13, 2025

Uh oh!

cromefire commented Mar 13, 2025

Uh oh!

Maes152 commented May 19, 2025

Uh oh!

cromefire commented May 19, 2025

Uh oh!

Uh oh!

Uh oh!

Make cert_expiry work with self-signed / private CA signed certificates #137452

Are you sure you want to change the base?

Make cert_expiry work with self-signed / private CA signed certificates #137452

Conversation

cromefire commented Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Breaking change

Proposed change

Type of change

Additional information

Checklist

Uh oh!

home-assistant bot left a comment

Choose a reason for hiding this comment

Uh oh!

home-assistant bot commented Feb 5, 2025

Uh oh!

cromefire commented Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

frenck Mar 2, 2025

Choose a reason for hiding this comment

Uh oh!

cromefire Mar 2, 2025

Choose a reason for hiding this comment

Uh oh!

PaarthShah Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cromefire Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

home-assistant bot commented Mar 2, 2025

Uh oh!

Uh oh!

pergolafabio commented Mar 13, 2025

Uh oh!

cromefire commented Mar 13, 2025

Uh oh!

cromefire commented Mar 13, 2025

Uh oh!

cromefire commented Mar 13, 2025

Uh oh!

Maes152 commented May 19, 2025

Uh oh!

cromefire commented May 19, 2025

Uh oh!

Uh oh!

cromefire commented Feb 5, 2025 •

edited

Loading

cromefire commented Feb 5, 2025 •

edited

Loading

PaarthShah Mar 13, 2025 •

edited

Loading