Re: [Qemu-devel] [v2 2/5] block/ssh: Add InetSocketAddress and accept it

[Kevin Wolf <kwolf@redhat.com>]

Am 17.10.2016 um 14:33 hat Ashijeet Acharya geschrieben:
> On Sun, Oct 16, 2016 at 4:00 AM, Max Reitz <mreitz@redhat.com> wrote:
> > On 15.10.2016 11:04, Ashijeet Acharya wrote:
> >> Add InetSocketAddress compatibility to SSH driver.
> >>
> >> Add a new option "server" to the SSH block driver which then accepts
> >> a InetSocketAddress.
> >>
> >> "host" and "port" are supported as legacy options and are mapped to
> >> their InetSocketAddress representation.
> >>
> >> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
> >> ---
> >>  block/ssh.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
> >>  1 file changed, 74 insertions(+), 9 deletions(-)
> >>
> >>
> >>      /* Open the socket and connect. */
> >>      s->sock = inet_connect(s->hostport, errp);
> >> @@ -634,7 +698,8 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
> >>      }
> >>
> >>      /* Check the remote host's key against known_hosts. */
> >> -    ret = check_host_key(s, host, port, host_key_check, errp);
> >> +    ret = check_host_key(s, s->inet->host, port, host_key_check,
> >
> > But then you're still using the port here... And I can't come up with a
> > way (not even a bad one) to get the numeric port. Maybe interpret the
> > addrinfo in inet_connect_saddr()? But getting that information out would
> > be ugly, if even possible...
> >
> > So maybe the best is to keep it this way and put a FIXME above the
> > atoi() call. :-/
> 
> Kevin, I believe (after talking with Max) that regarding the atoi()
> issue, I can't use any string to integer function since it won't
> succeed for cases like port = 'ssh' and putting a FIXME over it seems
> to be the only option. But Max did warn me, though, to get everybody's
> opinion before I do so. So I am awaiting your response on this one.
> Much better will be if you have a workaround solution in mind!! :-)

The integer port is only needed for libssh2_knownhost_checkp(). One
option could be to consider passing -1 instead:

    port is the port number used by the host (or a negative number to
    check the generic host). If the port number is given, libssh2 will
    check the key for the specific host + port number combination in
    addition to the plain host name only check.

In 99% of the cases, this shouldn't make any difference.

Alternatively it could be possible to use getservbyname() to get the
port number from the name, but maybe that's a bit too much for a feature
that most people don't even know of.

I'm also not completely opposed to simply requiring a numeric argument
for SSH. There is no real use to support service names here other than
being consistent with other places in qemu.

Kevin