Use less strict certificate checking for ComEd #74361

glenviewjeff · 2022-07-03T19:06:19Z

Fixing this issue: #74320

It may be because ComEd isn't using intermediate certificates per digicert's diagnostics tool:

DNS resolves hourlypricing.comed.com to 45.60.152.155
HTTP Server Header: Apache

TLS Certificate
Common Name = hourlypricing.comed.com

Organization = Exelon Corporation

City/Locality = Chicago

State/Province = Illinois

Country = US

Subject Alternative Names = hourlypricing.comed.com

Issuer = DigiCert TLS RSA SHA256 2020 CA1

Serial Number = 0CFD656D10D544BC1024EC8256F96786

SHA1 Thumbprint = 6947F33F43CBDEBD6350CCD8D00EB0D229837692

Key Length = 2048

Signature algorithm = SHA256-RSA

Secure Renegotiation:

TLS Certificate has not been revoked
OCSP Staple: Not Enabled
OCSP Origin: Good
CRL Status: Good

TLS Certificate expiration
The certificate expires June 23, 2023 (355 days from today)

Certificate Name matches hourlypricing.comed.com

Subject hourlypricing.comed.com
Valid from 23/Jun/2022 to 23/Jun/2023
Issuer DigiCert TLS RSA SHA256 2020 CA1
The server is not sending the required intermediate certificate.
In most cases, solving this problem in Apache is as simple as adding "SSLCertificateChainFile /path/to/DigiCertCA.crt" to your apache configuration file after/near your SSLCertificateFile line.

You can find the missing intermediate in the zip file containing your certificate, or download it from your customer account area.

Follow the directions on our certificate installation guide to install the missing intermediate.

If you have any problems correcting this issue, please contact our helpful support team and we would be happy to assist.

Breaking change

Proposed change

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)
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 # 74320
This PR is related to issue:
Link to documentation pull request:

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

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.
Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

No score or internal
🥈 Silver
🥇 Gold
🏆 Platinum

To help with the load of incoming pull requests:

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

It may be because ComEd isn't using intermediate certificates per digicert's diagnostics tool: DNS resolves hourlypricing.comed.com to 45.60.152.155 HTTP Server Header: Apache TLS Certificate Common Name = hourlypricing.comed.com Organization = Exelon Corporation City/Locality = Chicago State/Province = Illinois Country = US Subject Alternative Names = hourlypricing.comed.com Issuer = DigiCert TLS RSA SHA256 2020 CA1 Serial Number = 0CFD656D10D544BC1024EC8256F96786 SHA1 Thumbprint = 6947F33F43CBDEBD6350CCD8D00EB0D229837692 Key Length = 2048 Signature algorithm = SHA256-RSA Secure Renegotiation: TLS Certificate has not been revoked OCSP Staple: Not Enabled OCSP Origin: Good CRL Status: Good TLS Certificate expiration The certificate expires June 23, 2023 (355 days from today) Certificate Name matches hourlypricing.comed.com Subject hourlypricing.comed.com Valid from 23/Jun/2022 to 23/Jun/2023 Issuer DigiCert TLS RSA SHA256 2020 CA1 The server is not sending the required intermediate certificate. In most cases, solving this problem in Apache is as simple as adding "SSLCertificateChainFile /path/to/DigiCertCA.crt" to your apache configuration file after/near your SSLCertificateFile line. You can find the missing intermediate in the zip file containing your certificate, or download it from your customer account area. Follow the directions on our certificate installation guide to install the missing intermediate. If you have any problems correcting this issue, please contact our helpful support team and we would be happy to assist.

homeassistant · 2022-07-03T19:06:23Z

Hi @glenviewjeff,

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!

frenck · 2022-07-03T19:47:13Z

homeassistant/components/comed_hourly_pricing/sensor.py

@@ -113,7 +113,7 @@ async def async_update(self):
                    url_string += "?type=currenthouraverage"

                async with async_timeout.timeout(60):
-                    response = await self.websession.get(url_string)
+                    response = await self.websession.get(url_string, ssl=False)


Disabling SSL checks is not a solution to issues like this, especially considering this is a public service. This PR basically introduces a security issue.

@frenck From what I understand from the documentation, this setting doesn't disable the checks, it only uses less strict checks, presumably more along the lines of what Chrome does in practice. The certificates currently pass under Chrome but not with the default SSL values from this library.

By default aiohttp uses strict checks for HTTPS protocol. Certification checks can be relaxed by setting ssl to False:

r = await session.get('https://example.com', ssl=False)

If I'm misunderstanding the source of the problem (that it's the strict checking that's causing the issue because of the missing intermediate certificate,) then this SO post and/or this github issue may hold some clues. I'd play around but I don't yet know how to import a development version of an integration into HA (I've only been playing with HA for a couple of weeks.)

From what I understand from the documentation, this setting doesn't disable the checks, it only uses less strict checks

That is correct, meaning aiohttp will accept anything. Even self-signed and invalid certificates (that is the security issue created).

Actually ssl=false does appear to completely skip validation according to this section of the documentation.

does appear to completely skip validation according to

Yes. Which is the security issue :)

So... what do we do when Home Assistant uses different rules for intermediate certificate downloads/checks than modern browsers? I agree disabling SSL checks is not the solution, but the SSL certificate is valid, the chain is just incomplete or is expected to be part of the OS instead of served via the remote webserver. So what is the solution here?

The correct fix is ComEd needs to fix their site/service. I've reached out to them about this issue (via email and on twitter) as it's something they really need to address on their end. I also agree that not strictly checking the certificate reduces security and isn't the correct/proper fix. Their site/TLS endpoint needs to provide the full certificate chain as is required for proper TLS security.

