On 4/3/24 08:05, Jan Beulich wrote:
On 03.04.2024 13:10, Daniel P. Smith wrote:
On 4/3/24 02:16, Jan Beulich wrote:
On 02.04.2024 19:06, Andrew Cooper wrote:
The commit makes a claim without any kind of justification.
Well, what does "have no business" leave open?
Why does it not have any business? Why should a domain that creates an
event channel not be able to inquire about its status?
Event channels we talk about here are created via
alloc_unbound_xen_event_channel(). IOW it's not any domain creating them.
Once connected, the respective domain is of course fine to query its end
of the channel.
I would disagree, for instance alloc_unbound_xen_event_channel() is used
in response to XEN_DOMCTL_vuart_op:XEN_DOMCTL_VUART_OP_INIT and
XEN_DOMCTL_VM_EVENT_OP_PAGING:XEN_VM_EVENT_ENABLE, which are hypercalls
by a domain and not something initiated by the hypervisor.
The claim is false, and the commit broke lsevtchn in dom0.
Or alternatively lsevtchn was doing something that was never meant to work
(from Xen's perspective).
Again, you have not said why this is a problem. What concern does it
create? Does it open the door for access elevation, resource
deprivation, or some other malicious behaviors?
It exposes information that perhaps better wouldn't be exposed. Imo if
Xen owned resource state is of interest, it would want exposing via
hypfs.
You didn't answer why, just again expressed your opinion that it is not
better exposed. And I would have to wholly disagree with the sentiment
that hypfs exposure is the deciding factor what is or is not worth
exposing. This thinking is completely orthogonal to FLASK and
fine-grained access control.
It is also quite
obvious from XSM_TARGET that it has broken device model stubdoms too.
Why would that be "obvious"? What business would a stubdom have to look at
Xen's side of an evtchn?
Again, you have not expressed why it shouldn't be able to do so.
See above - not its resource, nor its guest's.
It is a resource provided to a domain that the domain can send/raise an
event to and a backing domain that can bind to it, ie. the two
parameters that must be passed to the allocation call.
Whether to return information about a xen-owned evtchn is a matter of policy,
and it's not acceptable to short circuit the XSM on the matter.
I can certainly accept this as one possible view point. As in so many cases
I'm afraid I dislike you putting it as if it was the only possible one.
In fact, this commit is in violation of the XSM. It hard-codes a
resource access check outside XSM, thus breaking the fine-grained access
control of FLASK.
Perhaps; see below and see the question raised in the subsequent reply
to the patch.
In summary: The supposed justification you claim is missing in the original
change is imo also missing here then: What business would any entity in the
system have to look at Xen's side of an event channel? Back at the time, 3
people agreed that it's "none".
As stated, you provided no reason or justification for "has no business"
and by face value is an opinion that a few people agreed with. As for
why, there could be a myriad number of reasons a domain may want to
check the status of an interface it has with the hypervisor. From just
logging its state for debug to throttling attempts at sending an event.
So why, from a security/access control decision, does this access have
to absolutely blocked, even from FLASK?
I didn't say it absolutely needs to be blocked. I'm okay to become
convinced otherwise. But in the description complaining about lack of
reasons in the 3-4 year old change, just to then again not provide any
reasons looks "interesting" to me. (And no, just to take that example,
lsevtchn not working anymore on such channels is not on its own a
reason. As indicated, it may well be that conceptually it was never
supposed to be able to have access to this information. The latest not
anymore when hypfs was introduced.)
This broke an existing behavior, whether that behavior is correct can
always be questioned, does not justify leaving an incorrect
implementation. And it is incorrect because as again you have not
articulated why the lsevtchn behavior is wrong and thus whether this is
the valid corrective action.
v/r,
dps
