Note: This bug is displayed in read-only format because the product is no longer active in Red Hat Bugzilla.

Bug 1551575

Summary: NPE while adding NIC to VM, if user has no sufficient permissions in VM
Product: [oVirt] ovirt-engine Reporter: biakymet
Component: BLL.InfraAssignee: Ori Liel <oliel>
Status: CLOSED NOTABUG QA Contact: Radim Hrazdil <rhrazdil>
Severity: high Docs Contact:
Priority: high    
Version: 4.3.0CC: bugs, danken, eraviv, lsvaty, michal.skrivanek, mperina, oliel, omachace, ylavi
Target Milestone: ---Keywords: Reopened
Target Release: ---Flags: rule-engine: ovirt-4.2+
ylavi: exception+
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2018-07-18 12:45:20 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: Infra RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
part of engine log
none
add clutster "fails on permissions"
none
add host "fails on permissions"
none
engine.log for add host and cluster none

Description biakymet 2018-03-05 12:48:39 UTC
https://<engine>/ovirt-engine/api/vms/<vm-id>/nics

Body:
{
	"name": "nic10",
	"interface": "virtio",
	"vnic_profile": {
		"id": "0000000a-000a-000a-000a-000000000398"
	}
}

Engine version: ovirt-engine-4.2.2-0.0.master.20180108193417.git45b0275.el7.centos.noarch
Exception: 2018-03-05 13:39:23,817+01 INFO  [org.ovirt.engine.core.bll.network.vm.AddVmInterfaceCommand] (default task-111) [ba6ce6b7-06df-40ea-a71b-7f6a3ad4f151] Running command: AddVmInterfaceCommand internal: false. Ent
ities affected :  ID: 04f1e30b-1f81-4b0b-84ce-050ff54c128a Type: VMAction group CONFIGURE_VM_NETWORK with role type USER,  ID: cdcb2b43-51ae-4cd2-8dfc-5359cd3c22b5 Type: VnicProfileAction group CONFIGURE_VM_NETW
ORK with role type USER
2018-03-05 13:39:23,859+01 INFO  [org.ovirt.engine.core.bll.network.vm.ActivateDeactivateVmNicCommand] (default task-111) [2a4737be] Running command: ActivateDeactivateVmNicCommand internal: true. Entities affec
ted :  ID: 04f1e30b-1f81-4b0b-84ce-050ff54c128a Type: VM
2018-03-05 13:39:23,873+01 INFO  [org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector] (default task-111) [2a4737be] EVENT_ID: NETWORK_ACTIVATE_VM_INTERFACE_SUCCESS(1,012), Network Interface nic
2 (VirtIO) was plugged to VM 0000VDSMPY3. (User: admin@internal-authz)
2018-03-05 13:39:23,882+01 INFO  [org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector] (default task-111) [2a4737be] EVENT_ID: NETWORK_ADD_VM_INTERFACE(932), Interface nic2 (VirtIO) was added to
 VM 0000VDSMPY3. (User: admin@internal-authz)
