[libvirt] [PATCH 0/2] Fix issue with attempting to use polkit
The "issue" is when using a remote URI and authn when attempting to connect to the local system when a local URI and usage of pkttyagent was "expected" for local authn. The current bug is: https://bugzilla.redhat.com/show_bug.cgi?id=1374126 which was "forked off" from: https://bugzilla.redhat.com/show_bug.cgi?id=986365 which added the support for pkttyagent processing for "local" authn when the GUI option wasn't available (e.g. when ssh@localhost, run virsh) in order to perform the authn of the user. This is one of those "either" patch 1 or patch 2 will fix the problem, but I really couldn't decide which I liked better. I'm leaning towards the second one only because of the one authn failure; however, I'm not sure I like the escape path comparison to ":///". With only the first patch one would get two challenge/responses before failure, e.g.: $ virsh -c qemu+ssh://someuser@localhost/system list --all someuser@localhost's password: someuser@localhost's password: error: failed to connect to the hypervisor error: authentication unavailable: no polkit agent available to authenticate action 'org.libvirt.unix.manage' $ With the second patch one would get one challenge/reponse before failure e.g.: $ virsh -c qemu+ssh://someuser@localhost/system list --all someuser@localhost's password: error: failed to connect to the hypervisor error: authentication unavailable: no polkit agent available to authenticate action 'org.libvirt.unix.manage' $ Going with #2 restores essentially older (e.g. 1.3.4) processing when the pkttyagent processing loop didn't exist. The challenge/response in either case comes from ssh and is successful; however, code in remoteDispatchAuthList doesn't let a non root user to take this URI option for a local authn/authz because the callerUid is not 0 (root). Although, I suppose it would be possible to check whether the @callerUid was authorized (e.g. part of some group), but that I think "outside" the scope of what the bz is reporting, so I didn't take that route. The reality is the bug is mostly a problem for non UI type processing where providing that ctrl-c to quit isn't as likely/possible. John Ferlan (2): virsh: Track when create pkttyagent virsh: Don't attempt polkit processing for non local authn/authz tools/virsh.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) -- 2.9.3
https://bugzilla.redhat.com/show_bug.cgi?id=1374126 Due to how the processing for authentication using polkit works, the virshConnect code must first "attempt" an virConnectOpenAuth and then check for a "special" return error code VIR_ERR_AUTH_UNAVAILABLE in order to attempt to "retry" the authentication after performing a creation of a pkttyagent to handle the challenge/response for the client. However, if pkttyagent creation is not possible for the authentication being attempted (such as perhaps a "qemu+ssh://someuser@localhost/system"), then the same failure pattern would be returned and another attempt to create a pkttyagent would be done. This would continue "forever" until someone forced quit (e.g. ctrl-c) from virsh as the 'authfail' was not incremented when creating the pkttyagent. So add a 'agentCreated' boolean to track if we've attempted to create the agent at least once and force a failure if that creation returned the same error pattern. This resolves a possible never ending loop and will generate an error: error: failed to connect to the hypervisor error: authentication unavailable: no polkit agent available to authenticate action 'org.libvirt.unix.manage' NB: If the authentication was for a sufficiently privileged client, such as qemu+ssh://root@localhost/system, then the remoteDispatchAuthList "allows" the authentication to use libvirt since @callerUid would be 0. Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/virsh.c b/tools/virsh.c index 90f8125..1f5c2b1 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -145,6 +145,7 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly) bool keepalive_forced = false; virPolkitAgentPtr pkagent = NULL; int authfail = 0; + bool agentCreated = false; if (ctl->keepalive_interval >= 0) { interval = ctl->keepalive_interval; @@ -166,10 +167,12 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly) goto cleanup; err = virGetLastError(); - if (err && err->domain == VIR_FROM_POLKIT && + if (!agentCreated && + err && err->domain == VIR_FROM_POLKIT && err->code == VIR_ERR_AUTH_UNAVAILABLE) { if (!pkagent && !(pkagent = virPolkitAgentCreate())) goto cleanup; + agentCreated = true; } else if (err && err->domain == VIR_FROM_POLKIT && err->code == VIR_ERR_AUTH_FAILED) { authfail++; -- 2.9.3
On 05/11/2017 05:04 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1374126
Due to how the processing for authentication using polkit works, the virshConnect code must first "attempt" an virConnectOpenAuth and then check for a "special" return error code VIR_ERR_AUTH_UNAVAILABLE in order to attempt to "retry" the authentication after performing a creation of a pkttyagent to handle the challenge/response for the client.
However, if pkttyagent creation is not possible for the authentication being attempted (such as perhaps a "qemu+ssh://someuser@localhost/system"), then the same failure pattern would be returned and another attempt to create a pkttyagent would be done. This would continue "forever" until someone forced quit (e.g. ctrl-c) from virsh as the 'authfail' was not incremented when creating the pkttyagent.
So add a 'agentCreated' boolean to track if we've attempted to create the agent at least once and force a failure if that creation returned the same error pattern.
This resolves a possible never ending loop and will generate an error:
error: failed to connect to the hypervisor error: authentication unavailable: no polkit agent available to authenticate action 'org.libvirt.unix.manage'
NB: If the authentication was for a sufficiently privileged client, such as qemu+ssh://root@localhost/system, then the remoteDispatchAuthList "allows" the authentication to use libvirt since @callerUid would be 0.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
ACK Michal
https://bugzilla.redhat.com/show_bug.cgi?id=1374126 Due to how the processing for authentication using polkit works, the virshConnect code must first "attempt" an virConnectOpenAuth and then check for a "special" return error code VIR_ERR_AUTH_UNAVAILABLE in order to attempt to "retry" the authentication after performing a creation of a pkttyagent to handle the challenge/response for the client. However, attempting to use a remote connection, (such as perhaps "qemu+ssh://someuser@localhost/system"), will cause a never ending loop since attempting to generate a pkttyagent would fail for the network client connection resulting in a never ending loop since the return code is always VIR_ERR_AUTH_UNAVAILABLE from virPolkitCheckAuth. The only way out of the loop is a forced quit (e.g. ctrl-c) as the @authfail wouldn't be incremented as a result of a failed authn from pkttyagent. So rather than take any extra step for which the only result will be a failure, let's check if there is a URI and if it's not using ":///", then just fail. This resolves the never ending loop and will generate an error: error: failed to connect to the hypervisor error: authentication unavailable: no polkit agent available to authenticate action 'org.libvirt.unix.manage' NB: If the authentication was for a sufficiently privileged client, such as qemu+ssh://root@localhost/system, then the remoteDispatchAuthList "allows" the authentication to use libvirt since @callerUid would be 0. Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/virsh.c b/tools/virsh.c index 1f5c2b1..be368ba 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -166,6 +166,11 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly) if (readonly) goto cleanup; + /* No URI or indication of a requesting a remote connection, then + * polkit will not work for the authentication/authorization */ + if (!uri || !(strstr(uri, ":///"))) + goto cleanup; + err = virGetLastError(); if (!agentCreated && err && err->domain == VIR_FROM_POLKIT && -- 2.9.3
On 05/11/2017 05:04 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1374126
Due to how the processing for authentication using polkit works, the virshConnect code must first "attempt" an virConnectOpenAuth and then check for a "special" return error code VIR_ERR_AUTH_UNAVAILABLE in order to attempt to "retry" the authentication after performing a creation of a pkttyagent to handle the challenge/response for the client.
However, attempting to use a remote connection, (such as perhaps "qemu+ssh://someuser@localhost/system"), will cause a never ending loop since attempting to generate a pkttyagent would fail for the network client connection resulting in a never ending loop since the return code is always VIR_ERR_AUTH_UNAVAILABLE from virPolkitCheckAuth. The only way out of the loop is a forced quit (e.g. ctrl-c) as the @authfail wouldn't be incremented as a result of a failed authn from pkttyagent.
So rather than take any extra step for which the only result will be a failure, let's check if there is a URI and if it's not using ":///", then just fail.
This resolves the never ending loop and will generate an error:
error: failed to connect to the hypervisor error: authentication unavailable: no polkit agent available to authenticate action 'org.libvirt.unix.manage'
NB: If the authentication was for a sufficiently privileged client, such as qemu+ssh://root@localhost/system, then the remoteDispatchAuthList "allows" the authentication to use libvirt since @callerUid would be 0.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/virsh.c b/tools/virsh.c index 1f5c2b1..be368ba 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -166,6 +166,11 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly) if (readonly) goto cleanup;
+ /* No URI or indication of a requesting a remote connection, then + * polkit will not work for the authentication/authorization */ + if (!uri || !(strstr(uri, ":///"))) + goto cleanup;
But we can get here even without polkit, right? Therefore it'd be much more safer to check err && err->domain == VIR_FROM_POLKIT. Michal
On 05/24/2017 10:38 AM, Michal Privoznik wrote:
On 05/11/2017 05:04 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1374126
Due to how the processing for authentication using polkit works, the virshConnect code must first "attempt" an virConnectOpenAuth and then check for a "special" return error code VIR_ERR_AUTH_UNAVAILABLE in order to attempt to "retry" the authentication after performing a creation of a pkttyagent to handle the challenge/response for the client.
However, attempting to use a remote connection, (such as perhaps "qemu+ssh://someuser@localhost/system"), will cause a never ending loop since attempting to generate a pkttyagent would fail for the network client connection resulting in a never ending loop since the return code is always VIR_ERR_AUTH_UNAVAILABLE from virPolkitCheckAuth. The only way out of the loop is a forced quit (e.g. ctrl-c) as the @authfail wouldn't be incremented as a result of a failed authn from pkttyagent.
So rather than take any extra step for which the only result will be a failure, let's check if there is a URI and if it's not using ":///", then just fail.
This resolves the never ending loop and will generate an error:
error: failed to connect to the hypervisor error: authentication unavailable: no polkit agent available to authenticate action 'org.libvirt.unix.manage'
NB: If the authentication was for a sufficiently privileged client, such as qemu+ssh://root@localhost/system, then the remoteDispatchAuthList "allows" the authentication to use libvirt since @callerUid would be 0.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/virsh.c b/tools/virsh.c index 1f5c2b1..be368ba 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -166,6 +166,11 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly) if (readonly) goto cleanup;
+ /* No URI or indication of a requesting a remote connection, then + * polkit will not work for the authentication/authorization */ + if (!uri || !(strstr(uri, ":///"))) + goto cleanup;
But we can get here even without polkit, right? Therefore it'd be much more safer to check err && err->domain == VIR_FROM_POLKIT.
True - that's simple enough to adjust... So instead of: err = virGetLastError(); if (err && err->domain == VIR_FROM_POLKIT && err->code == VIR_ERR_AUTH_UNAVAILABLE) { if (!pkagent && !(pkagent = virPolkitAgentCreate())) goto cleanup; } else if (err && err->domain == VIR_FROM_POLKIT && err->code == VIR_ERR_AUTH_FAILED) { authfail++; } else { goto cleanup; } I could have if (err && err->domain == VIR_FROM_POLKIT) { /* No URI or indication of a requesting a remote connection, then * polkit will not work for the authentication/authorization */ if (!uri || !(strstr(uri, ":///"))) goto cleanup; if (err->code == VIR_ERR_AUTH_UNAVAILABLE) { if (!pkagent && !(pkagent = virPolkitAgentCreate())) goto cleanup; } else if (err->code == VIR_ERR_AUTH_FAILED) { authfail++; } else { goto cleanup; } } else { goto cleanup; } So is there a preferred approach? See the cover letter... Tks - John
On 05/24/2017 05:45 PM, John Ferlan wrote:
On 05/24/2017 10:38 AM, Michal Privoznik wrote:
On 05/11/2017 05:04 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1374126
Due to how the processing for authentication using polkit works, the virshConnect code must first "attempt" an virConnectOpenAuth and then check for a "special" return error code VIR_ERR_AUTH_UNAVAILABLE in order to attempt to "retry" the authentication after performing a creation of a pkttyagent to handle the challenge/response for the client.
However, attempting to use a remote connection, (such as perhaps "qemu+ssh://someuser@localhost/system"), will cause a never ending loop since attempting to generate a pkttyagent would fail for the network client connection resulting in a never ending loop since the return code is always VIR_ERR_AUTH_UNAVAILABLE from virPolkitCheckAuth. The only way out of the loop is a forced quit (e.g. ctrl-c) as the @authfail wouldn't be incremented as a result of a failed authn from pkttyagent.
So rather than take any extra step for which the only result will be a failure, let's check if there is a URI and if it's not using ":///", then just fail.
This resolves the never ending loop and will generate an error:
error: failed to connect to the hypervisor error: authentication unavailable: no polkit agent available to authenticate action 'org.libvirt.unix.manage'
NB: If the authentication was for a sufficiently privileged client, such as qemu+ssh://root@localhost/system, then the remoteDispatchAuthList "allows" the authentication to use libvirt since @callerUid would be 0.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/virsh.c b/tools/virsh.c index 1f5c2b1..be368ba 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -166,6 +166,11 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly) if (readonly) goto cleanup;
+ /* No URI or indication of a requesting a remote connection, then + * polkit will not work for the authentication/authorization */ + if (!uri || !(strstr(uri, ":///"))) + goto cleanup;
But we can get here even without polkit, right? Therefore it'd be much more safer to check err && err->domain == VIR_FROM_POLKIT.
True - that's simple enough to adjust...
So instead of:
err = virGetLastError(); if (err && err->domain == VIR_FROM_POLKIT && err->code == VIR_ERR_AUTH_UNAVAILABLE) { if (!pkagent && !(pkagent = virPolkitAgentCreate())) goto cleanup; } else if (err && err->domain == VIR_FROM_POLKIT && err->code == VIR_ERR_AUTH_FAILED) { authfail++; } else { goto cleanup; }
I could have
if (err && err->domain == VIR_FROM_POLKIT) { /* No URI or indication of a requesting a remote connection, then * polkit will not work for the authentication/authorization */ if (!uri || !(strstr(uri, ":///"))) goto cleanup; if (err->code == VIR_ERR_AUTH_UNAVAILABLE) { if (!pkagent && !(pkagent = virPolkitAgentCreate())) goto cleanup; } else if (err->code == VIR_ERR_AUTH_FAILED) { authfail++; } else { goto cleanup; } } else { goto cleanup; }
So is there a preferred approach? See the cover letter...
Aha. So these two patches are mutually exclusive? I haven't read it until now O:-) Well, I like the first one better because it's more simple. It fixes just what is broken and is not introducing any bug. Michal
ping? Tks - John On 05/11/2017 11:04 AM, John Ferlan wrote:
The "issue" is when using a remote URI and authn when attempting to connect to the local system when a local URI and usage of pkttyagent was "expected" for local authn.
The current bug is:
https://bugzilla.redhat.com/show_bug.cgi?id=1374126
which was "forked off" from:
https://bugzilla.redhat.com/show_bug.cgi?id=986365
which added the support for pkttyagent processing for "local" authn when the GUI option wasn't available (e.g. when ssh@localhost, run virsh) in order to perform the authn of the user.
This is one of those "either" patch 1 or patch 2 will fix the problem, but I really couldn't decide which I liked better. I'm leaning towards the second one only because of the one authn failure; however, I'm not sure I like the escape path comparison to ":///".
With only the first patch one would get two challenge/responses before failure, e.g.:
$ virsh -c qemu+ssh://someuser@localhost/system list --all someuser@localhost's password: someuser@localhost's password: error: failed to connect to the hypervisor error: authentication unavailable: no polkit agent available to authenticate action 'org.libvirt.unix.manage' $
With the second patch one would get one challenge/reponse before failure e.g.:
$ virsh -c qemu+ssh://someuser@localhost/system list --all someuser@localhost's password: error: failed to connect to the hypervisor error: authentication unavailable: no polkit agent available to authenticate action 'org.libvirt.unix.manage' $
Going with #2 restores essentially older (e.g. 1.3.4) processing when the pkttyagent processing loop didn't exist.
The challenge/response in either case comes from ssh and is successful; however, code in remoteDispatchAuthList doesn't let a non root user to take this URI option for a local authn/authz because the callerUid is not 0 (root). Although, I suppose it would be possible to check whether the @callerUid was authorized (e.g. part of some group), but that I think "outside" the scope of what the bz is reporting, so I didn't take that route.
The reality is the bug is mostly a problem for non UI type processing where providing that ctrl-c to quit isn't as likely/possible.
John Ferlan (2): virsh: Track when create pkttyagent virsh: Don't attempt polkit processing for non local authn/authz
tools/virsh.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
ping^2? One option or the other? Tks - John On 05/17/2017 06:57 AM, John Ferlan wrote:
ping?
Tks -
John
On 05/11/2017 11:04 AM, John Ferlan wrote:
The "issue" is when using a remote URI and authn when attempting to connect to the local system when a local URI and usage of pkttyagent was "expected" for local authn.
The current bug is:
https://bugzilla.redhat.com/show_bug.cgi?id=1374126
which was "forked off" from:
https://bugzilla.redhat.com/show_bug.cgi?id=986365
which added the support for pkttyagent processing for "local" authn when the GUI option wasn't available (e.g. when ssh@localhost, run virsh) in order to perform the authn of the user.
This is one of those "either" patch 1 or patch 2 will fix the problem, but I really couldn't decide which I liked better. I'm leaning towards the second one only because of the one authn failure; however, I'm not sure I like the escape path comparison to ":///".
With only the first patch one would get two challenge/responses before failure, e.g.:
$ virsh -c qemu+ssh://someuser@localhost/system list --all someuser@localhost's password: someuser@localhost's password: error: failed to connect to the hypervisor error: authentication unavailable: no polkit agent available to authenticate action 'org.libvirt.unix.manage' $
With the second patch one would get one challenge/reponse before failure e.g.:
$ virsh -c qemu+ssh://someuser@localhost/system list --all someuser@localhost's password: error: failed to connect to the hypervisor error: authentication unavailable: no polkit agent available to authenticate action 'org.libvirt.unix.manage' $
Going with #2 restores essentially older (e.g. 1.3.4) processing when the pkttyagent processing loop didn't exist.
The challenge/response in either case comes from ssh and is successful; however, code in remoteDispatchAuthList doesn't let a non root user to take this URI option for a local authn/authz because the callerUid is not 0 (root). Although, I suppose it would be possible to check whether the @callerUid was authorized (e.g. part of some group), but that I think "outside" the scope of what the bz is reporting, so I didn't take that route.
The reality is the bug is mostly a problem for non UI type processing where providing that ctrl-c to quit isn't as likely/possible.
John Ferlan (2): virsh: Track when create pkttyagent virsh: Don't attempt polkit processing for non local authn/authz
tools/virsh.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
John Ferlan -
Michal Privoznik