Looks like they fixed it, it's working for me now suddenly with no changes required in HA. The entity just started responding.

glenviewjeff · 2022-07-03T21:13:52Z

Care to point me to some simple instructions for modifying and testing integrations? For now, I just copied the code to a custom_components directory. Is this sound? Do I have to rename it uniquely or will it override the built-in comed_hourly_pricing integration?

…

On Sun, Jul 3, 2022 at 3:58 PM Franck Nijhof ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In homeassistant/components/comed_hourly_pricing/sensor.py <#74361 (comment)>: > @@ -113,7 +113,7 @@ async def async_update(self): url_string += "?type=currenthouraverage" async with async_timeout.timeout(60): - response = await self.websession.get(url_string) + response = await self.websession.get(url_string, ssl=False) does appear to completely skip validation according to Yes. Which is the security issue :) — Reply to this email directly, view it on GitHub <#74361 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACRYQWFSLGOGXSZUOFYGM3VSH5F3ANCNFSM52RDYSQA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

frenck · 2022-07-03T21:17:38Z

All information regarding the development of Home Assistant can be found on our developers portal.

https://developers.home-assistant.io/docs/development_environment

glenviewjeff · 2022-07-03T21:39:52Z

Thanks, with the documentation help I at least got my own version functioning now with the SSL bypass. I'll see if I can fix it properly with the suggestions on those two pages I referenced.

…

On Sun, Jul 3, 2022 at 4:17 PM Franck Nijhof ***@***.***> wrote: All information regarding the development of Home Assistant can be found on our developers portal. https://developers.home-assistant.io/docs/development_environment — Reply to this email directly, view it on GitHub <#74361 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACRYQQ4P32DDO57QW42O7TVSH7P3ANCNFSM52RDYSQA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

kbx81 · 2022-07-05T22:17:20Z

Can probably close this PR now -- ComEd fixed the certificate on their end.

glenviewjeff · 2022-07-06T00:22:49Z

Yes I went another route and filed regulatory and watchdog group complaints. The watchdog group responded immediately that they sent it to the right person in the company along with instructions on how to fix the certificate.

…

On Tue, Jul 5, 2022, 6:27 PM Jake Burgy ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In homeassistant/components/comed_hourly_pricing/sensor.py <#74361 (comment)>: > @@ -113,7 +113,7 @@ async def async_update(self): url_string += "?type=currenthouraverage" async with async_timeout.timeout(60): - response = await self.websession.get(url_string) + response = await self.websession.get(url_string, ssl=False) Looks like they fixed it, it's working for me now suddenly with no changes required in HA. The entity just started responding. — Reply to this email directly, view it on GitHub <#74361 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACRYQSMABLI45JDKUVQTWLVSTAGZANCNFSM52RDYSQA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

frenck · 2022-07-06T06:56:14Z

Alright, since this issue has been resolved upstream, I'm going to close this PR.

../Frenck

probot-home-assistant bot added integration: comed_hourly_pricing small-pr PRs with less than 30 lines. labels Jul 3, 2022

homeassistant added the cla-needed label Jul 3, 2022

glenviewjeff changed the title ~~Use less strict certificate checking. It may be that ComEd~~ Use less strict certificate checking for ComEd Jul 3, 2022

glenviewjeff mentioned this pull request Jul 3, 2022

Comed API Integration stopped working #74320

Closed

frenck requested changes Jul 3, 2022

View reviewed changes

frenck closed this Jul 6, 2022

github-actions bot locked and limited conversation to collaborators Jul 7, 2022

homeassistant added cla-signed and removed cla-needed labels Jul 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use less strict certificate checking for ComEd #74361

Use less strict certificate checking for ComEd #74361

glenviewjeff commented Jul 3, 2022 •

edited

Loading

homeassistant commented Jul 3, 2022

frenck Jul 3, 2022

glenviewjeff Jul 3, 2022 •

edited

Loading

frenck Jul 3, 2022 •

edited

Loading

glenviewjeff Jul 3, 2022

frenck Jul 3, 2022

qJake Jul 5, 2022

kbx81 Jul 5, 2022

qJake Jul 5, 2022

glenviewjeff commented Jul 3, 2022 via email

frenck commented Jul 3, 2022

glenviewjeff commented Jul 3, 2022 via email

kbx81 commented Jul 5, 2022

glenviewjeff commented Jul 6, 2022 via email

frenck commented Jul 6, 2022

Use less strict certificate checking for ComEd #74361

Use less strict certificate checking for ComEd #74361

Conversation

glenviewjeff commented Jul 3, 2022 • edited Loading

Breaking change

Proposed change

Type of change

Additional information

Checklist

homeassistant commented Jul 3, 2022

frenck Jul 3, 2022

Choose a reason for hiding this comment

glenviewjeff Jul 3, 2022 • edited Loading

Choose a reason for hiding this comment

frenck Jul 3, 2022 • edited Loading

Choose a reason for hiding this comment

glenviewjeff Jul 3, 2022

Choose a reason for hiding this comment

frenck Jul 3, 2022

Choose a reason for hiding this comment

qJake Jul 5, 2022

Choose a reason for hiding this comment

kbx81 Jul 5, 2022

Choose a reason for hiding this comment

qJake Jul 5, 2022

Choose a reason for hiding this comment

glenviewjeff commented Jul 3, 2022 via email

frenck commented Jul 3, 2022

glenviewjeff commented Jul 3, 2022 via email

kbx81 commented Jul 5, 2022

glenviewjeff commented Jul 6, 2022 via email

frenck commented Jul 6, 2022

glenviewjeff commented Jul 3, 2022 •

edited

Loading

glenviewjeff Jul 3, 2022 •

edited

Loading

frenck Jul 3, 2022 •

edited

Loading