x/crypto/ssh - interoperability with SSH encoding of certificates with critical options (values encoding) #10569
Comments
dmitris
changed the title
|
sent a CL https://go-review.googlesource.com/#/c/9375/ Would it be OK to break parsing of certificates generated with the previous / "buggy" version of crypto/ssh? Do we need to introduce some sort of backward compatibility? We can "sniff" the type of certiicate (old vs new) by checking if the second 4-byte group has (the value of the first 4-byte)-4, if yes, consider it "new" = "fixed" format, otherwise "old" and "broken". If an "old format" certificate is detected, we could give a warning asking to regenerate certificates with the new version and verify them with ssh-keygen (which does not work with the "old" format if critical options are involved). But maybe it affects very small group of people and it is OK to give a parsing error? If so, maybe it would be worth to give a heads-up to golang-dev and golang-nuts (something like "[ANN] certificate format modified in the upcoming release of x/crypto/ssh")? |
|
Fixed by golang/crypto@5943553 |
Attention - BREAKING change for the certificates generated with the previous versions of crypto/ssh! Need to regenerate certificates with a version of crypto/ssh library including this fix. [PROTOCOL.cerkeys] requires two length fields for non-empty values of critical options (or extensions - but those are currently always empty) - see https://bugzilla.mindrot.org/show_bug.cgi?id=2389. Add SSH-conform handling of such composite values in marshalTuples and parseTuples and related test (TestParseCertWithOptions) parsing a certificate created with ssh-keygen which includes critical options. Fixes golang#10569 Change-Id: Iecbfca67a66668880635141c72bc5fc370a9c112 Reviewed-on: https://go-review.googlesource.com/9375 Reviewed-by: Adam Langley <agl@golang.org> Run-TryBot: Adam Langley <agl@golang.org>
Currently the Go ssh library generates certificates that cannot be read by OpenSSH "ssh-keygen -L" command if any of the critical options ("force-command" or "source-address" or both) are included in the certificate. See http://lists.mindrot.org/pipermail/openssh-unix-dev/2015-April/033849.html and http://lists.mindrot.org/pipermail/openssh-unix-dev/2015-April/033844.html for more details and examples/hexdumps. In summary, Go's ssh library encodes the values of the critical options ("data" in the <name, data> tuples of the PROTOCOL.certkeys spec http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL.certkeys?rev=HEAD, ) as a plain string (with one length prefix) but SSH expects a "double-prefixed" string - data is supposed to be a composite field that itself contains a string, therefore with two length prefixes [1st: len(s)+4, 2nd: len(s)]. When "ssh-keygen -L" attempts to read a Go-generated certificate with critical options, it ends up up reading the first four bytes of the string proper as the expected second length prefix and obviously fails ("string too large").
The other direction - from certificates generated with "ssh-keygen -s " to Go's ssh unmarshaling - happens to work (due to the reflection unwrapping magic).
in http://lists.mindrot.org/pipermail/openssh-unix-dev/2015-April/033849.html Damien Miller says that "Go's implementation is incorrect here" but also admits that the PROTOCOL.certkeys spec could be improved to prevent confusion (and I would say confusion is very easy here based on my reading of the spec and talking to several experienced engineers). I opened https://bugzilla.mindrot.org/show_bug.cgi?id=2389 to improve the spec wording in that part.
We should also add tests to generate and validate certificates that include critical options. Besides verifying interoperability with SSH, they would also be useful as examples and bootstarting help for developers using that functionality.
The text was updated successfully, but these errors were encountered: