Security enhancement: disable "old" fingerprint derivation by default

[wewa00]

PROBLEM DESCRIPTION

A clear and concise description of what the problem is.
Regarding the TLS patch from Castellucci (https://threadreaderapp.com/thread/1339101572832382981.html).
As far as I understand, Castellucci found an issue where an attacker could use a weakness in the "old" implementation of the Tasmota TLS fingerprint derivation for a MidM attack.
Castellucci provided a patch for this which improved the fingerprint derivation ("new method") and automatically updates the "old" and weaker fingerprints. Keys which are not "suspicious" as Castellucci calls it. The patch was introduced with this commit: 66b3dc1#diff-020ddbc1f66e15dc2b9e2d1608ad6913babc507a65625efbb07cb987e026ca08

But this function for updating the "old" fingerprint suggests to me, that the "old" method is still active and used somehow. I assume, that the fingerprint check falls back to the "old method" in case the fingerprint can not be verified with the "new method".
How is the function able to differ between "I have an 'old' fingerprint stored and need to update to the 'new' one." and "I have a 'new' fingerprint stored and the check fails because the MQTT host is using a wrong TLS certificate and do not need to check with 'old' method"?
Because according my current thinking, an attacker could use the weakness of the "old" algorithm in case Tasmota does not differ between "old" and "new".

REQUESTED INFORMATION

Make sure your have performed every step and checked the applicable boxes before submitting your issue. Thank you!

• Read the Contributing Guide and Policy and the Code of Conduct
• Searched the problem in issues
• Searched the problem in discussions
• Searched the problem in the docs
• Searched the problem in the chat
• Device used (e.g., Sonoff Basic): not relevant
• Tasmota binary firmware version number used: v9.2.0
  • Pre-compiled
  • Self-compiled
• Flashing tools used: OTA update
• Provide the output of command: Backlog Template; Module; GPIO 255:

  Configuration output here:
{"NAME":"Generic","GPIO":[1,1,1,1,1,1,1,1,1,1,1,1,1,1],"FLAG":0,"BASE":18}
{"Module":{"8":"Sonoff S2X"}}
{"GPIO0":{"32":"Button1"},"GPIO1":{"0":"None"},"GPIO2":{"0":"None"},"GPIO3":{"0":"None"},"GPIO4":{"0":"None"},"GPIO5":{"0":"None"},"GPIO9":{"0":"None"},"GPIO10":{"0":"None"},"GPIO12":{"224":"Relay1"},"GPIO13":{"320":"Led_i1"},"GPIO14":{"0":"None"},"GPIO15":{"0":"None"},"GPIO16":{"0":"None"},"GPIO17":{"0":"None"}}}

• If using rules, provide the output of this command: Backlog Rule1; Rule2; Rule3:

  Rules output here:

• Provide the output of this command: Status 0:

  STATUS 0 output here:
{"Status":{"Module":8,"DeviceName":"Tasmota","FriendlyName":["Tasmota"],"Topic":"wohnzimmer/audio","ButtonTopic":"0","Power":0,"PowerOnState":3,"LedState":0,"LedMask":"FFFF","SaveData":1,"SaveState":1,"SwitchTopic":"0","SwitchMode":[0,0,0,0,0,0,0,0],"ButtonRetain":0,"SwitchRetain":0,"SensorRetain":0,"PowerRetain":0}}

• Set weblog to 4 and then, when you experience your issue, provide the output of the Console log:

  Console output here:

TO REPRODUCE

Steps to reproduce the behavior:

Perform the MidM attach exploited by Castellucci.

EXPECTED BEHAVIOUR

A clear and concise description of what you expected to happen.

Disable the "old" fingerprint derivation in next Tasmota major release.

SCREENSHOTS

If applicable, add screenshots to help explain your problem.

ADDITIONAL CONTEXT

Add any other context about the problem here.

(Please, remember to close the issue when the problem has been addressed)

[wewa00]

Please assign enhancement label.
And by the way, this issue was already discussed with @s-hadinger at discord.

[issacg]

IMHO This is absolutely ridiculous. There can be devices in the wild already configured with a fingerprint running different versions of Tasmota.

The current implementation since the patch is to auto-update the fingerprint on first use after being updated to latest firmware which will effectively patch the security hole immediately on first use. Removing support for this auto-update will prevent devices in the wild from being able to effectively update over-the-air.

[s-hadinger]

I get your point and looked closer to this new code for fingerprints. I understand your scenario: upgrading from a previous version pre-new algo, to a one that does not support the new algo would make TLS break.

The problem is that the fingerprint in flash has no flag saying if it's against previous or new algo. The key question is whether it is possible to forge a fingerprint using the previous algo to match against a key from new algo. If yes then the vulnerability is not solved.

Anyways, I will not remove support for old fingerprints but make it a compile-time option. We can argue what is the default value, but you are in control in the end. And if you don't want support for the old algo, you can remove it.

[wewa00]

Thanks for the change.

Has anyone contact with Castellucci, so that he could provide a clarification on the security question we have here open?

[ryancdotorg]

@wewa00 @issacg @s-hadinger

I'm not sure why nobody @'d me here, and also please note that I'm a "them", not a "she" or "he".

To answer the question, no, an attacker can't abuse the old fingerprint format support to attack a device that has updated the fingerprint. The attack scenario imagined would require a preimage attack on SHA1. While SHA1 is vulnerable to collision attacks, there is no known vulnerability that would allow a preimage attack.

The old fingerprint format, as originally implemented, encodes the public key ambiguously. As a result, it is possible to create variations on the public key that have the same fingerprint. For a typical RSA key there will be several dozen of them. From empirical testing, in nearly all cases at least one of these variant keys will be possible to crack within a few minutes.

The proper solution is to actually fix the fingerprint, which I also implemented.

In addition to doing that, my patch falls back to the old format, but rejects technically valid RSA keys which have unusual parameters. As a result, there will only be one (or in some obscure edge cases two) possible matching public key under the old fingerprint format. This fallback is a compromise to ensure over-the-air updates don't break† while also preventing exploitation†.

I created a certificate an private key that have the same fingerprint as nsa.gov's certificate at the time because it seemed funny.

Original certificate: nsa.pem.txt

Evil twin with private key: 87552ec2af2ff32aa7bea871b769f7b609fc97a2_e0889_p04.pem.txt

† ...unless somebody changed the key generation parameters to something really weird when creating the certificate.

[s-hadinger]

Thanks @ryancdotorg
This discussion is from 3 years ago, I need to refresh my memory.

I added #define USE_MQTT_TLS_DROP_OLD_FINGERPRINT a while ago to disable moving from previous fingerprints. After 3 yeary, I believe we can now drop this feature and never convert from previous fingerprint formats.

[ryancdotorg]

At this point, the only risk for dropping support is for folks who are compiling in keys and never updated the fingerprints, and for those updating from 8.x.

That said, I think it'd be worth discussing how to go about removing the old code.