search

ProtonMail / go-crypto

Fork of go/x/crypto, providing an up-to-date OpenPGP implementation
https://pkg.go.dev/github.com/ProtonMail/go-crypto
BSD 3-Clause "New" or "Revised" License
334 stars 101 forks source link

Re-introduce "support" for decrypting messages without MDC #129

Closed aksdb closed 1 year ago

aksdb commented 2 years ago

For a while now (~2018), the decryption of symmetrically encrypted packets without MDC (Tag 9) has been disabled.

While this follows the recommendation in the current IETF Crypto Refresh Draft, it can cause problems when interacting with old clients. I think it's valid to block these interactions by default, but would like to propose offering an opening for cases where this can't be avoided.

An easy possible fix might be to introduce a package wide global variable that toggles support for MDC (and defaults to "not allowed"). When set to "allowed", this check would be rendered ineffective.

I would even more prefer the ability to decide about this on a per-decryption basis (vs a global variable), but at the point where this check is made, there is no access to any configuration passed from the outside and refactoring that would probably not be worth it for such a fringe case.

WDYT about allowing decryption of messages without MDC again (via opt-in)?

twiss commented 2 years ago

Hello :wave: Sorry for the delay - I'm off this week so may be slow to respond.

First off, a nitpick, as I said in #101, that PR wasn't what disabled reading messages without MDC, that was done much longer ago.

That being said, having an option to allow it seems reasonable. However, I would prefer not to have a global variable for this. Adding a config option in config.go seems better.

aksdb commented 2 years ago

Ah dammit I am sorry. I missed that this guard was just moved there. I only saw the added lines and therefore it looked like a fresh addition.

At a first look it also seemed like I have no way to access the config from the place where parsing/reading takes place, without a refactoring that would change the signature of the Read function.

I'll dive into this again to check if there is a better way than in my current PR.

aksdb commented 2 years ago

Ah of course the check can be pulled a few layers up, then it should work in relation to the config.

aksdb commented 2 years ago

I updated my PR accordingly.

twiss commented 1 year ago

(Closed by #130.)