Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

varlink: add introspection support + varlinkctl + varlinkify one first command line tool (systemd-pcrextend) #29325

Merged
merged 18 commits into from
Oct 6, 2023

Conversation

poettering
Copy link
Member

This beefs up our Varlink game: it adds support for Varlink introspection to our codebase, i.e. this stuff: https://varlink.org/Service

Introspection matters as we make our Varlink services more accessible ourside of systemd.

A new tool "varlinkctl" is added for interfacing with Varlink services. It's really nice any easy to use, for example:

$ varlinkctl info /run/systemd/resolve/io.systemd.Resolve

will introspect what the Varlink service behind the /run/systemd/resolve/io.systemd.Resolve socket provides. Equally nice:

# varlinkctl info /usr/lib/systemd/systemd-pcrextend

to see what Varlink interfaces the specified binary offers.

This also adds some glue to make it very easy to add Varlink APIs to command line tools, and then does that to systemd-pcrextend, making an IPC API available that allows measuring arbitrary strings to a TPM2 PCR, with an event log record associated with it.

Copy link
Contributor

@DaanDeMeyer DaanDeMeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I eventually want to hook up mkosi to a socket like this as well. Preferably I wouldn't have to pull in a python wrapper of libvarlink to be able to do so. Is there any way we can put the varlink bits of this in the socket unit itself and have --varlink instruct the binary to read a JSON object directly from stdin which comes from varlink? That makes this accessible to many more tools without requiring all those tools to pull in a varlink library.

@poettering
Copy link
Member Author

I don't follow?

@poettering
Copy link
Member Author

poettering commented Sep 26, 2023

