Peng Tao,

> a single enable/disable flag is much easier to implement and maintain than a white-list.

1) It's not really simpler. It's a flag check instead of a function call. The three-state variant you suggested in #16 is clearly more complicated, which is when I noticed that regexps were addressing your concerns while keeping the patch simple.

2) It's not a long term solution, but a "quick fix" that does not address the criteria I outlined in #25. This implies that we would then need to replace it with something else. Do we want to offer an option to user, then walk it back and offer another one later? Why would you prefer that?

3) It's also not simpler in the sense that there is no patch for it. Again, I really invite you to focus on the existing patch, and check if it solves the problem and if it can be amended later (i.e. if it would not require a walk-back).

> The latter one is more complex than you think.

Why do you think so? What is missing in my patch? Have you reviewed my patch?


> All the options I listed in my previous reply are not
> supposed to be altered by a random user.

But they can be altered by random users today.

This is another issue with the simple flag: it only gives back functionality to those who need it by re-exposing the bug entirely. This is what I called a "risk" in my response to you in comment #25.

> My fear is that you might end up with filtering just the binary paths.

Have you reviewed my patch?

It does filter only paths, because right now this is where the bug is. You cannot remote execute with other annotations. The dangerous bug today is remote execution.

I am not saying there is no risk on other annotations. However, I have reviewed them, and so far, my evaluation of the risk for all other annotations is that it is negligible *relative to* the ability to execute arbitrary code.

Specifically:

- Annotations that configure the guest kernel, including boot options, kernel image, guest hook paths, etc, are normally contained by the VM layer. It may make administrative sense to provide similar filtering for them, but it's not a security issue for the host unless there is a flaw in the hypervisor.

- Annotations that configure the guest, e.g. memory size, do require scrutiny. For example, memory could be use for denial-of-service attacks. We need to have a discussion on the mailing list on how to fix it. While there is a security risk, it is somewhat second-order relative to the ability to copy all data from the host to some remote machine or wipe its content clean.

> I would like to ask what is the long term plan going your direction?

I will split your question in three:

a) Is there a long term plan for the path filtering options in the patch I submitted: I tried to design them to address your earlier comment #5 and #8. I explained why a *global* option does not work, but I modified the *per-path* option to address your concern about making "free-for-all" possible (although the regexp approach has the additional benefit that you can restrict the free-for all, e.g. allow only /usr/bin/qemu.* and not any path).

b) Is there a long term plan for other annotations: Yes, they need to be reviewed carefully. I have convinced myself that there are secondary security holes in other annotations that can be addressed later, starting with memory allocation. I can start working on this if you want me to.

c) Is there a long-term plan for controlling the annotation feature itself: Re-reading your comments, I realize I may have missed one possible interpretation, which is that you believed my patch filtered annotations by name, instead of filtering the value of the annotations. In other words, my current patch filters what values of hypervisor.path are allowed, it does not enable/disable the hypervisor.path annotation. We could decide that it makes sense to also have an "annotations" configuration that filters which annotations names are whitelisted.

> I would prefer a simple boolean flag (which is also a quick fix)
> if there is no long term plan on how the other options are handled.

I understand. But please review my patch, as I believe now that it may not be exactly what you thought it was.


> To be specific, I was asking:
> 1. will there be a classification of user-modifiable options and non-user-modifiable options?
> 2. will sysadmin be able to modify the classification?

This is my point c) above. I believe this is easily added, but it's somewhat orthogonal to the proposed fix. It would not be sufficient from a security point of view, because the security issue arises from being able to pass arbitrary values to hypervisor.path, not from the feature itself. In other words, if you give a mechanism to whitelist the "hypervisor.path" feature, as soon as you whitelist it, the security risk is back. So even if we implement annotation name whitelisting, we still need annotation value whitelisting.

> If I understand your messages correctly, the answers are two YES.
> Then I agree with your solution. But please confirm if we are on the same page.

I believe the answer is yes, but please check by reviewing the patch.

> And w.r.t. filing new CVEs, anyone can file a CVE.
> VMT will be responsible for triaging the security issue and coordinate
> the progressive disclosure of a vulnerability. We've handled several
> CVEs before and they all come from outside reporters.

Let me make the questions more direct: do you want me to open the CVE
or is is more polite to let someone from the kata VMT do it? I just
want to make sure someone feels they "own" this.