diff mbox

[v1,20/22] migration: support TLS encryption with TCP migration backend

Message ID 1452599056-27357-21-git-send-email-berrange@redhat.com
State New
Headers show

Commit Message

Daniel P. Berrangé Jan. 12, 2016, 11:44 a.m. UTC
This extends the TCP migration backend so that it can make use
of QIOChannelTLS to provide transparent TLS encryption. To
trigger enablement the URI on the incoming and outgoing sides
should have 'tls-creds=ID' appended, eg

   tcp:$HOST:$PORT,tls-creds=ID

where ID is the object identifier of a set of TLS credentials
previously created using object_add / -object. There is not
currently any support for checking the migration client
certificate names against ACLs. This is pending a conversion
of the ACL code to QOM.

There is no support for dynamically negotiating use of TLS
between the incoming/outgoing side. Both sides must agree
on use of TLS out of band and set the URI accordingly. In
practice it is expected that the administrator will just
turn on use of TLS on their hosts in the libvirt config
and then libvirt will instruct QEMU to use TLS for migration.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 migration/tcp.c | 273 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 qemu-options.hx |   7 +-
 2 files changed, 264 insertions(+), 16 deletions(-)

Comments

Dr. David Alan Gilbert Feb. 12, 2016, 5:09 p.m. UTC | #1
* Daniel P. Berrange (berrange@redhat.com) wrote:
> This extends the TCP migration backend so that it can make use
> of QIOChannelTLS to provide transparent TLS encryption. To
> trigger enablement the URI on the incoming and outgoing sides
> should have 'tls-creds=ID' appended, eg
> 
>    tcp:$HOST:$PORT,tls-creds=ID
> 
> where ID is the object identifier of a set of TLS credentials
> previously created using object_add / -object. There is not
> currently any support for checking the migration client
> certificate names against ACLs. This is pending a conversion
> of the ACL code to QOM.

Does that change the option passed or is that just different
in the way the tls-creds are set up?

> There is no support for dynamically negotiating use of TLS
> between the incoming/outgoing side. Both sides must agree
> on use of TLS out of band and set the URI accordingly. In
> practice it is expected that the administrator will just
> turn on use of TLS on their hosts in the libvirt config
> and then libvirt will instruct QEMU to use TLS for migration.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

> ---
>  migration/tcp.c | 273 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  qemu-options.hx |   7 +-
>  2 files changed, 264 insertions(+), 16 deletions(-)
> 
> diff --git a/migration/tcp.c b/migration/tcp.c
> index ac73977..bef861c 100644
> --- a/migration/tcp.c
> +++ b/migration/tcp.c
> @@ -22,6 +22,8 @@
>  #include "migration/migration.h"
>  #include "migration/qemu-file.h"
>  #include "io/channel-socket.h"
> +#include "io/channel-tls.h"
> +#include "crypto/tlscreds.h"
>  
>  //#define DEBUG_MIGRATION_TCP
>  
> @@ -33,6 +35,22 @@
>      do { } while (0)
>  #endif
>  
> +typedef struct {
> +    MigrationState *s;
> +    QCryptoTLSCreds *tlscreds;
> +    char *hostname;
> +} TCPConnectData;
> +
> +typedef struct {
> +    MigrationState *s;
> +    QCryptoTLSCreds *tlscreds;
> +} TCPListenData;
> +
> +typedef struct {
> +    MigrationState *s;
> +    QIOChannel *ioc;
> +} TCPConnectTLSData;
> +