(there's a native python varlink module btw. given how trivial varlink actually is it's generally a better idea to use a native library for each language rather than wrapping systemd's C version)

@DaanDeMeyer
Copy link
Contributor

I still find varlink annoying to pull in as a dep, it means I have to make sure it is packaged in all distributions, keep track of updates, it feels messy to require each tool to implement this independently. It would be much nicer if I just had to handle JSON and could leave the varlink bits to systemd. What would be much nicer is if systemd shipped a helper tool that does the varlink specific bits which then just calls the actual tool, passing the input as a JSON object on stdin. Then all I need is to parse JSON, which is much more trivial than having to do varlink stuff, which most people have never heard of.

@DaanDeMeyer
Copy link
Contributor

Actually, thinking more about it, such a tool can be implemented independently anyway. Our tools can do varlink directly, and tools that can't do it directly could use such a wrapper tool to expose themselves over varlink. But such a wrapper tool can be added later

@poettering
Copy link
Member Author

I still find varlink annoying to pull in as a dep, it means I have to make sure it is packaged in all distributions, keep track of updates, it feels messy to require each tool to implement this independently. It would be much nicer if I just had to handle JSON and could leave the varlink bits to systemd. What would be much nicer is if systemd shipped a helper tool that does the varlink specific bits which then just calls the actual tool, passing the input as a JSON object on stdin. Then all I need is to parse JSON, which is much more trivial than having to do varlink stuff, which most people have never heard of.

varlink is just json, check the spec. it's trivial to reimplement: the method call ist just a json object followed by a single NUL byte, and the response is too. That is the whole protocol.

So if you just want to do json in mkosi, that's entirely fine, but you really don't need varlinkctl for that.

What varlinkctl gives you is introspection, syntax highlighting and stuff. But you don't need any of this for basic varlink.

@bluca
Copy link
Member

bluca commented Sep 26, 2023

The varlink python library is already in fedora, I'll make it available in debian/ubuntu too

@evverx
Copy link
Member

evverx commented Sep 26, 2023

@mrc0mmand looks like it should close #23874 too. I haven't taken a close look yet but it seems with this PR included it should be possible to introspect the systemd services. I'm not sure how to get the list of all the services automatically (apart from looking for the *.io files or hard-coding them)

varlinkctl info /run/systemd/resolve/io.systemd.Resolve
    Vendor: The systemd Project
   Product: systemd (systemd-resolved)
   Version: 254 (254-1250-g1cf432d)
   Version: https://systemd.io/
Interfaces: io.systemd
            io.systemd.Resolve
            org.varlink.service
=================================================================
==10239==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c0000000ab at pc 0x7f13c5926dac bp 0x7fffdf3c2c90 sp 0x7fffdf3c2c88
READ of size 1 at 0x60c0000000ab thread T0
    #0 0x7f13c5926dab in json_variant_unref ../src/shared/json.c:864
    #1 0x4032f9 in json_variant_unrefp ../src/shared/json.h:96
    #2 0x405aff in verb_info ../src/varlinkctl/varlinkctl.c:201
    #3 0x7f13c5b358a0 in dispatch_verb ../src/shared/verbs.c:170
    #4 0x4089ab in varlinkctl_main ../src/varlinkctl/varlinkctl.c:502
    #5 0x4089f1 in run ../src/varlinkctl/varlinkctl.c:514
    #6 0x408aad in main ../src/varlinkctl/varlinkctl.c:518
    #7 0x7f13c4a49b89 in __libc_start_call_main (/lib64/libc.so.6+0x27b89) (BuildId: c9f62793b9e886eb1b95077d4f26fe2b4aa1ac25)
    #8 0x7f13c4a49c4a in __libc_start_main_alias_2 (/lib64/libc.so.6+0x27c4a) (BuildId: c9f62793b9e886eb1b95077d4f26fe2b4aa1ac25)
    #9 0x4025c4 in _start (/home/vagrant/systemd/build/varlinkctl+0x4025c4) (BuildId: 7db282c724ffc0f72d298f583db879607c84339b)

0x60c0000000ab is located 107 bytes inside of 120-byte region [0x60c000000040,0x60c0000000b8)
freed by thread T0 here:
    #0 0x7f13c70d7fb8 in __interceptor_free.part.0 (/lib64/libasan.so.8+0xd7fb8) (BuildId: e5f0a0d511a659fbc47bf41072869139cb2db47f)
    #1 0x7f13c5926f8b in json_variant_unref ../src/shared/json.c:872
    #2 0x7f13c5af1159 in varlink_clear_current ../src/shared/varlink.c:595
    #3 0x7f13c5af166f in varlink_clear ../src/shared/varlink.c:610
    #4 0x7f13c5af2b21 in varlink_destroy ../src/shared/varlink.c:645
    #5 0x7f13c5af2f75 in varlink_unref ../src/shared/varlink.c:651
    #6 0x40399e in varlink_unrefp ../src/shared/varlink.h:196
    #7 0x405af0 in verb_info ../src/varlinkctl/varlinkctl.c:202
    #8 0x7f13c5b358a0 in dispatch_verb ../src/shared/verbs.c:170
    #9 0x4089ab in varlinkctl_main ../src/varlinkctl/varlinkctl.c:502
    #10 0x4089f1 in run ../src/varlinkctl/varlinkctl.c:514
    #11 0x408aad in main ../src/varlinkctl/varlinkctl.c:518
    #12 0x7f13c4a49b89 in __libc_start_call_main (/lib64/libc.so.6+0x27b89) (BuildId: c9f62793b9e886eb1b95077d4f26fe2b4aa1ac25)

@DaanDeMeyer
Copy link
Contributor

I guess we won't have a way to get the name of the unit spawned by varlinkctl call so we can tail logs or so?

@poettering
Copy link
Member Author

I guess we won't have a way to get the name of the unit spawned by varlinkctl call so we can tail logs or so?

Actually that should be easily implementable. If the client enables SO_PASSCRED on its side, then it will get SCM_CREDENTIALS metadata for everything the server sends, revealing its identity.

@poettering
Copy link
Member Author

I'm not sure how to get the list of all the services automatically (apart from looking for the *.io files or hard-coding them)

test-varlink-idl spits them all out (but does some other things too).

We probably should install them as .varlink files or so. i.e. generate them on developer machines automatically from the C definitions, then commit them to git (so that we have no cross-building issues) and install them to some dir in /usr/share/varlink/interfaces/ or so. Now if my Meson-fu would be good enough to implement this nicely (i.e. generate from C tool on devel computer, but use git version otherwise)...

@evverx
Copy link
Member

evverx commented Sep 26, 2023

test-varlink-idl spits them all out (but does some other things too).

The idea was to implement vfuzzer (by analogy with dfuzzer) that could discover services and then fuzz them. Ideally it should be separate from systemd (and its test suite) to make it easier to point it to any varlink service so test-varlink-idl isn't an option probably. Based on my experiments find /run/systemd/ -name 'io.*' is enough to discover all the systemd services. The varlink files would probably be nicer though.

@yuwata yuwata added good-to-merge/with-minor-suggestions and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels Oct 6, 2023
This is useful to allow event loops to run exactly as long as there's
something to do but not longer.
src/shared/varlink-idl.c Outdated Show resolved Hide resolved
@poettering poettering added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed good-to-merge/with-minor-suggestions labels Oct 6, 2023
src/shared/varlink-idl.c Outdated Show resolved Hide resolved
The data is not used for anything yet, but this will be added in later
commits.
…stemd interfaces

The official org.varlink.service interface definition, as per:

https://varlink.org/Service

And the io.systemd service where we carry some super generic errors our
Varlink implementation generates.
This error is a private error returned by PID 1 to oomd. It's internal,
and very specific to the use-case. Hence it should not be part of the
org.varlink.service interface (which isn't really our namespace anyway).

Hence, let's clean this up and move it over to the ManagedOOM varlink
interface of PID, where it belongs.

Since this is a private protocol of our two daemons, and the client
(i.e. oomd) doesn't explicitly test for this error anyway we can just
move it over without ill effects.
This adds a logic that if enabled ensures sd_event_exit() is called
whenever the varlink connection count hits zero.

This is useful for implementing pure Varlink services (i.e. services
whose only job is to serve Varlink requests), that shall run only as
long as needed, i.e. as long as at least one request is being served.
This new helper will automatically take listening fds passed in from the
service manager and processes varlink on them. It's useful for Varlink
services that shall be socket activatable.
This is a helper call that runs the specified VarlinkServer object in an
event loop, and exits once no more connections exist.

This is useful for pure varlink servers (i.e. those which only server
varlink requests and do nothing else), to run as long as there's
something to do and exit right after.
This call checks if we are invoked in a socket-activation Varlink server
context. It's useful for commands that can be run from the command line
or as Varlink service and then either serve commands from the cmdline or
those from Varlink.
This is primarily supposed to be a 1st step with varlinkifying our
various command line tools, and excercise in how this might look like
across our codebase one day. However, at AllSystemsGo! 2023 it was
requested that we provide an API to do a PCR measurement along with a
matching event log record, and this provides that.
@yuwata yuwata merged commit 34ba0f5 into systemd:main Oct 6, 2023
44 checks passed
@github-actions github-actions bot removed the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Oct 6, 2023
yuwata added a commit that referenced this pull request Oct 7, 2023
Follow-ups for #29325.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

7 participants