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

[Daniel P . Berrangé]

On Fri, Feb 22, 2019 at 01:34:12AM +0100, Philippe Mathieu-Daudé wrote:
> Hi Daniel,
> 
> On 2/15/19 4:57 PM, Daniel P. Berrangé wrote:
> > From: "Daniel P. Berrange" <address@hidden>
> > 
> > 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 <address@hidden>
> > ---

> > +static bool qauthz_pam_is_allowed(QAuthZ *authz,
> > +                                  const char *identity,
> > +                                  Error **errp)
> > +{
> > +    QAuthZPAM *pauthz = QAUTHZ_PAM(authz);
> > +    const struct pam_conv pam_conversation = { 0 };
> > +    pam_handle_t *pamh = NULL;
> > +    int ret;
> > +
> > +    trace_qauthz_pam_check(authz, identity, pauthz->service);
> > +    ret = pam_start(pauthz->service,
> > +                    identity,
> > +                    &pam_conversation,
> > +                    &pamh);
> > +    if (ret != PAM_SUCCESS) {
> > +        error_setg(errp, "Unable to start PAM transaction: %s",
> > +                   pam_strerror(NULL, ret));
> > +        return false;
> > +    }
> > +
> > +    ret = pam_acct_mgmt(pamh, PAM_SILENT);
> > +    if (ret != PAM_SUCCESS) {
> > +        error_setg(errp, "Unable to authorize user '%s': %s",
> > +                   identity, pam_strerror(pamh, ret));
> > +        goto cleanup;
> > +    }
> > +
> > + cleanup:
> > +    pam_end(pamh, ret);
> > +    return ret == PAM_SUCCESS;
> > +}
> 
> I still need to digest this function (reading more about PAM).

FWIW there's reasonably good manpages 'pam(3)' and 'pam(8)' are
starting points.


> > @@ -2864,6 +2870,33 @@ else
> >  fi
> >  
> >  
> > +##########################################
> > +# PAM probe
> > +
> > +if test "x$auth_pam" != "no"; then
> 
> Either check "x$auth_pam" != "xno", or "$auth_pam" != "no" (the latter
> seems to follow the style of this file).
> 
> Currently this condition is always true, so the script always calls
> compile_prog. And if an user has PAM locally installed, it is not
> possible to not use it.

Opps, yes, did I say I hate shell :-)

> 
> > +    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 development package"
> > +   else
> > +       auth_pam=no
> > +   fi
> > +    fi

I notice some indentation damage here now due to tabs that I'll also
fix.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|