what makes it TCP specific rather than sharing most of this between transports? i.e. should
this work for fd migration ? (rdma is probably not reasonable
giving it's scribbling directly in the other hosts RAM).
Certainly those types dont really look TCP specific.

>  static SocketAddress *tcp_build_address(const char *host_port, Error **errp)
>  {
> @@ -51,21 +69,174 @@ static SocketAddress *tcp_build_address(const char *host_port, Error **errp)
>  }
>  
>  
> +static void tcp_connect_data_free(gpointer opaque)
> +{
> +    TCPConnectData *data = opaque;
> +    if (!data) {
> +        return;
> +    }
> +    g_free(data->hostname);
> +    object_unref(OBJECT(data->tlscreds));
> +    g_free(data);
> +}
> +
> +
> +static void tcp_listen_data_free(gpointer opaque)
> +{
> +    TCPListenData *data = opaque;
> +    if (!data) {
> +        return;
> +    }
> +    object_unref(OBJECT(data->tlscreds));
> +    g_free(data);
> +}
> +
> +
> +static void tcp_connect_tls_data_free(gpointer opaque)
> +{
> +    TCPConnectTLSData *data = opaque;
> +    if (!data) {
> +        return;
> +    }
> +    object_unref(OBJECT(data->ioc));
> +    g_free(data);
> +}
> +
> +
> +static char *tcp_get_opt_str(const char *host_port,
> +                             const char *key)
> +{
> +    const char *offset, *end;
> +
> +    offset = strstr(host_port, key);
> +    if (!offset) {
> +        return NULL;
> +    }
> +
> +    offset += strlen(key);
> +    if (offset[0] != '=') {
> +        return NULL;
> +    }
> +    offset++;
> +    end = strchr(offset, ',');
> +    if (end) {
> +        return g_strndup(offset, end - offset);
> +    } else {
> +        return g_strdup(offset);
> +    }
> +}
> +
> +
> +static QCryptoTLSCreds *tcp_get_tls_creds(const char *host_port,
> +                                          bool is_listen,
> +                                          Error **errp)
> +{
> +    char *credname = NULL;
> +    Object *creds;
> +    QCryptoTLSCreds *ret;
> +
> +    credname = tcp_get_opt_str(host_port, "tls-creds");
> +    if (!credname) {
> +        return NULL;
> +    }

At what point does it get saner just to throw host_port into a qemu_opts 
and let it parse it?

> +    creds = object_resolve_path_component(
> +        object_get_objects_root(), credname);
> +    if (!creds) {
> +        error_setg(errp, "No TLS credentials with id '%s'",
> +                   credname);
> +        goto error;
> +    }
> +    ret = (QCryptoTLSCreds *)object_dynamic_cast(
> +        creds, TYPE_QCRYPTO_TLS_CREDS);
> +    if (!ret) {
> +        error_setg(errp, "Object with id '%s' is not TLS credentials",
> +                   credname);
> +        goto error;
> +    }
> +    if (is_listen) {
> +        if (ret->endpoint != QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
> +            error_setg(errp, "%s",
> +                       "Expected TLS credentials for server endpoint");
> +            goto error;
> +        }
> +    } else {
> +        if (ret->endpoint != QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT) {
> +            error_setg(errp, "%s",
> +                       "Expected TLS credentials for client endpoint");
> +            goto error;
> +        }
> +    }
> +
> +    g_free(credname);
> +    object_ref(OBJECT(ret));
> +    return ret;
> +
> + error:
> +    g_free(credname);
> +    return NULL;
> +}
> +
> +
> +static void tcp_outgoing_migration_tls(Object *src,
> +                                       Error *err,
> +                                       gpointer opaque)
> +{
> +    TCPConnectTLSData *data = opaque;
> +    QIOChannel *ioc = QIO_CHANNEL(src);
> +
> +    if (err) {
> +        DPRINTF("migrate connect TLS error: %s\n", error_get_pretty(err));

error_report ?

> +        data->s->file = NULL;
> +        migrate_fd_error(data->s);
> +    } else {
> +        DPRINTF("migrate connect success\n");

trace_

> +        data->s->file = qemu_fopen_channel_output(ioc);
> +        migrate_fd_connect(data->s);
> +    }
> +}
> +
> +
>  static void tcp_outgoing_migration(Object *src,
>                                     Error *err,
>                                     gpointer opaque)
>  {
> -    MigrationState *s = opaque;
> +    TCPConnectData *data = opaque;
>      QIOChannel *sioc = QIO_CHANNEL(src);
>  
>      if (err) {
>          DPRINTF("migrate connect error: %s\n", error_get_pretty(err));
> -        s->file = NULL;
> -        migrate_fd_error(s);
> +        data->s->file = NULL;
> +        migrate_fd_error(data->s);
>      } else {
> -        DPRINTF("migrate connect success\n");
> -        s->file = qemu_fopen_channel_output(sioc);
> -        migrate_fd_connect(s);
> +        if (data->tlscreds) {
> +            Error *local_err = NULL;
> +            QIOChannelTLS *tioc = qio_channel_tls_new_client(
> +                sioc, data->tlscreds, data->hostname,
> +                &local_err);
> +            if (!tioc) {
> +                DPRINTF("migrate tls setup error: %s\n",
> +                        error_get_pretty(local_err));

error_report ?  More below I think - just make sure that
any errors normally get logged.

> +                error_free(local_err);
> +                data->s->file = NULL;
> +                migrate_fd_error(data->s);
> +            } else {
> +                TCPConnectTLSData *tdata =
> +                    g_new0(TCPConnectTLSData, 1);
> +                DPRINTF("migrate connect tls handshake begin\n");
> +                tdata->s = data->s;
> +                tdata->ioc = sioc;
> +                object_ref(OBJECT(sioc));
> +                qio_channel_tls_handshake(tioc,
> +                                          tcp_outgoing_migration_tls,
> +                                          tdata,
> +                                          tcp_connect_tls_data_free);
> +            }
> +        } else {
> +            DPRINTF("migrate connect success\n");
> +            data->s->file = qemu_fopen_channel_output(sioc);
> +            migrate_fd_connect(data->s);
> +        }
>      }
>      object_unref(src);
>  }
> @@ -77,21 +248,56 @@ void tcp_start_outgoing_migration(MigrationState *s,
>  {
>      SocketAddress *saddr = tcp_build_address(host_port, errp);
>      QIOChannelSocket *sioc;
> +    Error *local_err = NULL;
> +    QCryptoTLSCreds *creds;
> +    TCPConnectData *data;
>  
>      if (!saddr) {
>          return;
>      }
>  
> +    creds = tcp_get_tls_creds(host_port, false, errp);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        qapi_free_SocketAddress(saddr);
> +        return;
> +    }
> +
> +    data = g_new0(TCPConnectData, 1);
> +    data->s = s;
> +    if (creds) {
> +        data->hostname = g_strdup(saddr->u.inet->host);
> +        data->tlscreds = creds;
> +    }
> +
>      sioc = qio_channel_socket_new();
>      qio_channel_socket_connect_async(sioc,
>                                       saddr,
>                                       tcp_outgoing_migration,
> -                                     s,
> -                                     NULL);
> +                                     data,
> +                                     tcp_connect_data_free);
>      qapi_free_SocketAddress(saddr);
>  }
>  
>  
> +static void tcp_incoming_migration_tls(Object *src,
> +                                       Error *err,
> +                                       gpointer opaque)
> +{
> +    QIOChannel *ioc = QIO_CHANNEL(src);
> +
> +    if (err) {
> +        DPRINTF("migrate listen TLS error: %s\n", error_get_pretty(err));
> +        object_unref(OBJECT(ioc));
> +    } else {
> +        DPRINTF("migrate listen success\n");
> +        QEMUFile *f = qemu_fopen_channel_input(ioc);
> +
> +        process_incoming_migration(f);
> +    }
> +}
> +
> +
>  static gboolean tcp_accept_incoming_migration(QIOChannel *ioc,
>                                                GIOCondition condition,
>                                                gpointer opaque)
> @@ -99,6 +305,7 @@ static gboolean tcp_accept_incoming_migration(QIOChannel *ioc,
>      QEMUFile *f;
>      QIOChannelSocket *cioc;
>      Error *err = NULL;
> +    TCPListenData *data = opaque;
>  
>      cioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc),
>                                       &err);
> @@ -108,16 +315,38 @@ static gboolean tcp_accept_incoming_migration(QIOChannel *ioc,
>          goto out;
>      }
>  
> -    DPRINTF("accepted migration\n");
> +    if (data->tlscreds) {
> +        DPRINTF("Starting client TLS\n");
> +        QIOChannelTLS *tioc = qio_channel_tls_new_server(
> +            QIO_CHANNEL(cioc), data->tlscreds,
> +            NULL, /* XXX pass ACL name */
> +            &err);
> +        object_unref(OBJECT(cioc));
> +        if (!tioc) {
> +            DPRINTF("migrate tls setup error: %s\n",
> +                    error_get_pretty(err));
> +            error_free(err);
> +            goto out;
> +        } else {
> +            DPRINTF("migrate connect tls handshake begin\n");
> +            qio_channel_tls_handshake(tioc,
> +                                      tcp_incoming_migration_tls,
> +                                      NULL,
> +                                      NULL);
> +        }
> +    } else {
> +        DPRINTF("accepted migration\n");
>  
> -    f = qemu_fopen_channel_input(QIO_CHANNEL(cioc));
> -    object_unref(OBJECT(cioc));
> +        f = qemu_fopen_channel_input(QIO_CHANNEL(cioc));
> +        object_unref(OBJECT(cioc));
>  
> -    process_incoming_migration(f);
> +        process_incoming_migration(f);
> +    }
>  
>  out:
>      /* Close listening socket as its no longer needed */
>      qio_channel_close(ioc, NULL);
> +    object_unref(OBJECT(ioc));
>      return FALSE;
>  }
>  
> @@ -126,23 +355,39 @@ void tcp_start_incoming_migration(const char *host_port, Error **errp)
>  {
>      SocketAddress *saddr = tcp_build_address(host_port, errp);
>      QIOChannelSocket *listen_ioc;
> +    TCPListenData *data;
> +    Error *local_err = NULL;
> +    QCryptoTLSCreds *creds;
>  
>      if (!saddr) {
>          return;
>      }
>  
> +    creds = tcp_get_tls_creds(host_port, true, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        qapi_free_SocketAddress(saddr);
> +        return;
> +    }
> +
>      listen_ioc = qio_channel_socket_new();
>      if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) {
>          object_unref(OBJECT(listen_ioc));
>          qapi_free_SocketAddress(saddr);
> +        object_unref(OBJECT(creds));
>          return;
>      }
>  
> +    data = g_new0(TCPListenData, 1);
> +    if (creds) {
> +        data->tlscreds = creds;
> +    }
> +
>      qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
>                            G_IO_IN,
>                            tcp_accept_incoming_migration,
> -                          listen_ioc,
> -                          (GDestroyNotify)object_unref);
> +                          data,
> +                          tcp_listen_data_free);
>  
>      qapi_free_SocketAddress(saddr);
>  }
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 215d00d..3f16cfc 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3364,7 +3364,7 @@ Set TB size.
>  ETEXI
>  
>  DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \
> -    "-incoming tcp:[host]:port[,to=maxport][,ipv4][,ipv6]\n" \
> +    "-incoming tcp:[host]:port[,to=maxport][,ipv4][,ipv6][,tls-creds=ID]\n" \
>      "-incoming rdma:host:port[,ipv4][,ipv6]\n" \
>      "-incoming unix:socketpath\n" \
>      "                prepare for incoming migration, listen on\n" \
> @@ -3377,11 +3377,14 @@ DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \
>      "                wait for the URI to be specified via migrate_incoming\n",
>      QEMU_ARCH_ALL)
>  STEXI
> -@item -incoming tcp:[@var{host}]:@var{port}[,to=@var{maxport}][,ipv4][,ipv6]
> +@item -incoming tcp:[@var{host}]:@var{port}[,to=@var{maxport}][,ipv4][,ipv6][,tls-creds=ID]
>  @itemx -incoming rdma:@var{host}:@var{port}[,ipv4][,ipv6]
>  @findex -incoming
>  Prepare for incoming migration, listen on a given tcp port.
>  
> +If the @var{tls-creds} parameter is specified, it should refer to the ID
> +of a TLS credentials object previously created with @var{-object}.
> +
>  @item -incoming unix:@var{socketpath}
>  Prepare for incoming migration, listen on a given unix socket.
>  
> -- 
> 2.5.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Daniel P. Berrangé Feb. 12, 2016, 5:25 p.m. UTC | #2
On Fri, Feb 12, 2016 at 05:09:52PM +0000, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrange (berrange@redhat.com) wrote:
> > This extends the TCP migration backend so that it can make use
> > of QIOChannelTLS to provide transparent TLS encryption. To
> > trigger enablement the URI on the incoming and outgoing sides
> > should have 'tls-creds=ID' appended, eg
> > 
> >    tcp:$HOST:$PORT,tls-creds=ID
> > 
> > where ID is the object identifier of a set of TLS credentials
> > previously created using object_add / -object. There is not
> > currently any support for checking the migration client
> > certificate names against ACLs. This is pending a conversion
> > of the ACL code to QOM.
> 
> Does that change the option passed or is that just different
> in the way the tls-creds are set up?