2018-03-05 13:39:23,887+01 ERROR [org.ovirt.engine.api.restapi.resource.AbstractBackendResource] (default task-111) [] Operation Failed: null
2018-03-05 13:39:23,887+01 ERROR [org.ovirt.engine.api.restapi.resource.AbstractBackendResource] (default task-111) [] Exception: java.lang.NullPointerException
        at org.ovirt.engine.api.restapi.resource.AbstractBackendResource$EntityIdResolver.resolve(AbstractBackendResource.java:403) [restapi-jaxrs.jar:]
        at org.ovirt.engine.api.restapi.resource.AbstractBackendCollectionResource.resolveCreated(AbstractBackendCollectionResource.java:180) [restapi-jaxrs.jar:]
        at org.ovirt.engine.api.restapi.resource.AbstractBackendCollectionResource.fetchCreatedEntity(AbstractBackendCollectionResource.java:201) [restapi-jaxrs.jar:]
        at org.ovirt.engine.api.restapi.resource.AbstractBackendCollectionResource.performCreate(AbstractBackendCollectionResource.java:148) [restapi-jaxrs.jar:]
        at org.ovirt.engine.api.restapi.resource.AbstractBackendCollectionResource.performCreate(AbstractBackendCollectionResource.java:135) [restapi-jaxrs.jar:]
        at org.ovirt.engine.api.restapi.resource.AbstractBackendCollectionResource.performCreate(AbstractBackendCollectionResource.java:154) [restapi-jaxrs.jar:]
        at org.ovirt.engine.api.restapi.resource.BackendVmNicsResource.add(BackendVmNicsResource.java:46) [restapi-jaxrs.jar:]
        at org.ovirt.engine.api.resource.VmNicsResource.doAdd(VmNicsResource.java:87) [restapi-definition.jar:]
        at sun.reflect.GeneratedMethodAccessor594.invoke(Unknown Source) [:1.8.0_151]
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) [rt.jar:1.8.0_151]
        at java.lang.reflect.Method.invoke(Method.java:498) [rt.jar:1.8.0_151]
        at org.jboss.resteasy.core.MethodInjectorImpl.invoke(MethodInjectorImpl.java:140) [resteasy-jaxrs-3.0.24.Final.jar:3.0.24.Final]
        at org.jboss.resteasy.core.ResourceMethodInvoker.invokeOnTarget(ResourceMethodInvoker.java:295) [resteasy-jaxrs-3.0.24.Final.jar:3.0.24.Final]
        at org.jboss.resteasy.core.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:249) [resteasy-jaxrs-3.0.24.Final.jar:3.0.24.Final]
        at org.jboss.resteasy.core.ResourceLocatorInvoker.invokeOnTargetObject(ResourceLocatorInvoker.java:138) [resteasy-jaxrs-3.0.24.Final.jar:3.0.24.Final]
        at org.jboss.resteasy.core.ResourceLocatorInvoker.invoke(ResourceLocatorInvoker.java:107) [resteasy-jaxrs-3.0.24.Final.jar:3.0.24.Final]
        at org.jboss.resteasy.core.ResourceLocatorInvoker.invokeOnTargetObject(ResourceLocatorInvoker.java:133) [resteasy-jaxrs-3.0.24.Final.jar:3.0.24.Final]
        at org.jboss.resteasy.core.ResourceLocatorInvoker.invoke(ResourceLocatorInvoker.java:107) [resteasy-jaxrs-3.0.24.Final.jar:3.0.24.Final]
        at org.jboss.resteasy.core.ResourceLocatorInvoker.invokeOnTargetObject(ResourceLocatorInvoker.java:133) [resteasy-jaxrs-3.0.24.Final.jar:3.0.24.Final]
        at org.jboss.resteasy.core.ResourceLocatorInvoker.invoke(ResourceLocatorInvoker.java:101) [resteasy-jaxrs-3.0.24.Final.jar:3.0.24.Final]
        at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:406) [resteasy-jaxrs-3.0.24.Final.jar:3.0.24.Final]
        at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:213) [resteasy-jaxrs-3.0.24.Final.jar:3.0.24.Final]
        at org.jboss.resteasy.plugins.server.servlet.ServletContainerDispatcher.service(ServletContainerDispatcher.java:228) [resteasy-jaxrs-3.0.24.Final.jar:3.0.24.Final]
        at org.jboss.resteasy.plugins.server.servlet.HttpServletDispatcher.service(HttpServletDispatcher.java:56) [resteasy-jaxrs-3.0.24.Final.jar:3.0.24.Final]
        at org.jboss.resteasy.plugins.server.servlet.HttpServletDispatcher.service(HttpServletDispatcher.java:51) [resteasy-jaxrs-3.0.24.Final.jar:3.0.24.Final]
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:790) [jboss-servlet-api_3.1_spec-1.0.0.Final.jar:1.0.0.Final]
        at io.undertow.servlet.handlers.ServletHandler.handleRequest(ServletHandler.java:85) [undertow-servlet-1.4.18.Final.jar:1.4.18.Final]
        at io.undertow.servlet.handlers.FilterHandler.handleRequest(FilterHandler.java:81) [undertow-servlet-1.4.18.Final.jar:1.4.18.Final]
        at io.undertow.servlet.handlers.security.ServletSecurityRoleHandler.handleRequest(ServletSecurityRoleHandler.java:62) [undertow-servlet-1.4.18.Final.jar:1.4.18.Final]
        at io.undertow.servlet.handlers.ServletDispatchingHandler.handleRequest(ServletDispatchingHandler.java:36) [undertow-servlet-1.4.18.Final.jar:1.4.18.Final]
        at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43) [undertow-core-1.4.18.Final.jar:1.4.18.Final]
        at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43) [undertow-core-1.4.18.Final.jar:1.4.18.Final]
        at io.undertow.server.handlers.PredicateHandler.handleRequest(PredicateHandler.java:43) [undertow-core-1.4.18.Final.jar:1.4.18.Final]
        at io.undertow.servlet.handlers.ServletInitialHandler.dispatchRequest(ServletInitialHandler.java:274) [undertow-servlet-1.4.18.Final.jar:1.4.18.Final]
        at io.undertow.servlet.handlers.ServletInitialHandler.dispatchToPath(ServletInitialHandler.java:209) [undertow-servlet-1.4.18.Final.jar:1.4.18.Final]
        at io.undertow.servlet.spec.RequestDispatcherImpl.forwardImpl(RequestDispatcherImpl.java:221) [undertow-servlet-1.4.18.Final.jar:1.4.18.Final]
        at io.undertow.servlet.spec.RequestDispatcherImpl.forwardImplSetup(RequestDispatcherImpl.java:147) [undertow-servlet-1.4.18.Final.jar:1.4.18.Final]
        at io.undertow.servlet.spec.RequestDispatcherImpl.forward(RequestDispatcherImpl.java:111) [undertow-servlet-1.4.18.Final.jar:1.4.18.Final]
        at org.ovirt.engine.api.restapi.invocation.VersionFilter.doFilter(VersionFilter.java:180) [restapi-jaxrs.jar:]
        at org.ovirt.engine.api.restapi.invocation.VersionFilter.doFilter(VersionFilter.java:98) [restapi-jaxrs.jar:]
        at io.undertow.servlet.core.ManagedFilter.doFilter(ManagedFilter.java:61) [undertow-servlet-1.4.18.Final.jar:1.4.18.Final]
        at io.undertow.servlet.handlers.FilterHandler$FilterChainImpl.doFilter(FilterHandler.java:131) [undertow-servlet-1.4.18.Final.jar:1.4.18.Final]

