On Thu, Jan 25, 2018 at 05:09:30PM +0000, Daniel P. Berrangé wrote: > From: "Daniel P. Berrange" <berra...@redhat.com>
[...] > +@node vnc_setup_sasl > + > +@subsection 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 > + > +@example > +mech_list: scram-sha-1 > +sasldb_path: /etc/qemu/passwd.db > +@end 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/ [...] > +@node tls_creds_setup > +@subsection TLS x509 credential configuration > > -@node 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 > +@code{tls-creds-x509} 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.) > -@subsection 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}, > +@code{server-key.pem}, @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. > +@code{dh-params.pem}, which can be created using the > +@code{certtool --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 <kcham...@redhat.com> -- /kashyap