openssh unix dev - Apr 2015 - double length prefix in ssh-keygen certificates (values of critical options)

Hi,

I have a question regarding the binary format of the certificates generated
with ssh-keygen, in particular when the critical options of source-address
or force-command are present and the correspondence to the certificate
format specifications such as
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL.certkeys?rev=HEAD
.

It appears that the string values of the source-address and force-command
are prepended with *two* length offsets - 4-byte offset with the integer
value of len(string)+4 followed by the 4-byte offset with the proper
length, and then the string.  Is it a correct behavior?  I could not find
anything in the spec that would prescribe such double-prefixing, or any
description of why of all the strings it is done only for the values of
critical options (not the labels etc.)   The "Critical Options"
section of
the PROTOCOL.certkeys (referenced above) says only the following about the
format for those options:
   string       name
   string       data

so I would expect the "normal" string serialization for both name
(label)
and the data (actual value).  There is also no list or multiple string
values involved - both the source-address and force-command are singe
"flat" strings (source-address can have multiple IPs but they are
comma-separated inside of the same string).

Could it be a bug in ssh-keygen?

When I generate certificates that include such options - for example, with
"ssh-keygen -s ... -O source-address=10.78.72.0/29 -O
force-command=/tmp/foobar" and then decode the generated certificate (awk
'{print $2}' filename-cert.pub | base64 -D | hexdump -C )
I get the following relevant snippet of the dump:

00000190  00 00 00 27 00 00 00 0e  73 6f 75 72 63 65 2d 61
 |...'....source-a|
000001a0  64 64 72 65 73 73 *00 00  00 11 00 00 00 0d *31 30
 |ddress........10|
000001b0  2e 37 38 2e 37 32 2e 30  2f 32 39 00 00 00 82 00
 |.78.72.0/29.....|

highlighted is the double-prefix in question: 00 00  00 11 00 00 00 0d

The same happens with the force-command value.

