Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow empty fingerprints of MQTT SSL server #1682

Closed
wants to merge 1 commit into from

Conversation

Niek
Copy link
Contributor

@Niek Niek commented Apr 5, 2019

This change allows empty fingerprints of MQTT SSL servers - so bypass key pinning.

Of course this is not really secure, as it allows MITM attacks with fake signatures. But in my opinion this is worth supporting, this way it's much easier to get started with a MQTT/SSL broker setup. In addition, LetsEncrypt certificates change signature every renewal (90 days), so it makes it possible to work with LE-issued certificated without changing the fingerprint at every renewal.

@mcspr
Copy link
Collaborator

mcspr commented Apr 6, 2019

Maybe additional settings flag instead? something like mqttInsecure

Another thing, from #1465 : we can use PubSubClient with real certificates starting with Core 2.4.2 (although, as I've noticed, it might choke on some specific certificate types. cloudmqtt was one of them)
It is a separate thing from fingerprinting and might also use such option.

@Niek
Copy link
Contributor Author

Niek commented Apr 8, 2019

Ah, I was not even aware of the other issue.

In case support for real certificates is added in the future, it might make sense to use something like `MQTT_SSL_VALIDATION`` which can be none/insecure, fingerprinting, and in future known key (even though it has the same issue as fingerprinting), or trust anchor (maybe in combination with SNI check).

I also have some issues with MQTT/SSL timing of messages, but I guess the other topic is a better place for that.

@Niek
Copy link
Contributor Author

Niek commented Jun 26, 2019

Will be updated once #1751 is merged

@Niek
Copy link
Contributor Author

Niek commented Jul 17, 2019

Closed in favor of #1829

@Niek Niek closed this Jul 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants