decred
dex/encrypt: resolve deprecation issue for golang.org/x/crypto/poly1305
We use this golang.org/x/crypto package for our encryption/decryption scheme, but I just noticed a scary deprectation warning
"golang.org/x/crypto/poly1305" is deprecated: Poly1305 as implemented by this package is a cryptographic building block that is not safe for general purpose use. For encryption, use the full ChaCha20-Poly1305 construction implemented by golang.org/x/crypto/chacha20poly1305. For authentication, use a general purpose MAC such as HMAC implemented by crypto/hmac.
@buck54321 I'd like to pick this on and add it as a sub-work/follow-up under #2477. I think, that way, we can avoid another seed upgrade. Also, this change will affect server operators as it will change their private key encryption.
Both upgrades can be available in the next 0.6.x release, and then we can safely dump poly1305. What do you think?
that way, we can avoid another seed upgrade
Why would we need any kind of upgrade for this? Should just be subbing out the two functions and one constant we use from the deprecated package. Nothing will change outside of the encrypt package.
Both upgrades can be available in the next 0.6.x release
Neither of the changes will be incorporated in the v0.6 release branch.
OK. I see the wider problem now. The deprecation is a recommendation against using poly1305 for encryption due to its fragility.
https://github.com/golang/go/issues/36646
But we're not using it for encryption, we're using it for message authentication. And while they still recommend using a different HMAC scheme, the primary concerns around fragility are about encryption.
We should upgrade to using a different HMAC. I don't see a reason to roll it into #2477, which has already undergone so much review and is virtually ready for merge.
We you thinking of combining them just to avoid another PrimaryCredentials upgrade? I'm not sure what you mean by "seed upgrade".
I'm also OK just not changing anything. I made a bad choice for HMAC. I can't remember how I made that decision. But if it's not a security issue, why should we change anything?
I was going to use hmac as a replacement for poly1305 and looking at the way Deserialize is used in core and dex server it’ll break existing serialized crypters and I think will require an upgrade.
From the OP https://github.com/golang/go/issues/36646 If anyone is considering using the x/crypto/poly1305 package, they should be dissuaded and pointed towards HMAC, which acts much more like people expect it to. If anyone is using the x/crypto/poly1305 package, they should be notified they are doing something nonstandard and dangerous. I think deprecating the package would be the right way to send that message.
From the OP https://github.com/golang/go/issues/36646 If anyone is considering using the x/crypto/poly1305 package, they should be dissuaded and pointed towards HMAC, which acts much more like people expect it to. If anyone is using the x/crypto/poly1305 package, they should be notified they are doing something nonstandard and dangerous. I think deprecating the package would be the right way to send that message.
Yeah, but we are already using the package. They failed to dissuade us because they didn't even have this conversation until after we had used the package. And again, it's not a security issue for us.
They also say later in the conversation
Yes, to clarify, it would still be totally ok for x/crypto/chacha20poly1305, x/crypto/nacl/secretbox, and x/crypto/ssh to use this package. Deprecation warnings are not meant to be recursive.
I don't think we're going to change anything. Maybe some docs for now.
We actually added a version byte https://github.com/decred/dcrdex/blob/ac979f4cb6dd47dd8f2a2f80f8c4f67ae00845ca/dex/encrypt/encrypt.go#L139 checked at https://github.com/decred/dcrdex/blob/ac979f4cb6dd47dd8f2a2f80f8c4f67ae00845ca/dex/encrypt/encrypt.go#L148-L150
So we could, in theory, just support two algos and everything new would be version 1.
So we could, in theory, just support two algos and everything new would be version 1.
Yes, we could but will want to remove support for poly1305 sometime in the future.