openssh unix dev - May 2017 - Golang CertChecker hostname validation differs to OpenSSH

On Mon, May 15, 2017 at 11:39 AM, Peter Moody <mindrot at hda3.com>
wrote:
> my reading of the sshd manpage is that ssh is more permissive than it
should be
>
> SSH_KNOWN_HOSTS FILE FORMAT :
>   ...
>
>   A hostname or address may optionally be enclosed within `[' and
`]'
> brackets then followed by `:' and a non-standard port number.
Hi Peter, I'm not sure that quite answers the same question.

ie at one level there is a decision that is made about whether a line
in the known hosts file should be evaluated for a given host/port -
and I think that's what you are referring to above.

However once a line from known hosts is allowed for evaluation for a
host/port, there's a second matter of checking whether the certificate
presented contains the appropriate principal.

I think this what "check_host_cert()" does, and as far as I can tell,
OpenSSH only passes it the hostname (not "host:port"). See:
https://github.com/openssh/openssh-portable/blob/f382362e8dfb6b277f16779ab1936399d7f2af78/sshconnect.c#L866

(for better or for worse, this would be roughly inline with X.509v3
cert host matching, which also doesn't match on port numbers)

Adam Eijdenberg wrote:
> I think this what "check_host_cert()" does, and as far as I can
tell,
> OpenSSH only passes it the hostname (not "host:port").
> 
> (for better or for worse, this would be roughly inline with X.509v3
> cert host matching, which also doesn't match on port numbers)
If possible OpenSSH IMO should not reproduce this particular deficiency of the
TLS
hostname check.

Ciao, Michael.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3829 bytes
Desc: S/MIME Cryptographic Signature
URL:
<http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20170515/07c65840/attachment-0001.bin>

On Mon, May 15, 2017 at 2:01 AM, Adam Eijdenberg <adam at continusec.com>
wrote:
> On Mon, May 15, 2017 at 11:39 AM, Peter Moody <mindrot at hda3.com>
wrote:
>> my reading of the sshd manpage is that ssh is more permissive than it
should be
>>
>> SSH_KNOWN_HOSTS FILE FORMAT :
>>   ...
>>
>>   A hostname or address may optionally be enclosed within `[' and
`]'
>> brackets then followed by `:' and a non-standard port number.
>
> Hi Peter, I'm not sure that quite answers the same question.
>
> ie at one level there is a decision that is made about whether a line
> in the known hosts file should be evaluated for a given host/port -
> and I think that's what you are referring to above.
>
> However once a line from known hosts is allowed for evaluation for a
> host/port, there's a second matter of checking whether the certificate
> presented contains the appropriate principal.
>
> I think this what "check_host_cert()" does, and as far as I can
tell,
> OpenSSH only passes it the hostname (not "host:port"). See:
>
https://github.com/openssh/openssh-portable/blob/f382362e8dfb6b277f16779ab1936399d7f2af78/sshconnect.c#L866
>
> (for better or for worse, this would be roughly inline with X.509v3
> cert host matching, which also doesn't match on port numbers)
possibly.

your proposed patch removes both checks though. I think you'd want to
modify knownhosts.go if you want to support not including non-standard
ports in IsHostAuthority.

Note, you can also write your own IsHostAuthority that ignores the
port, I think this just affects the HostKeyCallback provided by
golang.org/x/crypto/ssh/knownhosts.

On May 15, 2017 09:38, "Peter Moody" <mindrot at hda3.com>
wrote:

On Mon, May 15, 2017 at 2:01 AM, Adam Eijdenberg <adam at continusec.com>
wrote:
> On Mon, May 15, 2017 at 11:39 AM, Peter Moody <mindrot at hda3.com>
wrote:
>> my reading of the sshd manpage is that ssh is more permissive than it
should be
>>
>> SSH_KNOWN_HOSTS FILE FORMAT :
>>   ...
>>
>>   A hostname or address may optionally be enclosed within `[' and
`]'
>> brackets then followed by `:' and a non-standard port number.
>
> Hi Peter, I'm not sure that quite answers the same question.
>
> ie at one level there is a decision that is made about whether a line
> in the known hosts file should be evaluated for a given host/port -
> and I think that's what you are referring to above.
>
> However once a line from known hosts is allowed for evaluation for a
> host/port, there's a second matter of checking whether the certificate
> presented contains the appropriate principal.
>
> I think this what "check_host_cert()" does, and as far as I can
tell,
> OpenSSH only passes it the hostname (not "host:port"). See:
> https://github.com/openssh/openssh-portable/blob/
f382362e8dfb6b277f16779ab1936399d7f2af78/sshconnect.c#L866
>
> (for better or for worse, this would be roughly inline with X.509v3
> cert host matching, which also doesn't match on port numbers)
possibly.

your proposed patch removes both checks though. I think you'd want to
modify knownhosts.go if you want to support not including non-standard
ports in IsHostAuthority.

Note, you can also write your own IsHostAuthority that ignores the
port, I think this just affects the HostKeyCallback provided by
golang.org/x/crypto/ssh/knownhosts.


I could be wrong about that though, I'm about I to jump on an airplane and
I haven't inspected it closely.

On Tue, May 16, 2017 at 2:38 AM, Peter Moody <mindrot at hda3.com>
wrote:
> your proposed patch removes both checks though. I think you'd want to
> modify knownhosts.go if you want to support not including non-standard
> ports in IsHostAuthority.
My intention wasn't to modify both checks - I'm currently only
concerned with principal checking, although I can see how your recent
patch (as implemented) would also be affected (so if we do change
anything here, we'll probably need to refactor a little).

Let me give a concrete example, currently our certificates (OpenSSH
server, and OpenSSH client) look like this and everything works great:

Principals:
        auth.example.local
        auth.example.com.au

However, if I write a Go client (which requires a port number be
specified in their Dial string):

    log.Println(ssh.Dial("tcp", "auth.example.local:10000",
&ssh.ClientConfig{
        HostKeyCallback: (&ssh.CertChecker{}).CheckHostKey,
    }))

I get the following error, before even attempting to evaluating
IsHostAuthority():

    ssh: handshake failed: ssh: principal "auth.example.local:10000"
not in the set of valid principals for given certificate:
["auth.example.local" "auth.example.com.au"]


If I want a certificate to work with OpenSSH server, and both Go and
OpenSSH clients, I need to re-generate a certificate like this:

Principals:
        auth.example.local
        auth.example.com.au
        auth.example.local:10000
        auth.example.com.au:10000


That doesn't seem right, and I think the Go principal evaluation is
incorrect, but I would like a second opinion.

(that code in Go also seems to be at least 3 years old)