ianlancetaylor
commented
9 years ago @agl
Closed dmitris closed 9 years ago
ianlancetaylor
commented
9 years ago @agl
dmitris
commented
9 years ago 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")?
dmitris
commented
9 years ago
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.