Re: [Qemu-devel] [PATCH v5 10/11] authz: add QAuthZPAM object type for authorizing using PAM

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 19/10/2018 14:55, Daniel P. Berrangé wrote:
> On Fri, Oct 19, 2018 at 12:02:57PM +0200, Philippe Mathieu-Daudé wrote:
>> On 09/10/2018 15:04, Daniel P. Berrangé wrote:
>>> From: "Daniel P. Berrange" <berrange@redhat.com>
>>>
>>> Add an authorization backend that talks to PAM to check whether the user
>>> identity is allowed. This only uses the PAM account validation facility,
>>> which is essentially just a check to see if the provided username is permitted
>>> access. It doesn't use the authentication or session parts of PAM, since
>>> that's dealt with by the relevant part of QEMU (eg VNC server).
>>>
>>> Consider starting QEMU with a VNC server and telling it to use TLS with
>>> x509 client certificates and configuring it to use an PAM to validate
>>> the x509 distinguished name. In this example we're telling it to use PAM
>>> for the QAuthZ impl with a service name of "qemu-vnc"
>>>
>>>  $ qemu-system-x86_64 \
>>>      -object tls-creds-x509,id=tls0,dir=/home/berrange/security/qemutls,\
>>>              endpoint=server,verify-peer=yes \
>>>      -object authz-pam,id=authz0,service=qemu-vnc \
>>>      -vnc :1,tls-creds=tls0,tls-authz=authz0
>>>
>>> This requires an /etc/pam/qemu-vnc file to be created with the auth
>>> rules. A very simple file based whitelist can be setup using
>>>
>>>   $ cat > /etc/pam/qemu-vnc <<EOF
>>>   account         requisite       pam_listfile.so item=user sense=allow file=/etc/qemu/vnc.allow
>>>   EOF
>>>
>>> The /etc/qemu/vnc.allow file simply contains one username per line. Any
>>> username not in the file is denied. The usernames in this example are
>>> the x509 distinguished name from the client's x509 cert.
>>>
>>>   $ cat > /etc/qemu/vnc.allow <<EOF
>>>   CN=laptop.berrange.com,O=Berrange Home,L=London,ST=London,C=GB
>>>   EOF
>>>
>>> More interesting would be to configure PAM to use an LDAP backend, so
>>> that the QEMU authorization check data can be centralized instead of
>>> requiring each compute host to have file maintained.
>>>
>>> The main limitation with this PAM module is that the rules apply to all
>>> QEMU instances on the host. Setting up different rules per VM, would
>>> require creating a separate PAM service name & config file for every
>>> guest. An alternative approach for the future might be to not pass in
>>> the plain username to PAM, but instead combine the VM name or UUID with
>>> the username. This requires further consideration though.
>>>
>>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>>> ---
>>>  authz/Makefile.objs     |   3 +
>>>  authz/pamacct.c         | 149 ++++++++++++++++++++++++++++++++++++++++
>>>  authz/trace-events      |   3 +
>>>  configure               |  37 ++++++++++
>>>  include/authz/pamacct.h | 100 +++++++++++++++++++++++++++
>>>  qemu-options.hx         |  35 ++++++++++
>>>  6 files changed, 327 insertions(+)
>>>  create mode 100644 authz/pamacct.c
>>>  create mode 100644 include/authz/pamacct.h
>>>
>>> diff --git a/configure b/configure
>>> index f89d293585..bca8fd1b49 100755
>>> --- a/configure
>>> +++ b/configure
>>> +##########################################
>>> +# PAM probe
>>> +
>>> +if test "x$auth_pam" != "no"; then
>>> +    cat > $TMPC <<EOF
>>> +#include <security/pam_appl.h>
>>> +#include <stdio.h>
>>> +int main(void) {
>>> +   const char *service_name = "qemu";
>>> +   const char *user = "frank";
>>> +   const struct pam_conv *pam_conv = NULL;
>>> +   pam_handle_t *pamh = NULL;
>>> +   pam_start(service_name, user, pam_conv, &pamh);
>>> +   return 0;
>>> +}
>>> +EOF
>>> +    if compile_prog "" "-lpam" ; then
>>> +	auth_pam=yes
>>> +    else
>>> +	if test "$auth_pam" = "yes"; then
>>> +	    feature_not_found "PAM" "Install pam-devel"
>>
>> Not all distributions name this package 'pam-devel', but I think we get
>> the message.
> 
> I'll squash in
> 
> diff --git a/configure b/configure
> index 3ec5578c5a..db35d52e12 100755
> --- a/configure
> +++ b/configure
> @@ -2902,7 +2902,7 @@ EOF
>         auth_pam=yes
>      else
>         if test "$auth_pam" = "yes"; then
> -           feature_not_found "PAM" "Install pam-devel"
> +           feature_not_found "PAM" "Install PAM development package"

This seems better, thanks.

>         else
>             auth_pam=no
>         fi
> 
> 
> Regards,
> Daniel
>