Comment 1 Ori Liel 2018-03-12 08:02:50 UTC
I'm not able to reproduce this bug. Creating a vm nic as described above worked for me with not problem

Comment 2 biakymet 2018-03-29 16:04:38 UTC
Created attachment 1414797 [details]
part of engine log

When I'm logged in like `admin@internal` and sending request to:
https://brq-dev.rhev.lab.eng.brq.redhat.com/ovirt-engine/api/vms/db6e8ef7-a414-4584-ad6e-927ae7c461ef/nics

With headers:
Content-Type: "application/json"
Accept: "application/json"
Filter: "true"

And body:
{"name":"nic3","interface":"virtio","vnic_profile":{"id":"0000000a-000a-000a-000a-000000000398"}}

I get error:
{
    "reason": "Operation Failed"
}

Without header "Filter: true" or with use "ujo@internal" (which is not admin) everything works fine.

Comment 3 biakymet 2018-03-29 16:06:05 UTC
Reopening reproducible in comment 2.

Comment 4 Michal Skrivanek 2018-03-29 16:10:26 UTC
bumping up prio, this is blocking VM Portal

Comment 5 Ondra Machacek 2018-04-04 15:17:35 UTC
The issue is in 'AbstractBackendNicsResource:NicResolver::lookupEntity'[1].

This executes 'GetVmNetworkInterfaceViewByVmId' which expects the user has permission on the VM. But that's admin user who don't have direct permissions there.

https://github.com/oVirt/ovirt-engine/blob/e2aad594a55c7272b513736616cb4b9841c2c43d/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendNicsResource.java#L45