It will mean another new paramter will be added. For example
the above command will become something like this:

   tcp:$HOST:$PORT,tls-creds=ID,auth=ID

where 'auth=ID' provides the ID of an object implementing
the authentication/authorization check

> > +typedef struct {
> > +    MigrationState *s;
> > +    QCryptoTLSCreds *tlscreds;
> > +    char *hostname;
> > +} TCPConnectData;
> > +
> > +typedef struct {
> > +    MigrationState *s;
> > +    QCryptoTLSCreds *tlscreds;
> > +} TCPListenData;
> > +
> > +typedef struct {
> > +    MigrationState *s;
> > +    QIOChannel *ioc;
> > +} TCPConnectTLSData;
> > +
> 
> what makes it TCP specific rather than sharing most of this between transports? i.e. should
> this work for fd migration ? (rdma is probably not reasonable
> giving it's scribbling directly in the other hosts RAM).
> Certainly those types dont really look TCP specific.

TLS as a protocol doesn't have any limitations, but as part of having
the client validate the x509 certificates, it needs to have a hostname
or IP address to validate against the certificate. This is available
for TCP and RDMA, but there's no user provided hostname for unix,
exec, and fd migration protocols.

We could extend the syntax for each of those, so that the user could
provide a hostname, and that would then allow us to wire up TLS for
that backend. If we did that, then it would make sense to have the
TLS setup in a separate migration/tls.c file, that we could activate
over all channels.

> > +static QCryptoTLSCreds *tcp_get_tls_creds(const char *host_port,
> > +                                          bool is_listen,
> > +                                          Error **errp)
> > +{
> > +    char *credname = NULL;
> > +    Object *creds;
> > +    QCryptoTLSCreds *ret;
> > +
> > +    credname = tcp_get_opt_str(host_port, "tls-creds");
> > +    if (!credname) {
> > +        return NULL;
> > +    }
> 
> At what point does it get saner just to throw host_port into a qemu_opts 
> and let it parse it?

Possibly quite soon :-)

> > +static void tcp_outgoing_migration_tls(Object *src,
> > +                                       Error *err,
> > +                                       gpointer opaque)
> > +{
> > +    TCPConnectTLSData *data = opaque;
> > +    QIOChannel *ioc = QIO_CHANNEL(src);
> > +
> > +    if (err) {
> > +        DPRINTF("migrate connect TLS error: %s\n", error_get_pretty(err));
> 
> error_report ?

Is it desirable for code triggered from the QMP/HMP monitor to be
doing that. I thought we were not supposed todo that in general.
Normally we could propagate an error back to the monitor, but
since migrate happens in the background, we've no way todo that.
We really need to be able to feed the errors back to the client
app over QMP somehow, but there's no mechanism for that right
now :-(

Perhaps 'info migrate' should have a field to report the actual
error message when status == failed ?  Or we could define an
async event that we emit to the client with the actual error.
Either would make life nicer for libvirt, which can't report
any good errors - this is the main reason libvirt currently
does all TCP connection setup itself and uses fd:, and not
tcp:.  Libvirt will need to use tcp: again though, to get
TLS working, unless we add a 'hostname' field to the 'fd:'
protocol....

> > +        data->s->file = NULL;
> > +        migrate_fd_error(data->s);
> > +    } else {
> > +        DPRINTF("migrate connect success\n");
> 
> trace_

Yep, I mentioned in a repl to a previous review, that I
intend to do a single cleanup patch at the end which
adds some consistent trace points across all the backend
drivers for migrate.

> > +        data->s->file = qemu_fopen_channel_output(ioc);
> > +        migrate_fd_connect(data->s);
> > +    }
> > +}
> > +
> > +
> >  static void tcp_outgoing_migration(Object *src,
> >                                     Error *err,
> >                                     gpointer opaque)
> >  {
> > -    MigrationState *s = opaque;
> > +    TCPConnectData *data = opaque;
> >      QIOChannel *sioc = QIO_CHANNEL(src);
> >  
> >      if (err) {
> >          DPRINTF("migrate connect error: %s\n", error_get_pretty(err));
> > -        s->file = NULL;
> > -        migrate_fd_error(s);
> > +        data->s->file = NULL;
> > +        migrate_fd_error(data->s);
> >      } else {
> > -        DPRINTF("migrate connect success\n");
> > -        s->file = qemu_fopen_channel_output(sioc);
> > -        migrate_fd_connect(s);
> > +        if (data->tlscreds) {
> > +            Error *local_err = NULL;
> > +            QIOChannelTLS *tioc = qio_channel_tls_new_client(
> > +                sioc, data->tlscreds, data->hostname,
> > +                &local_err);
> > +            if (!tioc) {
> > +                DPRINTF("migrate tls setup error: %s\n",
> > +                        error_get_pretty(local_err));
> 
> error_report ?  More below I think - just make sure that
> any errors normally get logged.

Same issue as above.


Regards,
Daniel
Daniel P. Berrangé Feb. 15, 2016, 11 a.m. UTC | #3
On Fri, Feb 12, 2016 at 05:25:31PM +0000, Daniel P. Berrange wrote:
> On Fri, Feb 12, 2016 at 05:09:52PM +0000, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrange (berrange@redhat.com) wrote:
> > > This extends the TCP migration backend so that it can make use
> > > of QIOChannelTLS to provide transparent TLS encryption. To
> > > trigger enablement the URI on the incoming and outgoing sides
> > > should have 'tls-creds=ID' appended, eg
> > > 
> > >    tcp:$HOST:$PORT,tls-creds=ID
> > > 
> > > where ID is the object identifier of a set of TLS credentials
> > > previously created using object_add / -object. There is not
> > > currently any support for checking the migration client
> > > certificate names against ACLs. This is pending a conversion
> > > of the ACL code to QOM.
> > 
> > Does that change the option passed or is that just different
> > in the way the tls-creds are set up?
> 
> It will mean another new paramter will be added. For example
> the above command will become something like this:
> 
>    tcp:$HOST:$PORT,tls-creds=ID,auth=ID
> 
> where 'auth=ID' provides the ID of an object implementing
> the authentication/authorization check
> 
> > > +typedef struct {
> > > +    MigrationState *s;
> > > +    QCryptoTLSCreds *tlscreds;
> > > +    char *hostname;
> > > +} TCPConnectData;
> > > +
> > > +typedef struct {
> > > +    MigrationState *s;
> > > +    QCryptoTLSCreds *tlscreds;
> > > +} TCPListenData;
> > > +
> > > +typedef struct {
> > > +    MigrationState *s;
> > > +    QIOChannel *ioc;
> > > +} TCPConnectTLSData;
> > > +
> > 
> > what makes it TCP specific rather than sharing most of this between transports? i.e. should
> > this work for fd migration ? (rdma is probably not reasonable
> > giving it's scribbling directly in the other hosts RAM).
> > Certainly those types dont really look TCP specific.
> 
> TLS as a protocol doesn't have any limitations, but as part of having
> the client validate the x509 certificates, it needs to have a hostname
> or IP address to validate against the certificate. This is available
> for TCP and RDMA, but there's no user provided hostname for unix,
> exec, and fd migration protocols.
> 
> We could extend the syntax for each of those, so that the user could
> provide a hostname, and that would then allow us to wire up TLS for
> that backend. If we did that, then it would make sense to have the
> TLS setup in a separate migration/tls.c file, that we could activate
> over all channels.
> 
> > > +static QCryptoTLSCreds *tcp_get_tls_creds(const char *host_port,
> > > +                                          bool is_listen,
> > > +                                          Error **errp)
> > > +{
> > > +    char *credname = NULL;
> > > +    Object *creds;
> > > +    QCryptoTLSCreds *ret;
> > > +
> > > +    credname = tcp_get_opt_str(host_port, "tls-creds");
> > > +    if (!credname) {
> > > +        return NULL;
> > > +    }
> > 
> > At what point does it get saner just to throw host_port into a qemu_opts 
> > and let it parse it?
> 
> Possibly quite soon :-)

So, having thought about this some more, I think rather than munging
parameters onto the end of the URI, it'll make more sense to use the
'migrate-set-parameters' QMP call ie. to enable use of tls

  migrate-set-parameters tls-creds=tls0

and then to deal with the problem I mention above about not having a
hostname available for fd/exec migration, we can also allow

  migrate-set-parameters tls-hostname=peerhostname

which would set the hostname to be used to validate the x509 certificate.
This would be quite nice for libvirt, since we can carry on using the
fd: migration and establish the connection ourselves, while letting QEMU
do the x509 validation.

This would in turn motivate moving of the TLS IO Channel creation into
a separate file, instead of having it inline in tcp.c. This would in
turn let me address the feedback you had previous about possibility of
unix: and tcp: code being dealt with in the same file to avoid the
code duplication.

Regards,
Daniel
diff mbox

Patch

diff --git a/migration/tcp.c b/migration/tcp.c
index ac73977..bef861c 100644
--- a/migration/tcp.c
+++ b/migration/tcp.c
@@ -22,6 +22,8 @@ 
 #include "migration/migration.h"
 #include "migration/qemu-file.h"
 #include "io/channel-socket.h"
+#include "io/channel-tls.h"
+#include "crypto/tlscreds.h"
 
 //#define DEBUG_MIGRATION_TCP
 
@@ -33,6 +35,22 @@ 
     do { } while (0)
 #endif
 
+typedef struct {
+    MigrationState *s;
+    QCryptoTLSCreds *tlscreds;
+    char *hostname;
+} TCPConnectData;
+
+typedef struct {
+    MigrationState *s;
+    QCryptoTLSCreds *tlscreds;
+} TCPListenData;
+
+typedef struct {
+    MigrationState *s;
+    QIOChannel *ioc;
+} TCPConnectTLSData;
+
 
 static SocketAddress *tcp_build_address(const char *host_port, Error **errp)
 {
@@ -51,21 +69,174 @@  static SocketAddress *tcp_build_address(const char *host_port, Error **errp)
 }
 
 
+static void tcp_connect_data_free(gpointer opaque)
+{
+    TCPConnectData *data = opaque;
+    if (!data) {
+        return;
+    }
+    g_free(data->hostname);
+    object_unref(OBJECT(data->tlscreds));
+    g_free(data);
+}
+
+
+static void tcp_listen_data_free(gpointer opaque)
+{
+    TCPListenData *data = opaque;
+    if (!data) {
+        return;
+    }
+    object_unref(OBJECT(data->tlscreds));
+    g_free(data);
+}
+
+
+static void tcp_connect_tls_data_free(gpointer opaque)
+{
+    TCPConnectTLSData *data = opaque;
+    if (!data) {
+        return;
+    }
+    object_unref(OBJECT(data->ioc));
+    g_free(data);
+}
+
+
+static char *tcp_get_opt_str(const char *host_port,
+                             const char *key)
+{
+    const char *offset, *end;
+
+    offset = strstr(host_port, key);
+    if (!offset) {
+        return NULL;
+    }
+
+    offset += strlen(key);
+    if (offset[0] != '=') {
+        return NULL;
+    }
+    offset++;
+    end = strchr(offset, ',');
+    if (end) {
+        return g_strndup(offset, end - offset);
+    } else {
+        return g_strdup(offset);
+    }
+}
+
+
+static QCryptoTLSCreds *tcp_get_tls_creds(const char *host_port,
+                                          bool is_listen,
+                                          Error **errp)
+{
+    char *credname = NULL;
+    Object *creds;
+    QCryptoTLSCreds *ret;
+
+    credname = tcp_get_opt_str(host_port, "tls-creds");
+    if (!credname) {
+        return NULL;
+    }
+
+    creds = object_resolve_path_component(
+        object_get_objects_root(), credname);
+    if (!creds) {
+        error_setg(errp, "No TLS credentials with id '%s'",
+                   credname);
+        goto error;
+    }
+    ret = (QCryptoTLSCreds *)object_dynamic_cast(
+        creds, TYPE_QCRYPTO_TLS_CREDS);
+    if (!ret) {
+        error_setg(errp, "Object with id '%s' is not TLS credentials",
+                   credname);
+        goto error;
+    }
+    if (is_listen) {
+        if (ret->endpoint != QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
+            error_setg(errp, "%s",
+                       "Expected TLS credentials for server endpoint");
+            goto error;
+        }
+    } else {
+        if (ret->endpoint != QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT) {
+            error_setg(errp, "%s",
+                       "Expected TLS credentials for client endpoint");
+            goto error;
+        }
+    }
+
+    g_free(credname);
+    object_ref(OBJECT(ret));
+    return ret;
+
+ error:
+    g_free(credname);
+    return NULL;
+}
+
+
+static void tcp_outgoing_migration_tls(Object *src,
+                                       Error *err,
+                                       gpointer opaque)
+{
+    TCPConnectTLSData *data = opaque;
+    QIOChannel *ioc = QIO_CHANNEL(src);
+
+    if (err) {
+        DPRINTF("migrate connect TLS error: %s\n", error_get_pretty(err));
+        data->s->file = NULL;
+        migrate_fd_error(data->s);
+    } else {
+        DPRINTF("migrate connect success\n");
+        data->s->file = qemu_fopen_channel_output(ioc);
+        migrate_fd_connect(data->s);
+    }
+}
+
+
 static void tcp_outgoing_migration(Object *src,
                                    Error *err,
                                    gpointer opaque)
 {
-    MigrationState *s = opaque;
+    TCPConnectData *data = opaque;
     QIOChannel *sioc = QIO_CHANNEL(src);
 
     if (err) {
         DPRINTF("migrate connect error: %s\n", error_get_pretty(err));
-        s->file = NULL;
-        migrate_fd_error(s);
+        data->s->file = NULL;
+        migrate_fd_error(data->s);
     } else {
-        DPRINTF("migrate connect success\n");
-        s->file = qemu_fopen_channel_output(sioc);
-        migrate_fd_connect(s);
+        if (data->tlscreds) {
+            Error *local_err = NULL;
+            QIOChannelTLS *tioc = qio_channel_tls_new_client(
+                sioc, data->tlscreds, data->hostname,
+                &local_err);
+            if (!tioc) {
+                DPRINTF("migrate tls setup error: %s\n",
+                        error_get_pretty(local_err));
+                error_free(local_err);
+                data->s->file = NULL;
+                migrate_fd_error(data->s);
+            } else {
+                TCPConnectTLSData *tdata =
+                    g_new0(TCPConnectTLSData, 1);
+                DPRINTF("migrate connect tls handshake begin\n");
+                tdata->s = data->s;
+                tdata->ioc = sioc;
+                object_ref(OBJECT(sioc));
+                qio_channel_tls_handshake(tioc,
+                                          tcp_outgoing_migration_tls,
+                                          tdata,
+                                          tcp_connect_tls_data_free);
+            }
+        } else {
+            DPRINTF("migrate connect success\n");
+            data->s->file = qemu_fopen_channel_output(sioc);
+            migrate_fd_connect(data->s);
+        }
     }
     object_unref(src);
 }
@@ -77,21 +248,56 @@  void tcp_start_outgoing_migration(MigrationState *s,
 {
     SocketAddress *saddr = tcp_build_address(host_port, errp);
     QIOChannelSocket *sioc;
+    Error *local_err = NULL;
+    QCryptoTLSCreds *creds;
+    TCPConnectData *data;
 
     if (!saddr) {
         return;
     }
 
+    creds = tcp_get_tls_creds(host_port, false, errp);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        qapi_free_SocketAddress(saddr);
+        return;
+    }
+
+    data = g_new0(TCPConnectData, 1);
+    data->s = s;
+    if (creds) {
+        data->hostname = g_strdup(saddr->u.inet->host);
+        data->tlscreds = creds;
+    }
+
     sioc = qio_channel_socket_new();
     qio_channel_socket_connect_async(sioc,
                                      saddr,
                                      tcp_outgoing_migration,
-                                     s,
-                                     NULL);
+                                     data,
+                                     tcp_connect_data_free);
     qapi_free_SocketAddress(saddr);
 }
 
 
+static void tcp_incoming_migration_tls(Object *src,
+                                       Error *err,
+                                       gpointer opaque)
+{
+    QIOChannel *ioc = QIO_CHANNEL(src);
+
+    if (err) {
+        DPRINTF("migrate listen TLS error: %s\n", error_get_pretty(err));
+        object_unref(OBJECT(ioc));
+    } else {
+        DPRINTF("migrate listen success\n");
+        QEMUFile *f = qemu_fopen_channel_input(ioc);
+
+        process_incoming_migration(f);
+    }
+}
+
+
 static gboolean tcp_accept_incoming_migration(QIOChannel *ioc,
                                               GIOCondition condition,
                                               gpointer opaque)
@@ -99,6 +305,7 @@  static gboolean tcp_accept_incoming_migration(QIOChannel *ioc,
     QEMUFile *f;
     QIOChannelSocket *cioc;
     Error *err = NULL;
+    TCPListenData *data = opaque;
 
     cioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc),
                                      &err);
@@ -108,16 +315,38 @@  static gboolean tcp_accept_incoming_migration(QIOChannel *ioc,
         goto out;
     }
 
-    DPRINTF("accepted migration\n");
+    if (data->tlscreds) {
+        DPRINTF("Starting client TLS\n");
+        QIOChannelTLS *tioc = qio_channel_tls_new_server(
+            QIO_CHANNEL(cioc), data->tlscreds,
+            NULL, /* XXX pass ACL name */
+            &err);
+        object_unref(OBJECT(cioc));
+        if (!tioc) {
+            DPRINTF("migrate tls setup error: %s\n",
+                    error_get_pretty(err));
+            error_free(err);
+            goto out;
+        } else {
+            DPRINTF("migrate connect tls handshake begin\n");
+            qio_channel_tls_handshake(tioc,
+                                      tcp_incoming_migration_tls,
+                                      NULL,
+                                      NULL);
+        }
+    } else {
+        DPRINTF("accepted migration\n");
 
-    f = qemu_fopen_channel_input(QIO_CHANNEL(cioc));
-    object_unref(OBJECT(cioc));
+        f = qemu_fopen_channel_input(QIO_CHANNEL(cioc));
+        object_unref(OBJECT(cioc));
 
-    process_incoming_migration(f);
+        process_incoming_migration(f);
+    }
 
 out:
     /* Close listening socket as its no longer needed */
     qio_channel_close(ioc, NULL);
+    object_unref(OBJECT(ioc));
     return FALSE;
 }
 
@@ -126,23 +355,39 @@  void tcp_start_incoming_migration(const char *host_port, Error **errp)
 {
     SocketAddress *saddr = tcp_build_address(host_port, errp);
     QIOChannelSocket *listen_ioc;
+    TCPListenData *data;
+    Error *local_err = NULL;
+    QCryptoTLSCreds *creds;
 
     if (!saddr) {
         return;
     }
 
+    creds = tcp_get_tls_creds(host_port, true, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        qapi_free_SocketAddress(saddr);
+        return;
+    }
+
     listen_ioc = qio_channel_socket_new();
     if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) {
         object_unref(OBJECT(listen_ioc));
         qapi_free_SocketAddress(saddr);
+        object_unref(OBJECT(creds));
         return;
     }
 
+    data = g_new0(TCPListenData, 1);
+    if (creds) {
+        data->tlscreds = creds;
+    }
+
     qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
                           G_IO_IN,
                           tcp_accept_incoming_migration,
-                          listen_ioc,
-                          (GDestroyNotify)object_unref);
+                          data,
+                          tcp_listen_data_free);
 
     qapi_free_SocketAddress(saddr);
 }
diff --git a/qemu-options.hx b/qemu-options.hx
index 215d00d..3f16cfc 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3364,7 +3364,7 @@  Set TB size.
 ETEXI
 
 DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \
-    "-incoming tcp:[host]:port[,to=maxport][,ipv4][,ipv6]\n" \
+    "-incoming tcp:[host]:port[,to=maxport][,ipv4][,ipv6][,tls-creds=ID]\n" \
     "-incoming rdma:host:port[,ipv4][,ipv6]\n" \
     "-incoming unix:socketpath\n" \
     "                prepare for incoming migration, listen on\n" \
@@ -3377,11 +3377,14 @@  DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \
     "                wait for the URI to be specified via migrate_incoming\n",
     QEMU_ARCH_ALL)
 STEXI
-@item -incoming tcp:[@var{host}]:@var{port}[,to=@var{maxport}][,ipv4][,ipv6]
+@item -incoming tcp:[@var{host}]:@var{port}[,to=@var{maxport}][,ipv4][,ipv6][,tls-creds=ID]
 @itemx -incoming rdma:@var{host}:@var{port}[,ipv4][,ipv6]
 @findex -incoming
 Prepare for incoming migration, listen on a given tcp port.
 
+If the @var{tls-creds} parameter is specified, it should refer to the ID
+of a TLS credentials object previously created with @var{-object}.
+
 @item -incoming unix:@var{socketpath}
 Prepare for incoming migration, listen on a given unix socket.