Include missing CA certificate chain + minor build improvements

[jkrall]

I believe this PR fixes the issues w/ the current (new) certificate that iot.controlmyspa.com has installed. They have installed this new certificate without the necessary intermediate certs, which is technically a server misconfiguration...

However, since Chrome (and other browsers) include the missing intermediate certs to patch over problems like this, it seems unlikely that we'll be able to get them to fix the server configuration anytime soon.

This PR ships the missing CA chain file, along with a patch to app.js that sets the CA file for all axios requests to the controlmyspa.com hostname.

It also includes a couple of build-related tweaks that I fixed-up while debugging this locally.

[mikakoivisto]

Just setting the NODE_TLS_REJECT_UNAUTHORIZED=0 environment variable is enough to fix the badly configured TLS on iot.controlmyspa.com. I can include all the other fixes but I really don't want to include fixed CA chain as that will break the moment they switch their CA or the CA changes the intermediate.

[jkrall]

OK — it's obviously your call.

IMO, setting NODE_TLS_REJECT_UNAUTHORIZED is a hack that makes the add-on less-secure... it's not a huge deal, but there are a number of MITM or DNS-based attacks that could serve an invalid certificate that would be allowed if that environment variable is set.

It also seems like a weird thing to have an option that is required 100% of the time — since the current certificate is broken. How are end-users supposed to know whether to enable that new option or not? The certificate error isn't exactly obvious in the logs.

[MartinGoX]

Hi Joshua Thanks a lot for your feedback. Yes, it was me who contacted Mika also. And he did this "Hack". I'm aware of its danger. Nevertheless I will do it 😉

Best regards Martin

11.07.2023 09:40:42 Joshua Krall @.***>:

OK — it's obviously your call.

IMO, setting NODE_TLS_REJECT_UNAUTHORIZED is a hack that makes the add-on less-secure... it's not a huge deal, but there are a number of MITM or DNS-based attacks that could serve an invalid certificate that would be allowed if that environment variable is set.

It also seems like a weird thing to have an option that is required 100% of the time — since the current certificate is broken. How are end-users supposed to know whether to enable that new option or not? The certificate error isn't exactly obvious in the logs.

— Reply to this email directly, view it on GitHub[https://github.com/mikakoivisto/controlmyspa-ha-mqtt/pull/11#issuecomment-1630312813], or unsubscribe[https://github.com/notifications/unsubscribe-auth/A7TH3NYLTMMQQF3WBTPWOFTXPT7PRANCNFSM6AAAAAA2FRRQKA]. You are receiving this because you are subscribed to this thread.[Verfolgungsbild][https://github.com/notifications/beacon/A7TH3NZFI7ZZPUFF6EDQKXDXPT7PRA5CNFSM6AAAAAA2FRRQKCWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTBFSMW2.gif]

[mikakoivisto]

I don't disagree with the dangers of the option to turn off the verification. I just don't like adding that kind of code either. Better option would be to add the missing intermedia certificate to the CA truststore of the system. That is what is done Java application usually because it is way easier than doing the override that based on your code looks quite easy in NodeJS. I'll investigate that option when I have time but for now only the disabling of TLS verification is the only option I include. Last time they messed up the certificate setup it got eventually fixed on the server.

[jkrall]

Yeah, I agree that adding the intermediate CA to the CA truststore would be a much better answer — in my quick research, I wasn't able to find a solution like this that worked with nodejs. Perhaps I missed something, but the solutions I found all worked via explicitly loading the CA cert and providing it as configuration to the https request. If you can figure out how to do it at the system level, I agree that would be better.

An additional call-out: while I agree that they will likely fix this at the server level at some point ... the certificate they purchased will always be locked to these specific intermediate CAs. So installing/providing our own intermediate CA chain isn't going to conflict with a future server config fix, at least until they purchase and install a new certificate that has a different chain.

Finally — I can also see an argument that IF we were to keep an application-level fix like the one I proposed here... it might actually be more appropriate to have it upstream in the controlmyspa repo instead of in here.

[jkrall]

Anyway — above comments aside — feel free to close this PR, as the current version seems acceptable to everyone involved.

[MartinGoX]

Hi everyone. As I was travelling yesterday I had no chance to watch for all the messages (although I didn't understood everything in detail)... Let me thank you both of you for your support and time spent on this issue. Spa is now available again, great!

[mikakoivisto]

This commit fixes it properly without any code changes and won't break even if they change their CA. https://github.com/mikakoivisto/controlmyspa-ha-mqtt/commit/973453eaffc8a4db194a3b1a76c5f742dfdf1a2e