Comment 6 Martin Perina 2018-04-04 17:45:56 UTC
(In reply to Ondra Machacek from comment #5)
> The issue is in 'AbstractBackendNicsResource:NicResolver::lookupEntity'[1].
> 
> This executes 'GetVmNetworkInterfaceViewByVmId' which expects the user has
> permission on the VM. But that's admin user who don't have direct
> permissions there.
> 
> https://github.com/oVirt/ovirt-engine/blob/
> e2aad594a55c7272b513736616cb4b9841c2c43d/backend/manager/modules/restapi/
> jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/
> AbstractBackendNicsResource.java#L45

The permission part is OK, but the networking code needs to handle the NPE, when results are returned from database, because usually admin@internal doesn't have any directly assigned permissions and using 'Filter: true' invokes exactly that use case

Comment 7 Martin Perina 2018-04-04 17:47:01 UTC
(In reply to Martin Perina from comment #6)
> (In reply to Ondra Machacek from comment #5)
> > The issue is in 'AbstractBackendNicsResource:NicResolver::lookupEntity'[1].
> > 
> > This executes 'GetVmNetworkInterfaceViewByVmId' which expects the user has
> > permission on the VM. But that's admin user who don't have direct
> > permissions there.
> > 
> > https://github.com/oVirt/ovirt-engine/blob/
> > e2aad594a55c7272b513736616cb4b9841c2c43d/backend/manager/modules/restapi/
> > jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/
> > AbstractBackendNicsResource.java#L45
> 
> The permission part is OK, but the networking code needs to handle the NPE,
> when results are returned from database, because usually admin@internal
> doesn't have any directly assigned permissions and using 'Filter: true'
> invokes exactly that use case

Correction "... when no results are returned ..."

Comment 8 eraviv 2018-05-22 10:20:20 UTC
(In reply to Martin Perina from comment #7)
> (In reply to Martin Perina from comment #6)
> > (In reply to Ondra Machacek from comment #5)
> > > The issue is in 'AbstractBackendNicsResource:NicResolver::lookupEntity'[1].
> > > 
> > > This executes 'GetVmNetworkInterfaceViewByVmId' which expects the user has
> > > permission on the VM. But that's admin user who don't have direct
> > > permissions there.
> > > 
> > > https://github.com/oVirt/ovirt-engine/blob/
> > > e2aad594a55c7272b513736616cb4b9841c2c43d/backend/manager/modules/restapi/
> > > jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/
> > > AbstractBackendNicsResource.java#L45
> > 
> > The permission part is OK, but the networking code needs to handle the NPE,
> > when results are returned from database, because usually admin@internal
> > doesn't have any directly assigned permissions and using 'Filter: true'
> > invokes exactly that use case
> 
> Correction "... when no results are returned ..."

Martin,

Is it "legal" to filter admin permissions via the request header, because IIUC from the above, admins are not associated directly with permissions.
If this is the case, then the bug might be that this policy is not implemented in engine but rather:
https://github.com/oVirt/ovirt-engine/blob/master/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BaseBackendResource.java#L384 (isFiltered)

WDYT?
Thanks

Comment 9 eraviv 2018-05-22 10:56:25 UTC
Also, I see an implementation which I do not understand in light of the above "admin@internal doesn't have any directly assigned permissions":

https://github.com/oVirt/ovirt-engine/blob/master/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/QueriesCommandBase.java#L147

where a filter=true condition is prioritized over whether the user is an admin (and also over internal execution).

What am I missing?

Thanks

Comment 10 eraviv 2018-05-22 11:10:16 UTC
Lastly, in this bug an admin adds a vnic to a vm. The addition completes successfully, but when the response is being assembled, a query for getting all the current vnics is run internally and fails due to the filter=true header. Therefore the response reports a failure although the vnic was created.

All of this has no relation to the fact that this a networking request, and ti all happens in the generic base classes I quoted above.

So it feels to me like there should be some general mechanism to handle this use case. The above needinfo comments reflect the fact that I am not able to find such a mechanism in the code not any external documentation on the expected behaviour.

Any help greatly appreciated.
Thanks

Comment 11 eraviv 2018-05-30 10:10:58 UTC
Created attachment 1445751 [details]
add clutster "fails on permissions"

Comment 12 eraviv 2018-05-30 10:11:30 UTC
Created attachment 1445752 [details]
add host "fails on permissions"

Comment 13 eraviv 2018-05-30 10:12:05 UTC
Trying to add a new host or a new cluster
when user is admin@internal and headers include filter=true produces the same results: entity is created by REST API response is 'operation failed... insufficient permissions'. see attached. therefore moving this bug to infra team.

Comment 14 Martin Perina 2018-05-30 10:51:25 UTC
(In reply to eraviv from comment #13)
> Trying to add a new host or a new cluster
> when user is admin@internal and headers include filter=true produces the
> same results: entity is created by REST API response is 'operation failed...
> insufficient permissions'. see attached. therefore moving this bug to infra
> team.

What is the actual exception about those? Could you please add logs?

Comment 15 eraviv 2018-05-31 05:11:23 UTC
Created attachment 1446073 [details]
engine.log for add host and cluster

Comment 17 Ori Liel 2018-07-18 12:45:20 UTC
The analysis is: 

If a VM is created by an admin without providing filter=true, no permissions are created on it beyond the admin's generic permissions. When adding a NIC to this VM with filter=true, the NIC is created, but its representation can't be fetched from the engine due to the requested filtering, since this user doesn't have explicit permission on the VM. That is the reason for the 500 response.

While the failure could be more graceful, the main point is that this scenario entails improper usage of the filter property. If an admin user intends to make POST requests to a VM with filter=true, it should have created the VM with filter=true to begin with.

The resolution of this issue is to clearly document the filter property on Add-Vm operation. This was done in ovirt-engine-api-model project: 

   https://gerrit.ovirt.org/#/c/92579/

In summary I believe this issue can be closed, so I am closing it. If there are any reservations, please re-open it and explain what is the reason