[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] docs: update information for TLS certificate
From: |
Kashyap Chamarthy |
Subject: |
Re: [Qemu-devel] [PATCH v2] docs: update information for TLS certificate management |
Date: |
Fri, 26 Jan 2018 13:53:09 +0100 |
User-agent: |
NeoMutt/20171215 |
On Thu, Jan 25, 2018 at 05:09:30PM +0000, Daniel P. Berrangé wrote:
> From: "Daniel P. Berrange" <address@hidden>
[...]
> address@hidden vnc_setup_sasl
> +
> address@hidden Configuring SASL mechanisms
> +
> +The following documentation assumes use of the Cyrus SASL implementation on a
> +Linux host, but the principals should apply to any other SASL implementation
s/principals/principles/
[I can imagine how the typo could've come about -- as you talk about
Kerberos principal(s) further below :-)]
> +or host. When SASL is enabled, the mechanism configuration will be loaded
> from
> +system default SASL service config /etc/sasl2/qemu.conf. If running QEMU as
> an
> +unprivileged user, an environment variable SASL_CONF_PATH can be used to make
> +behaviour suddenly changedit search alternate locations for the service
> config.
The last part above reads odd, maybe you accidentally removed some
words?
[...]
> +This says to use the 'GSSAPI' mechanism with the Kerberos v5 protocol, with
> +the server principal stored in /etc/qemu/krb5.tab. For this to work the
> +administrator of your KDC must generate a Kerberos principal for the server,
> +with a name of 'qemu/somehost.example.com@@EXAMPLE.COM' replacing
> +'somehost.example.com' with the fully qualified host name of the machine
> +running QEMU, and 'EXAMPLE.COM' with the Kerberos Realm.
> +
> +When using TLS, if username+password authentication is desired, then a
> +reasonable configuration is
> +
> address@hidden
> +mech_list: scram-sha-1
> +sasldb_path: /etc/qemu/passwd.db
> address@hidden example
> +
> +The saslpasswd2 program can be used to populate the passwd.db file with
> +accounts.
Maybe: "saslpasswd2" --> "@code{saslpasswd2}"
(Also helps with consistency, as I see you've used the @code{} attribute
further below.)
> +
> +Other SASL configurations will be left as an exercise for the reader. Note
> that
> +all mechanisms except GSSAPI, should be combined with use of TLS to ensure a
Spurious comma above.
> +secure data channel.
[...]
> +The GNUTLS package includes a command called @code{certtool} which can
Super nit: s/GNUTLS/GnuTLS/
[...]
> address@hidden tls_creds_setup
> address@hidden TLS x509 credential configuration
>
> address@hidden vnc_setup_sasl
> +QEMU has a standard mechanism for loading x509 credentials that will be
> +used for network services and clients. It requires specifying the
> address@hidden class name to the @code{-object} command line
> +argument for the system emulators. This also works for the helper tools
> +like @code{qemu-nbd} and @code{qemu-img}, but is named @code{--object}.
The '-object' for QEMU command-line, and the double-dashed '--object'
for external tools looks a tiny bit odd. But not this patch's problem.
(I just noticed Eric remarked on this too.)
> address@hidden Configuring SASL mechanisms
> +When specifying the object, the @code{dir} parameters specifies which
> +directory contains the credential files. This directory is expected to
> +contain files with the names mentioned previously, @code{ca-cert.pem},
> address@hidden, @code{server-cert.pem}, @code{client-key.pem}
> +and @code{client-cert.pem} as appropriate. It is also possible to
> +include a set of pre-generated diffie-hellman parameters in a file
Would be nice capitalize proper nouns: s/diffie-hellman/Diffie–Hellman
(DH) --- 'DH' in brackets because you use that acronym two lines below,
although it should be obvious to those setting up TLS.
> address@hidden, which can be created using the
> address@hidden --generate-dh-params} command. If omitted, QEMU will
> +dynamically generate DH parameters when loading the credentials.
[...]
> -The saslpasswd2 program can be used to populate the passwd.db file with
> -accounts.
> +Network services which support TLS will all have a @code{tls-creds}
> +parameter which expects the ID of the tls credentials object.
s/tls/TLS
[...]
Whether you fix these minor nits or not, your write-up is a strict
improvement, so:
Reviewed-by: Kashyap Chamarthy <address@hidden>
--
/kashyap