Qubes Admin API #853

Closed
marmarek opened this Issue Mar 8, 2015 · 22 comments

Comments

Projects
None yet
4 participants
Owner

marmarek commented Mar 8, 2015

Reported by joanna on 10 May 2014 12:56 UTC
https://groups.google.com/d/msg/qubes-devel/f2gDpXE3NJ8/_kH7LUrzJ80J

Migrated-From: https://wiki.qubes-os.org/ticket/853


TODO list, aggregated from commits below:

  • a method to get VM statistics (memory usage, cpu time etc), maybe also a variant returning this as events (to avoid polling in tools like qubes-manager)?
  • missing handling of global properties: I propose mgmt.property. prefix with exactly the same set of actions as for mgmt.vm.property.. Or maybe mgmt.global.property. ?
  • Very limited error reporting - for example the reason why VM startup failed (no such VM, not enough memory, missing files, etc)
  • mgmt.vm.property.List and mgmt.vm.property.Help are bound to VM instance at API level, while in practice those are bound to VM class. Generally I'm ok with that, but I face a practical issue: don't know how to properly handle docstrings for client-side wrappers. Our API have vm.property_list, so this part is not a problem (IMO dir(vm) not working is not a problem), but no idea how to handle help(VMClass.property). Maybe shouldn't? And handle only docstring on the object(s) returned by vm.property_list?
  • missing calls for cloning - either at VM level (mgmt.vm.Clone), or individual elements (mgmt.vm.property.Clone, mgmt.vm.volume.Clone etc). I'm in favor of the first option (full VM clone with just new name+qid).
  • No way to distinguish empty return data from failed call (at local socket level). Maybe qubesd should always respond with OK/FAIL on the first line and only then with actual data - this could be translated by qubesd-query to exit code.
  • A way to query for vm -> label colour for GUI domain.
  • Some way of editing labels themselves? (see #2523 where I got)
  • A way to query state of a single VM (and maybe VM existence check at the same time). Proposal: allow calling mgmt.vm.List to a specific VM, not only to dom0.
  • Serialization of VM type (like 'netvm' property). Should it be just string? Then stubs deserializing it will have no idea it's more than string (vm.netvm.ip will not work). Maybe we need explicit type declaration in result of *.Get methods - instead of default=yes|no value, it would be default=yes|no type=... value?
  • qrexec call argument cannot have : - we need a better idea for passing volume identifier (pool:vid currently). Or allow : in qrexec.
  • volume id (vid) may contains chars not allowed in qrexec argument - for example /; we need to encode it somehow, or think of some other way (keep only pool name in argument and move vid to data stream?)
  • no way to create StandaloneVM out of TemplateVM (previously having separate add_new_vm and clone_disk_files was used for that),
  • mgmt.vm.Clone operation may be used to bypass some policy - for example user is not allowed to change netvm property of some VM (like force using some application through Tor), but can clone VM under another name and then change it.
  • admin.vm.Shutdown should shutdown netvm even if is has vms connected to it
  • check if failed start sends any event about vm.kill() called from except: clause
  • features: the tool, if choses to, should set empty string for common "false" values; (document it)
  • draw nice FSM of events (possibly similar to https://qubes-os.readthedocs.io/projects/core-admin/en/latest/qubes-vm/qubesvm.html#qubes.vm.qubesvm.QubesVM.get_power_state)

@marmarek marmarek added this to the Release 3 milestone Mar 8, 2015

@marmarek marmarek modified the milestones: Release 3.1, Release 3.0 May 13, 2015

@marmarek marmarek modified the milestones: Release 4.1, Release 3.1 Feb 8, 2016

andrewdavidwong added a commit that referenced this issue Jun 9, 2016

Member

woju commented Nov 18, 2016

Just pushed a draft of the API, to the qubes-doc, since the table is messy and won't be readable in the issue.

https://www.qubes-os.org/doc/mgmt1/

Didn't link anywhere yet.

@andrewdavidwong could you look on the page? It breaks the layout horribly and I don't know how to make this neither pretty nor readable. I took a shortcut and embedded a <style> directly in the page, which is bad, too.

Member

andrewdavidwong commented Nov 18, 2016

@woju: No problem; I'll work on this soon.

andrewdavidwong added a commit to QubesOS/qubes-doc that referenced this issue Nov 19, 2016

andrewdavidwong added a commit to QubesOS/qubes-doc that referenced this issue Nov 19, 2016

Member

andrewdavidwong commented Nov 19, 2016

@woju: How does it look now? (Note: You might need to clear your cache to get the new CSS.)

andrewdavidwong added a commit to QubesOS/qubes-doc that referenced this issue Nov 19, 2016

Clean up table source
* Use backticks to wrap literal text
* Replace HTML entities with ASCII characters
* Make column spacing uniform

QubesOS/qubes-issues#853
Member

woju commented Nov 19, 2016

Thanks. It is prettier, though it's less readable, since it gets scrolled independent of page, and wraps the cells. I disabled it on purpose, because if there is <name> class=<class> state=<state>\n, I didn't want it to get on multiple lines.

Member

andrewdavidwong commented Nov 19, 2016

Thanks. It is prettier, though it's less readable, since it gets scrolled independent of page,

The only way to avoid that is to use a different layout for the page. It sounds like maybe you don't want it to have the doc layout with the right-hand side bar. If the table doesn't get clipped, it'll simply overlap that side bar and be even less readable.

and wraps the cells. I disabled it on purpose, because if there is class= state=\n, I didn't want it to get on multiple lines.

Ok.

Owner

marmarek commented Nov 19, 2016

Scrolling horizontally IMO isn't a problem - vertically is.

Member

andrewdavidwong commented Nov 19, 2016

If the table doesn't get clipped vertically, it'll be larger than the height of most viewports. My impression is that this is bad practice in web design, but I don't have any source to back that up.

Member

andrewdavidwong commented Nov 19, 2016

What if we create a full layout version of the table that takes up an entire page and provide a link from the existing page underneath the clipped table?

Alternatively, we could convert the entire page into the full layout and lose the side bar.

Member

woju commented Nov 19, 2016

I don't mind the side bar disappearing at all. And, as Marek said, table shouldn't be vertically scrollable independent of the page, because it makes it very annoying on touchscreen.

You may be right in the sense that nowadays it is fashionable to design custom scrolling, those designers do it and people get used to that, but IMHO this is not usable.

It may be valuable to devise some CSS class for the tables in documentation, since I will have at least two more such tables, for VM "events" and "features" (new functionality in R4.0 core).

But we are getting sidetracked.

andrewdavidwong added a commit to QubesOS/qubesos.github.io that referenced this issue Nov 19, 2016

andrewdavidwong added a commit to QubesOS/qubesos.github.io that referenced this issue Nov 19, 2016

andrewdavidwong added a commit to QubesOS/qubes-doc that referenced this issue Nov 19, 2016

Member

andrewdavidwong commented Nov 19, 2016

@woju @marmarek: Let me know what you think now.

Member

woju commented Nov 19, 2016

Great! Thank you.

Member

kalkin commented Nov 21, 2016

@woju What should mgmt1.vm.volume.List list?

Member

woju commented Nov 23, 2016

@kalkin some identifier about the volume and possibly very basic info, but I'm not sure what exactly.

The idea is that there are two set of calls, mgmt1.vm.volume.* and mgmt1.pool.volume.*, to allow for expressing the policy decisions "allow management of pool" and "allow management of storage for VM". They overlap in scope, on purpose. Currently I don't know who will use which one and in what context.

woju added a commit to QubesOS/qubes-doc that referenced this issue Nov 24, 2016

services/mgmt1: various updates
- remove `1` from RPC names
- change `mgmt.vm.Create` wrt class
- misc notes and TODOs

QubesOS/qubes-issues#853
Owner

marmarek commented Feb 13, 2017

Issue(s) with current API draft:

  • missing handling of global properties: I propose mgmt.property. prefix with exactly the same set of actions as for mgmt.vm.property.. Or maybe mgmt.global.property. ?
  • Very limited error reporting - for example the reason why VM startup failed (no such VM, not enough memory, missing files, etc)
  • mgmt.vm.property.List and mgmt.vm.property.Help are bound to VM instance at API level, while in practice those are bound to VM class. Generally I'm ok with that, but I face a practical issue: don't know how to properly handle docstrings for client-side wrappers. Our API have vm.property_list, so this part is not a problem (IMO dir(vm) not working is not a problem), but no idea how to handle help(VMClass.property). Maybe shouldn't? And handle only docstring on the object(s) returned by vm.property_list?
  • missing calls for cloning - either at VM level (mgmt.vm.Clone), or individual elements (mgmt.vm.property.Clone, mgmt.vm.volume.Clone etc). I'm in favor of the first option (full VM clone with just new name+qid).

edited by @woju: deleted checkboxes

Owner

marmarek commented Feb 13, 2017

And issues with current qubesd implementation:

  • No way to distinguish empty return data from failed call (at local socket level). Maybe qubesd should always respond with OK/FAIL on the first line and only then with actual data - this could be translated by qubesd-query to exit code.

edited by @woju: deleted checkboxes

Member

woju commented Feb 22, 2017

  • A way to query for vm -> label colour for GUI domain.
  • Some way of editing labels themselves? (see #2523 where I got)

edited by @woju: deleted checkboxes

Owner

marmarek commented Feb 22, 2017

A way to query for vm -> label colour for GUI domain.

mgmt.vm.property.Get+label? But it will give only label name, not other properties (color, icon). Maybe we need mgmt.vm.label.*? But the target of this call can't be a label, must be some VM (probably dom0).

Also:

  • A way to query state of a single VM (and maybe VM existence check at the same time). Proposal: allow calling mgmt.vm.List to a specific VM, not only to dom0.

edited by @woju: deleted checkboxes

Owner

marmarek commented Feb 23, 2017

  • Serialization of VM type (like 'netvm' property). Should it be just string? Then stubs deserializing it will have no idea it's more than string (vm.netvm.ip will not work). Maybe we need explicit type declaration in result of *.Get methods - instead of default=yes|no value, it would be default=yes|no type=... value?

Note that *.Set calls don't have this problem, because qubesd know what type should be there, and it's responsibility of appropriate handler to convert string to appropriate type. Actually it would be wrong to pass any type information here - it would make parsing of untrusted input more complex.

edited by @woju: deleted checkboxes

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Apr 7, 2017

qubesd: add second socket for in-dom0 internal calls
This socket (and commands) are not exposed to untrusted input, so no
need to extensive sanitization. Also, there is no need to provide a
stable API here, as those methods are used internally only.

QubesOS/qubes-issues#853

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Apr 10, 2017

tools: remove qvm-ls tool and related integration in qubes.property
qvm-ls tool (as all other tools) will be accessing properties through
API, so no need (nor sense) for this tool-specific attributes in
qubes.property. The only somehow used was ls_width, and in fact it made
the output unnecessary wide.

The tool itself is already moved to core-mgmt-client repository.

QubesOS/qubes-issues#853

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Apr 10, 2017

tools: remove qvm-ls tool and related integration in qubes.property
qvm-ls tool (as all other tools) will be accessing properties through
API, so no need (nor sense) for this tool-specific attributes in
qubes.property. The only somehow used was ls_width, and in fact it made
the output unnecessary wide.

The tool itself is already moved to core-mgmt-client repository.

QubesOS/qubes-issues#853

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Apr 21, 2017

tools: remove qvm-ls tool and related integration in qubes.property
qvm-ls tool (as all other tools) will be accessing properties through
API, so no need (nor sense) for this tool-specific attributes in
qubes.property. The only somehow used was ls_width, and in fact it made
the output unnecessary wide.

The tool itself is already moved to core-mgmt-client repository.

QubesOS/qubes-issues#853
Owner

marmarek commented Apr 25, 2017

Another problems with the current API design:

  • no way to create StandaloneVM out of TemplateVM (previously having separate add_new_vm and clone_disk_files was used for that),
  • mgmt.vm.Clone operation may be used to bypass some policy - for example user is not allowed to change netvm property of some VM (like force using some application through Tor), but can clone VM under another name and then change it.

The second one would be in fact a problem with a specific qrexec policy, but IMO an easy to make one. Using tags instead of VM names to identify sources/targets would somehow mitigate it (tags are preserved with clone/rename etc), but not all places accept tags (for example you can't use tag in target= parameter). Maybe we need to decouple clone operation to separate "create VM", "copy properties", and "copy disk files", as it was in the old API?

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue May 1, 2017

tools: remove qvm-ls tool and related integration in qubes.property
qvm-ls tool (as all other tools) will be accessing properties through
API, so no need (nor sense) for this tool-specific attributes in
qubes.property. The only somehow used was ls_width, and in fact it made
the output unnecessary wide.

The tool itself is already moved to core-mgmt-client repository.

QubesOS/qubes-issues#853

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue May 1, 2017

devices: adjust API documentation
Device ident may contain only characters allowed in qrexec argument.
This will allow using it directly in qrexec argument in Attach/Detach
methods.
This also means PCI extension will need to be updated (it uses ':' in
ident).

QubesOS/qubes-issues#853

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue May 9, 2017

tools: remove qvm-ls tool and related integration in qubes.property
qvm-ls tool (as all other tools) will be accessing properties through
API, so no need (nor sense) for this tool-specific attributes in
qubes.property. The only somehow used was ls_width, and in fact it made
the output unnecessary wide.

The tool itself is already moved to core-mgmt-client repository.

QubesOS/qubes-issues#853

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue May 10, 2017

devices: adjust API documentation
Device ident may contain only characters allowed in qrexec argument.
This will allow using it directly in qrexec argument in Attach/Detach
methods.
This also means PCI extension will need to be updated (it uses ':' in
ident).

QubesOS/qubes-issues#853
Owner

marmarek commented May 11, 2017

Next thing to do: rename "Mgmt API" to "Admin API", as it should be from the beginning.

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue May 11, 2017

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue May 11, 2017

marmarek added a commit to QubesOS/qubes-doc that referenced this issue May 11, 2017

marmarek added a commit to QubesOS/qubes-core-admin-client that referenced this issue May 11, 2017

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue May 12, 2017

tools: remove qvm-ls tool and related integration in qubes.property
qvm-ls tool (as all other tools) will be accessing properties through
API, so no need (nor sense) for this tool-specific attributes in
qubes.property. The only somehow used was ls_width, and in fact it made
the output unnecessary wide.

The tool itself is already moved to core-mgmt-client repository.

QubesOS/qubes-issues#853

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue May 12, 2017

devices: adjust API documentation
Device ident may contain only characters allowed in qrexec argument.
This will allow using it directly in qrexec argument in Attach/Detach
methods.
This also means PCI extension will need to be updated (it uses ':' in
ident).

QubesOS/qubes-issues#853

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue May 12, 2017

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue May 12, 2017

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue May 12, 2017

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue May 12, 2017

marmarek added a commit to QubesOS/qubes-core-admin-client that referenced this issue May 12, 2017

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue May 23, 2017

admin-api: create and install actual Admin API RPC endpoints
Install files in /etc/qubes-rpc for all methods defined in API
documentation, even if not yet implemented (qubesd will handle it
raising appropriate exception).
Use minimal program written in C (qubesd-query-fast), instead of
qubesd-query in python for performance reasons:
 - a single qubesd-query run: ~300ms
 - equivalent in shell (echo | nc -U): ~40ms
 - qubesd-query-fast: ~20ms

Many tools makes multiple API calls, so performance here do matter. For
example qvm-ls (from VM) currently takes about 60s on a system with 24
VMs.

Also make use of `$include:` directive in policy file, to make it easier
defining a VM with full Admin API access.

QubesOS/qubes-issues#853

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue May 23, 2017

admin: raise QubesNoSuchPropertyError for non-existing properties
Accessing non-existing property is a common action (for example
hasattr() do try to access the property). So, introduce specific
exception, inheriting from AttributeError. It will behave very similar
to standard (non-Admin-API) property access.

This exception is reported to the Admin API user, so it will be possible
to distinguish between non-existing property and access denied. But it
isn't any significant information leak, as list of valid properties is
publicly available in the source code.

QubesOS/qubes-issues#853

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue May 23, 2017

qubesd: reduce verbosity
Remove debug prints, log full traceback (of handled exception) only when
debug mode enabled (--debug, introduce in this commit too).
--debug option also enables sending tracebacks to the API clients.

QubesOS/qubes-issues#853

marmarek added a commit to QubesOS/qubes-core-admin-client that referenced this issue May 25, 2017

storage: implement admin.vm.volume.Import as volume.import_data
Use newly introduced payload_stream= argument to qubesd_call to pass
data directly from some file-like object - without loading it all into
memory.

QubesOS/qubes-issues#853

marmarek added a commit to QubesOS/qubes-core-admin-client that referenced this issue May 25, 2017

storage: implement admin.vm.volume.Import as volume.import_data
Use newly introduced payload_stream= argument to qubesd_call to pass
data directly from some file-like object - without loading it all into
memory.

QubesOS/qubes-issues#853

marmarek added a commit to QubesOS/qubes-core-admin-client that referenced this issue May 25, 2017

storage: implement admin.vm.volume.Import as volume.import_data
Use newly introduced payload_stream= argument to qubesd_call to pass
data directly from some file-like object - without loading it all into
memory.

QubesOS/qubes-issues#853

@qubesos-bot qubesos-bot referenced this issue in QubesOS/updates-status Jul 5, 2017

Closed

core-admin-client v4.0.1 (r4.0) #116

marmarek added a commit to marmarek/qubes-doc that referenced this issue Jul 27, 2017

marmarek added a commit to marmarek/qubes-doc that referenced this issue Jul 27, 2017

marmarek added a commit to marmarek/qubes-doc that referenced this issue Jul 27, 2017

marmarek added a commit to marmarek/qubes-doc that referenced this issue Jul 27, 2017

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Jul 27, 2017

app: refresh getting VM statistics, rename to QubesHost.get_vm_stats
Get a VM statistics once. If previous measurements are provided,
calculate difference too. This is backend part of upcoming
admin.vm.Stats service.

QubesOS/qubes-issues#853

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Jul 27, 2017

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Jul 28, 2017

marmarek added a commit to marmarek/qubes-core-admin that referenced this issue Jul 28, 2017

@marmarek marmarek closed this Oct 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment