golang
proposal: x/crypto: support OpenSSH variant of ChaCha20Poly1305
What version of Go are you using (go version)?
$ go version go version go1.19.4 linux/amd64
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (go env)?
go env Output
$ go env GO111MODULE="" GOARCH="amd64" GOBIN="" GOCACHE="/home/[REDACTED]/.cache/go-build" GOENV="/home/[REDACTED]/.config/go/env" GOEXE="" GOEXPERIMENT="" GOFLAGS="" GOHOSTARCH="amd64" GOHOSTOS="linux" GOINSECURE="" GOMODCACHE="/opt/dev/go/pkg/mod" GONOPROXY="[REDACTED]" GONOSUMDB="[REDACTED]" GOOS="linux" GOPATH="/opt/dev/go" GOPRIVATE="[REDACTED]" GOPROXY="https://proxy.golang.org,direct" GOROOT="/usr/lib/go" GOSUMDB="sum.golang.org" GOTMPDIR="" GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64" GOVCS="" GOVERSION="go1.19.4" GCCGO="gccgo" GOAMD64="v1" AR="ar" CC="gcc" CXX="g++" CGO_ENABLED="1" GOMOD="/dev/null" GOWORK="" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2" PKG_CONFIG="pkg-config" GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1148312864=/tmp/go-build -gno-record-gcc-switches"
What did you do?
This isn't so much what I did, but rather something the x/crypto team did. Understandably so, but nonetheless limiting.
With the implementation of #36646, it is now no longer possible to generate and modify [email protected]-encrypted OpenSSH keypairs in a low/direct manner. (AES-encrypted keys are, of course, quite easy to modify and generate as long as the new key structure is understood.)
That is, one cannot create chacha20poly1305-encrypted OpenSSH private keys without vendoring/modifying ChaCha20 and Poly1305. Which then means they are highly susceptible to be not kept in-line with upstream (x/crypto)'s implementation, thus defeating a part of the intention of removing the public components in the first place as outlined in the original discussion.
What did you expect to see?
I expect to see an ability to use a cipher.AEAD with [email protected] (which uses a 16-byte nonce for ChaCha20 instead of a 12 or 24-byte nonce).
What did you see instead?
This functionality is not possible without vendoring or maintaining a separate fork of, at the least:
x/crypto/chacha20x/crypto/poly1305x/crypto/internal/poly1305(unless implemented in fork of the above directly)
Add'l Notes (EDIT)
I received an email notification that someone commented inquiring if (golang.org/x/crypto/chacha20poly1305).New can be used but it appears it has been removed as it seems they understood the issue after commenting. :)
To avoid others making the same mistake, no, chacha20poly1305.New cannot be used because it uses a 12-byte nonce (and uses the RFC-specified padding), both of which are incompatible with the OpenSSH variant. Likewise, chacha20poly1305.NewX cannot be used because it uses a 24-byte nonce (and the RFC-specified padding), which also is incompatible with the OpenSSH variant. OpenSSH implements a 16-byte nonce and a "proprietary" padding mechanism using sequential uint8 integers. This is why their cipher implementation is named [email protected] instead of chacha20poly1305.
For more information/technical details:
- https://datatracker.ietf.org/doc/html/draft-josefsson-ssh-chacha20-poly1305-openssh-00
- https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.chacha20poly1305
- https://github.com/openssh/openssh-portable/blob/4a5590a5ee47b7dfd49773e9fdba48ad3089fe64/cipher.c#L106-L107 (struct defined here )
- https://github.com/openssh/openssh-portable/blob/master/sshkey.c
It sounds like this is essentially a proposal to add a new API, say NewOpenSSH, which returns a cipher.AEAD that uses a 16 byte nonce.
Given it's no worse than the standard (smaller) nonce size, and it provides support for a deployed construction (OpenSSH choosing the middle of the road size between standard and X was a fun choice), I have no real opposition.
Can someone write down the proposed new API with its doc comment? Thanks.
Probably this:
// NonceSizeOpenSSH is the size of the nonce used with the [email protected] variant of this
// AEAD, in bytes
NonceSizeOpenSSH = 16
// NewOpenSSH returns a [email protected] AEAD that uses the given 256-bit key.
//
// [email protected] is a ChaCha20-Poly1305 variant that uses a non-standard nonce size,
// suitable for usage with OpenSSH.
func NewOpenSSH(key []byte) (cipher.AEAD, error)
I also need this. My workaround is to copy the source into my personal ssh module, which is a sub-optimal outcome. Indeed poly1305 is a footgun, but as used by ssh it is ok. For the crypto library to hide it entirely, preventing independent implementation of ssh, seems peculiar.
Are we talking about the same nonce? I allocate a 12-byte nonce and for OpenSSH compatibility use a 4-byte packet counter and zero-fill.
This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — rsc for the proposal review group
Perhaps we can just move poly1305 to crypto/subtle instead of crypto/internal?
On Wed, Sep 4, 2024, 11:45 Russ Cox @.***> wrote:
This proposal has been added to the active column https://go.dev/s/proposal-status#active of the proposals project and will now be reviewed at the weekly proposal review meetings. — rsc for the proposal review group
— Reply to this email directly, view it on GitHub https://github.com/golang/go/issues/57699#issuecomment-2329746453, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACADPOSOXPD5ODLGZK4TUKDZU5IKZAVCNFSM6AAAAABNGNG762VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMRZG42DMNBVGM . You are receiving this because you commented.Message ID: @.***>
It sounds like there are few enough of these variants that we should add them all explicitly instead of exposing primitives like poly1305 and leaving people to connect them correctly. https://github.com/golang/go/issues/57699#issuecomment-1379343978 seems fine except that we need to resolve whether we're talking about two different variants, or maybe three (nonce lengths 12, 16, 24).
The existence of an "original" variant of ChaCha20Poly1305 and an IETF variant is unfortunate, but nearly everything and everyone converged on the IETF variant, and I wouldn't want to contribute to rekindling that mess.
Besides, my understanding is that the "original" variant has a 8 bytes (64 bit) nonce, not a 16 bytes nonce. https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.chacha20poly1305 and https://github.com/golang/crypto/blob/9e92970a1eb41e446822e037016aa89d24c0ce7a/ssh/cipher.go#L669 seems to agree with that. Where is the 16 bytes nonce size coming from?
Taking a step back, why do you need to use this variant of the AEAD directly? You mention generating encrypted OpenSSH keypairs, maybe we should just extend the ssh.MarshalPrivateKeyWithPassphrase API, maybe as part of #68723 which is already adding a struct for marshal parameters?
Finally, x/crypto/poly1305 is Deprecated, not removed, so it's not the case that it is no longer possible to use it. If you know what you are doing enough to want to use a non-standard AEAD to do low-level operations on an encrypted key, using a deprecated package doesn't sound wrong.
Yes, I was also puzzled about the OP comment about 16 byte nonce. Perhaps he can clarify.
I may have misunderstood your proposal with respect to marking deprecated versus moving to internal. As long as crypto/ssh/cipher.go interoperates with OpenSSH and does not import crypto/internal then fine.
As long as crypto/ssh/cipher.go interoperates with OpenSSH and does not import crypto/internal then fine.
x/crypto/ssh may move to the standard library, at which point who knows what it will import. My point is that your ssh module can keep importing x/crypto/poly1305 even if it is deprecated.
I withdraw my request. For ssh, I'll revert to aes-gcm. For other contexts, if aes is too heavyweight, I'll select ascon80pq.
My understanding is that, while x/crypto/poly1305 is deprecated (owing to the potential for misusing the API), it's still usable as part of the implementation of the ssh protocol if needed. Since it sounds like this proposal isn't needed, I'm going to move it toward decline.
Based on the discussion above, this proposal seems like a likely decline. — aclements for the proposal review group