This apparent deviation (unless I misread the spec, of course!) creates
problems in terms of interoperability with other tools.  Go ssh library (
https://godoc.org/golang.org/x/crypto/ssh), for example, does not do the
"double-wrapping", and as a result, you cannot read the certificates
generated with Go using ssh-keygen -L -f <filename>.  ssh-keygen tries to
read the first 4 bytes of the string value as the second length offset and
of course things quickly go south. Here's a hexdump of the certificate
generated with Go around the critical options section:
00000180  00 00 00 00 00 00 32 00  00 00 00 00 00 00 64 00
 |......2.......d.|
00000190  00 00 43 00 00 00 0d 66  6f 72 63 65 2d 63 6f 6d
 |..C....force-com|
000001a0  6d 61 6e 64 *00 00 00 0b*  2f 74 6d 70 2f 66 6f 6f
 |mand..../tmp/foo|
000001b0  62 61 72 00 00 00 0e 73  6f 75 72 63 65 2d 61 64
 |bar....source-ad|
000001c0  64 72 65 73 73 00 00 00  0d 31 30 2e 37 38 2e 37
 |dress....10.78.7|
000001d0  32 2e 30 2f 32 39 00 00  00 16 00 00 00 0e 70 65
 |2.0/29........pe|

- so before the value of force-command, there is a single length offset 00
00 00 0b, and before the IP address - a single length offset: 00 00 00  0d

ssh-keygen -L on such a Go-generated certificate gives the following error:
Critical Options:
buffer_get_string_ret: bad string length 796159344
buffer_get_string: buffer error

The "bad string length" is easily explained  - the decimal 796159344
is
0x2f746d70 which comes from the bytes 2f 74 6d 70 - "*/tmp*" in the
"/tmp/foobar" string value of the force-command critical option.  If I
hack
the Go ssh library to add an extra prefix with length+4 value, then
ssh-keygen -L is happy again.

Let me know if you agree that it is a bug in ssh-keygen, I'll be happy to
open a Bugzilla ticket.

Thanks,

Dmitry

I believe the double-prefixing in ssh-keygen.c in the add_string_option
function:

cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/ssh-keygen.c?annotate=1.269
                   1453: static void
                   1454: add_string_option(struct sshbuf *c, const char
*name, const char *value)
                   1455: {
                   1456:        struct sshbuf *b;
                   1457:        int r;
                   1458:
                   1459:        debug3("%s: %s=%s", __func__, name,
value);
                   1460:        if ((b = sshbuf_new()) == NULL)
                   1461:                fatal("%s: sshbuf_new failed",
__func__);
                   1462:        if ((r = *sshbuf_put_cstring(b, value))* !0 ||
                   1463:            (r = sshbuf_put_cstring(c, name)) != 0
||
                   1464:            (r = *sshbuf_put_stringb(c, b)*) != 0)
                   1465:                fatal("%s: buffer error: %s",
__func__, ssh_err(r));

First time the value is length-prefixed with the sshbuf_put_cstring call
(line 1462), and then the result ("b") is sent to sshbuf_put_stringb
(line
1464) which treats b (that already has the length prefix) as a string and
prepends the second length field.

The unwrapping of the double length prefix is done in the show_options
function (from line 1795 of ssh-keygen.c).  The first length prefix is
"eaten up" by the sshbuf_froms(options, &option) call on line 1807
in:

                   1806:                if ((r sshbuf_get_cstring(options,
&name, NULL)) != 0 ||
                   1807:                    (r = *sshbuf_froms(options,
&option)*) != 0)

and the second one processed by sshbuf_get_cstring(option, &arg, NULL) a
few lines later (notice that the option structure that was the destination
on line 1807 becomes the source on line 1820):
                   1817:                else if ((v00 || in_critical) &&
                   1818:                    (strcmp(name,
"force-command")
== 0 ||
                   1819:                    strcmp(name,
"source-address")
== 0)) {
                  1820:                        if ((r
*sshbuf_get_cstring(option,
&arg, NULL)*) != 0)
                   1821:                                fatal("%s: buffer
error: %s",
                   1822:                                    __func__,
ssh_err(r));
                   1823:                        printf(" %s\n", arg);


On Thu, Apr 23, 2015 at 11:22 AM, Dmitry Savintsev <dsavints at gmail.com>
wrote:
> Hi,
>
> I have a question regarding the binary format of the certificates
> generated with ssh-keygen, in particular when the critical options of
> source-address or force-command are present and the correspondence to the
> certificate format specifications such as
>
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL.certkeys?rev=HEAD
> .
>
> It appears that the string values of the source-address and force-command
> are prepended with *two* length offsets - 4-byte offset with the integer
> value of len(string)+4 followed by the 4-byte offset with the proper
> length, and then the string.  Is it a correct behavior?  I could not find
> anything in the spec that would prescribe such double-prefixing, or any
> description of why of all the strings it is done only for the values of
> critical options (not the labels etc.)   The "Critical Options"
section of
> the PROTOCOL.certkeys (referenced above) says only the following about the
> format for those options:
>    string       name
>    string       data
>
> so I would expect the "normal" string serialization for both name
(label)
> and the data (actual value).  There is also no list or multiple string
> values involved - both the source-address and force-command are singe
> "flat" strings (source-address can have multiple IPs but they are
> comma-separated inside of the same string).
>
> Could it be a bug in ssh-keygen?
>
> When I generate certificates that include such options - for example, with
> "ssh-keygen -s ... -O source-address=10.78.72.0/29 -O
> force-command=/tmp/foobar" and then decode the generated certificate
(awk
> '{print $2}' filename-cert.pub | base64 -D | hexdump -C )
> I get the following relevant snippet of the dump:
>
> 00000190  00 00 00 27 00 00 00 0e  73 6f 75 72 63 65 2d 61
>  |...'....source-a|
> 000001a0  64 64 72 65 73 73 *00 00  00 11 00 00 00 0d *31 30
>  |ddress........10|
> 000001b0  2e 37 38 2e 37 32 2e 30  2f 32 39 00 00 00 82 00
>  |.78.72.0/29.....|
>
> highlighted is the double-prefix in question: 00 00  00 11 00 00 00 0d
>
> The same happens with the force-command value.
>
> This apparent deviation (unless I misread the spec, of course!) creates
> problems in terms of interoperability with other tools.  Go ssh library (
> https://godoc.org/golang.org/x/crypto/ssh), for example, does not do the
> "double-wrapping", and as a result, you cannot read the
certificates
> generated with Go using ssh-keygen -L -f <filename>.  ssh-keygen
tries to
> read the first 4 bytes of the string value as the second length offset and
> of course things quickly go south. Here's a hexdump of the certificate
> generated with Go around the critical options section:
> 00000180  00 00 00 00 00 00 32 00  00 00 00 00 00 00 64 00
>  |......2.......d.|
> 00000190  00 00 43 00 00 00 0d 66  6f 72 63 65 2d 63 6f 6d
>  |..C....force-com|
> 000001a0  6d 61 6e 64 *00 00 00 0b*  2f 74 6d 70 2f 66 6f 6f
>  |mand..../tmp/foo|
> 000001b0  62 61 72 00 00 00 0e 73  6f 75 72 63 65 2d 61 64
>  |bar....source-ad|
> 000001c0  64 72 65 73 73 00 00 00  0d 31 30 2e 37 38 2e 37
>  |dress....10.78.7|
> 000001d0  32 2e 30 2f 32 39 00 00  00 16 00 00 00 0e 70 65
>  |2.0/29........pe|
>
> - so before the value of force-command, there is a single length offset 00
> 00 00 0b, and before the IP address - a single length offset: 00 00 00  0d
>
> ssh-keygen -L on such a Go-generated certificate gives the following error:
> Critical Options:
> buffer_get_string_ret: bad string length 796159344
> buffer_get_string: buffer error
>
> The "bad string length" is easily explained  - the decimal
796159344 is
> 0x2f746d70 which comes from the bytes 2f 74 6d 70 - "*/tmp*" in
the
> "/tmp/foobar" string value of the force-command critical option. 
If I hack
> the Go ssh library to add an extra prefix with length+4 value, then
> ssh-keygen -L is happy again.
>
> Let me know if you agree that it is a bug in ssh-keygen, I'll be happy
to
> open a Bugzilla ticket.
>
> Thanks,
>
> Dmitry
>
>
>
>

On Thu, 23 Apr 2015, Dmitry Savintsev wrote:
> Hi,
> 
> I have a question regarding the binary format of the certificates generated
> with ssh-keygen, in particular when the critical options of source-address
> or force-command are present and the correspondence to the certificate
> format specifications such as
>
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL.certkeys?rev=HEAD
> .
> 
> It appears that the string values of the source-address and force-command
> are prepended with *two* length offsets - 4-byte offset with the integer
> value of len(string)+4 followed by the 4-byte offset with the proper
> length, and then the string.  Is it a correct behavior?
Yes, as per PROTOCOL.certkeys:
> The critical options section of the certificate specifies zero or more
> options on the certificates validity. The format of this field
> is a sequence of zero or more tuples:
> 
>     string       name
>     string       data
So that's the first header for the data. You then have to look at the
table to determine the format of data's contents. E.g.
> force-command           string        Specifies a command that is executed
...

So, for name=force-command, the contents of the data buffer are a string.
That's the second header.

The intent is to support options/extensions with data types other
than string, e.g. integers or arrays of string.
> This apparent deviation (unless I misread the spec, of course!) creates
> problems in terms of interoperability with other tools.  Go ssh library (
> https://godoc.org/golang.org/x/crypto/ssh), for example, does not do the
> "double-wrapping", and as a result, you cannot read the
certificates
> generated with Go using ssh-keygen -L -f <filename>.  ssh-keygen
tries to
> read the first 4 bytes of the string value as the second length offset and
> of course things quickly go south. Here's a hexdump of the certificate
> generated with Go around the critical options section.
Go's implementation is incorrect here.

Maybe the wording of PROTOCOL.certkeys could be improved to avoid
the confusion, but I'm surprised the Go SSH developers didn't check
against what OpenSSH actually generates (or ask if in doubt).

-d

Thanks Damien for your response and explanations - I opened
https://bugzilla.mindrot.org/show_bug.cgi?id=2389 to improve the wording
the PROTOCOL.certkeys spec.

Currently reading that the format the critical options field is a sequence
of tuples:
>
>     string       name
>     string       data
I would expect both fields to have the same encoding since they have the
same type "string" (and also by analogy with multiple other string
fields
in the specification).  Do you think it would be worth to give the second
field of the tuple some other type like "object" or
"composite" or string[]
- especially since you say the intent is to support "data types other than
string, e.g. integers or arrays of string"?
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL?rev=HEAD&content-type=text/x-cvsweb-markup
uses the type string[] for the list of strings (as in: "    string[]
hostkeys") which makes it clear that the field has different semantics and
encoding than a "plain" string one.

I opened an issue for Go's crypto/ssh library:
https://github.com/golang/go/issues/10569

Thanks,

Dmitry


On Fri, Apr 24, 2015 at 8:11 AM, Damien Miller <djm at mindrot.org> wrote:
> On Thu, 23 Apr 2015, Dmitry Savintsev wrote:
>
> > Hi,
> >
> > I have a question regarding the binary format of the certificates
> generated
> > with ssh-keygen, in particular when the critical options of
> source-address
> > or force-command are present and the correspondence to the certificate
> > format specifications such as
> >
>
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL.certkeys?rev=HEAD
> > .
> >
> > It appears that the string values of the source-address and
force-command
> > are prepended with *two* length offsets - 4-byte offset with the
integer
> > value of len(string)+4 followed by the 4-byte offset with the proper
> > length, and then the string.  Is it a correct behavior?
>
> Yes, as per PROTOCOL.certkeys:
>
> > The critical options section of the certificate specifies zero or more
> > options on the certificates validity. The format of this field
> > is a sequence of zero or more tuples:
> >
> >     string       name
> >     string       data
>
> So that's the first header for the data. You then have to look at the
> table to determine the format of data's contents. E.g.
>
> > force-command           string        Specifies a command that is
> executed
> ...
>
> So, for name=force-command, the contents of the data buffer are a string.
> That's the second header.
>
> The intent is to support options/extensions with data types other
> than string, e.g. integers or arrays of string.
>
> > This apparent deviation (unless I misread the spec, of course!)
creates
> > problems in terms of interoperability with other tools.  Go ssh
library (
> > https://godoc.org/golang.org/x/crypto/ssh), for example, does not do
the
> > "double-wrapping", and as a result, you cannot read the
certificates
> > generated with Go using ssh-keygen -L -f <filename>.  ssh-keygen
tries to
> > read the first 4 bytes of the string value as the second length offset
> and
> > of course things quickly go south. Here's a hexdump of the
certificate
> > generated with Go around the critical options section.
>
> Go's implementation is incorrect here.
>
> Maybe the wording of PROTOCOL.certkeys could be improved to avoid
> the confusion, but I'm surprised the Go SSH developers didn't check
> against what OpenSSH actually generates (or ask if in doubt).
>
> -d
>