* Re: [Qemu-devel] [PATCH 1/1] chardev: add support for authorization for TLS clients
2019-02-27 13:55 ` [Qemu-devel] [PATCH 1/1] chardev: add support for authorization for TLS clients Daniel P. Berrangé
@ 2019-02-27 14:29 ` Philippe Mathieu-Daudé
2019-03-07 18:51 ` Marc-André Lureau
1 sibling, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-02-27 14:29 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Paolo Bonzini, Markus Armbruster, Marc-André Lureau
On 2/27/19 2:55 PM, Daniel P. Berrangé wrote:
> From: "Daniel P. Berrange" <berrange@redhat.com>
>
> Currently any client which can complete the TLS handshake is able to use
> a chardev server. The server admin can turn on the 'verify-peer' option
> for the x509 creds to require the client to provide a x509
> certificate. This means the client will have to acquire a certificate
> from the CA before they are permitted to use the chardev server. This is
> still a fairly low bar.
>
> This adds a 'tls-authz=OBJECT-ID' option to the socket chardev backend
> which takes the ID of a previously added 'QAuthZ' object instance. This
> will be used to validate the client's x509 distinguished name. Clients
> failing the check will not be permitted to use the chardev server.
>
> For example to setup authorization that only allows connection from a
> client whose x509 certificate distinguished name contains 'CN=fred', you
> would use:
>
> $QEMU -object tls-creds-x509,id=tls0,dir=/home/berrange/qemutls,\
> endpoint=server,verify-peer=yes \
> -object authz-simple,id=authz0,identity=CN=laptop.example.com,,\
> O=Example Org,,L=London,,ST=London,,C=GB \
> -chardev socket,host=127.0.0.1,port=9000,server,\
> tls-creds=tls0,tls-authz=authz0 \
> ...other qemu args...
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> chardev/char-socket.c | 12 +++++++++++-
> chardev/char.c | 3 +++
> qapi/char.json | 6 ++++++
> qemu-options.hx | 9 +++++++--
> 4 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 4fcdd8aedd..12e78426aa 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -59,6 +59,7 @@ typedef struct {
> QIONetListener *listener;
> GSource *hup_source;
> QCryptoTLSCreds *tls_creds;
> + char *tls_authz;
> TCPChardevState state;
> int max_size;
> int do_telnetopt;
> @@ -807,7 +808,7 @@ static void tcp_chr_tls_init(Chardev *chr)
> if (s->is_listen) {
> tioc = qio_channel_tls_new_server(
> s->ioc, s->tls_creds,
> - NULL, /* XXX Use an ACL */
> + s->tls_authz,
> &err);
> } else {
> tioc = qio_channel_tls_new_client(
> @@ -1055,6 +1056,7 @@ static void char_socket_finalize(Object *obj)
> if (s->tls_creds) {
> object_unref(OBJECT(s->tls_creds));
> }
> + g_free(s->tls_authz);
>
> qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
> }
> @@ -1242,6 +1244,11 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock,
> break;
> }
>
> + if (sock->has_tls_authz && !sock->has_tls_creds) {
> + error_setg(errp, "'tls_authz' option requires 'tls_creds' option");
> + return false;
> + }
> +
> /* Validate any options which have a dependancy on client vs server */
> if (!sock->has_server || sock->server) {
> if (sock->has_reconnect) {
> @@ -1320,6 +1327,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
> }
> }
> }
> + s->tls_authz = g_strdup(sock->tls_authz);
>
> s->addr = addr = socket_address_flatten(sock->addr);
>
> @@ -1399,6 +1407,8 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
> sock->reconnect = qemu_opt_get_number(opts, "reconnect", 0);
> sock->has_tls_creds = qemu_opt_get(opts, "tls-creds");
> sock->tls_creds = g_strdup(qemu_opt_get(opts, "tls-creds"));
> + sock->has_tls_authz = qemu_opt_get(opts, "tls-authz");
> + sock->tls_authz = g_strdup(qemu_opt_get(opts, "tls-authz"));
>
> addr = g_new0(SocketAddressLegacy, 1);
> if (path) {
> diff --git a/chardev/char.c b/chardev/char.c
> index f6d61fa5f8..514cd6b0c3 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -880,6 +880,9 @@ QemuOptsList qemu_chardev_opts = {
> },{
> .name = "tls-creds",
> .type = QEMU_OPT_STRING,
> + },{
> + .name = "tls-authz",
> + .type = QEMU_OPT_STRING,
> },{
> .name = "websocket",
> .type = QEMU_OPT_BOOL,
> diff --git a/qapi/char.json b/qapi/char.json
> index 77ed847972..7847c98d05 100644
> --- a/qapi/char.json
> +++ b/qapi/char.json
> @@ -248,6 +248,11 @@
> # @addr: socket address to listen on (server=true)
> # or connect to (server=false)
> # @tls-creds: the ID of the TLS credentials object (since 2.6)
> +# @tls-authz: the ID of the QAuthZ authorization object against which
> +# the client's x509 distinguished name will validated. This
> +# object is only resolved at time of use, so can be deleted
> +# and recreated on the fly while the chardev server is active.
> +# If missing, it will default to denying access (since 4.0)
> # @server: create server socket (default: true)
> # @wait: wait for incoming connection on server
> # sockets (default: false).
> @@ -268,6 +273,7 @@
> { 'struct': 'ChardevSocket',
> 'data': { 'addr': 'SocketAddressLegacy',
> '*tls-creds': 'str',
> + '*tls-authz' : 'str',
> '*server': 'bool',
> '*wait': 'bool',
> '*nodelay': 'bool',
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 1cf9aac1fe..aecbb2ca9c 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2413,7 +2413,7 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
> "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4][,ipv6][,nodelay][,reconnect=seconds]\n"
> " [,server][,nowait][,telnet][,websocket][,reconnect=seconds][,mux=on|off]\n"
> - " [,logfile=PATH][,logappend=on|off][,tls-creds=ID] (tcp)\n"
> + " [,logfile=PATH][,logappend=on|off][,tls-creds=ID][,tls-authz=ID] (tcp)\n"
> "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,websocket][,reconnect=seconds]\n"
> " [,mux=on|off][,logfile=PATH][,logappend=on|off] (unix)\n"
> "-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n"
> @@ -2542,7 +2542,7 @@ The available backends are:
> A void device. This device will not emit any data, and will drop any data it
> receives. The null backend does not take any options.
>
> -@item -chardev socket,id=@var{id}[,@var{TCP options} or @var{unix options}][,server][,nowait][,telnet][,websocket][,reconnect=@var{seconds}][,tls-creds=@var{id}]
> +@item -chardev socket,id=@var{id}[,@var{TCP options} or @var{unix options}][,server][,nowait][,telnet][,websocket][,reconnect=@var{seconds}][,tls-creds=@var{id}][,tls-authz=@var{id}]
>
> Create a two-way stream socket, which can be either a TCP or a unix socket. A
> unix socket will be created if @option{path} is specified. Behaviour is
> @@ -2568,6 +2568,11 @@ and specifies the id of the TLS credentials to use for the handshake. The
> credentials must be previously created with the @option{-object tls-creds}
> argument.
>
> +@option{tls-auth} provides the ID of the QAuthZ authorization object against
> +which the client's x509 distinguished name will validated. This object is only
> +resolved at time of use, so can be deleted and recreated on the fly while the
> +chardev server is active. If missing, it will default to denying access.
> +
> TCP and unix socket options are given below:
>
> @table @option
>
LGTM:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH 1/1] chardev: add support for authorization for TLS clients
2019-02-27 13:55 ` [Qemu-devel] [PATCH 1/1] chardev: add support for authorization for TLS clients Daniel P. Berrangé
2019-02-27 14:29 ` Philippe Mathieu-Daudé
@ 2019-03-07 18:51 ` Marc-André Lureau
2019-03-07 19:00 ` Eric Blake
1 sibling, 1 reply; 6+ messages in thread
From: Marc-André Lureau @ 2019-03-07 18:51 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Markus Armbruster, Paolo Bonzini, Eric Blake
Hi
On Wed, Feb 27, 2019 at 2:55 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> From: "Daniel P. Berrange" <berrange@redhat.com>
>
> Currently any client which can complete the TLS handshake is able to use
> a chardev server. The server admin can turn on the 'verify-peer' option
> for the x509 creds to require the client to provide a x509
> certificate. This means the client will have to acquire a certificate
> from the CA before they are permitted to use the chardev server. This is
> still a fairly low bar.
>
> This adds a 'tls-authz=OBJECT-ID' option to the socket chardev backend
> which takes the ID of a previously added 'QAuthZ' object instance. This
> will be used to validate the client's x509 distinguished name. Clients
> failing the check will not be permitted to use the chardev server.
>
> For example to setup authorization that only allows connection from a
> client whose x509 certificate distinguished name contains 'CN=fred', you
> would use:
>
> $QEMU -object tls-creds-x509,id=tls0,dir=/home/berrange/qemutls,\
> endpoint=server,verify-peer=yes \
> -object authz-simple,id=authz0,identity=CN=laptop.example.com,,\
> O=Example Org,,L=London,,ST=London,,C=GB \
> -chardev socket,host=127.0.0.1,port=9000,server,\
> tls-creds=tls0,tls-authz=authz0 \
> ...other qemu args...
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> chardev/char-socket.c | 12 +++++++++++-
> chardev/char.c | 3 +++
> qapi/char.json | 6 ++++++
> qemu-options.hx | 9 +++++++--
> 4 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 4fcdd8aedd..12e78426aa 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -59,6 +59,7 @@ typedef struct {
> QIONetListener *listener;
> GSource *hup_source;
> QCryptoTLSCreds *tls_creds;
> + char *tls_authz;
> TCPChardevState state;
> int max_size;
> int do_telnetopt;
> @@ -807,7 +808,7 @@ static void tcp_chr_tls_init(Chardev *chr)
> if (s->is_listen) {
> tioc = qio_channel_tls_new_server(
> s->ioc, s->tls_creds,
> - NULL, /* XXX Use an ACL */
> + s->tls_authz,
> &err);
> } else {
> tioc = qio_channel_tls_new_client(
> @@ -1055,6 +1056,7 @@ static void char_socket_finalize(Object *obj)
> if (s->tls_creds) {
> object_unref(OBJECT(s->tls_creds));
> }
> + g_free(s->tls_authz);
>
> qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
> }
> @@ -1242,6 +1244,11 @@ static bool qmp_chardev_validate_socket(ChardevSocket *sock,
> break;
> }
>
> + if (sock->has_tls_authz && !sock->has_tls_creds) {
> + error_setg(errp, "'tls_authz' option requires 'tls_creds' option");
> + return false;
> + }
> +
> /* Validate any options which have a dependancy on client vs server */
> if (!sock->has_server || sock->server) {
> if (sock->has_reconnect) {
> @@ -1320,6 +1327,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
> }
> }
> }
> + s->tls_authz = g_strdup(sock->tls_authz);
>
> s->addr = addr = socket_address_flatten(sock->addr);
>
> @@ -1399,6 +1407,8 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
> sock->reconnect = qemu_opt_get_number(opts, "reconnect", 0);
> sock->has_tls_creds = qemu_opt_get(opts, "tls-creds");
> sock->tls_creds = g_strdup(qemu_opt_get(opts, "tls-creds"));
> + sock->has_tls_authz = qemu_opt_get(opts, "tls-authz");
> + sock->tls_authz = g_strdup(qemu_opt_get(opts, "tls-authz"));
>
> addr = g_new0(SocketAddressLegacy, 1);
> if (path) {
> diff --git a/chardev/char.c b/chardev/char.c
> index f6d61fa5f8..514cd6b0c3 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -880,6 +880,9 @@ QemuOptsList qemu_chardev_opts = {
> },{
> .name = "tls-creds",
> .type = QEMU_OPT_STRING,
> + },{
> + .name = "tls-authz",
> + .type = QEMU_OPT_STRING,
> },{
> .name = "websocket",
> .type = QEMU_OPT_BOOL,
> diff --git a/qapi/char.json b/qapi/char.json
> index 77ed847972..7847c98d05 100644
> --- a/qapi/char.json
> +++ b/qapi/char.json
> @@ -248,6 +248,11 @@
> # @addr: socket address to listen on (server=true)
> # or connect to (server=false)
> # @tls-creds: the ID of the TLS credentials object (since 2.6)
> +# @tls-authz: the ID of the QAuthZ authorization object against which
> +# the client's x509 distinguished name will validated. This
will be? (not a native speaker, but sounds weird to me)
> +# object is only resolved at time of use, so can be deleted
> +# and recreated on the fly while the chardev server is active.
> +# If missing, it will default to denying access (since 4.0)
> # @server: create server socket (default: true)
> # @wait: wait for incoming connection on server
> # sockets (default: false).
> @@ -268,6 +273,7 @@
> { 'struct': 'ChardevSocket',
> 'data': { 'addr': 'SocketAddressLegacy',
> '*tls-creds': 'str',
> + '*tls-authz' : 'str',
> '*server': 'bool',
> '*wait': 'bool',
> '*nodelay': 'bool',
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 1cf9aac1fe..aecbb2ca9c 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2413,7 +2413,7 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
> "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
> "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4][,ipv6][,nodelay][,reconnect=seconds]\n"
> " [,server][,nowait][,telnet][,websocket][,reconnect=seconds][,mux=on|off]\n"
> - " [,logfile=PATH][,logappend=on|off][,tls-creds=ID] (tcp)\n"
> + " [,logfile=PATH][,logappend=on|off][,tls-creds=ID][,tls-authz=ID] (tcp)\n"
> "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,websocket][,reconnect=seconds]\n"
> " [,mux=on|off][,logfile=PATH][,logappend=on|off] (unix)\n"
> "-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n"
> @@ -2542,7 +2542,7 @@ The available backends are:
> A void device. This device will not emit any data, and will drop any data it
> receives. The null backend does not take any options.
>
> -@item -chardev socket,id=@var{id}[,@var{TCP options} or @var{unix options}][,server][,nowait][,telnet][,websocket][,reconnect=@var{seconds}][,tls-creds=@var{id}]
> +@item -chardev socket,id=@var{id}[,@var{TCP options} or @var{unix options}][,server][,nowait][,telnet][,websocket][,reconnect=@var{seconds}][,tls-creds=@var{id}][,tls-authz=@var{id}]
>
> Create a two-way stream socket, which can be either a TCP or a unix socket. A
> unix socket will be created if @option{path} is specified. Behaviour is
> @@ -2568,6 +2568,11 @@ and specifies the id of the TLS credentials to use for the handshake. The
> credentials must be previously created with the @option{-object tls-creds}
> argument.
>
> +@option{tls-auth} provides the ID of the QAuthZ authorization object against
> +which the client's x509 distinguished name will validated. This object is only
same
> +resolved at time of use, so can be deleted and recreated on the fly while the
> +chardev server is active. If missing, it will default to denying access.
Why not have "(since 4.0)" here?
> +
> TCP and unix socket options are given below:
>
> @table @option
> --
> 2.20.1
>
Other than that,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread