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.

Sign up for GitHub

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

Jump to bottom

systemd-run seems to be not working #13

Open
mvo5 opened this issue Dec 3, 2020 · 12 comments
Open

systemd-run seems to be not working #13

mvo5 opened this issue Dec 3, 2020 · 12 comments
Labels
bug Something isn't working

Comments

@mvo5
Copy link
Contributor

mvo5 commented Dec 3, 2020

I tried to use systemd-run in the initramfs to run the "fde-reveal-key" binary. It seems this is not quite working yet, see the attached screenshot.
Screenshot from 2020-12-03 16-18-07
@xnox
Copy link
Contributor

xnox commented Dec 3, 2020

Ah, cute.

Will check...... it might be that systemd-run requires dbus to be up. I would have hoped that it can work without dbus.... over the private socket.

@xnox xnox added the bug Something isn't working label Dec 3, 2020
@mvo5
Copy link
Contributor Author

mvo5 commented Dec 3, 2020

If it's too hard/annoying just let us know, it would be lovely to be able to use it but we can try to talk to the private socket if the systemd-run binary is doing weird stuff

@xnox
Copy link
Contributor

xnox commented Dec 3, 2020

If it's too hard/annoying just let us know, it would be lovely to be able to use it but we can try to talk to the private socket if the systemd-run binary is doing weird stuff

i will double check things. It should be able to do all the things. And no, one would not want to reimplement the systemd-run argument regeneration.

@mvo5 mvo5 mentioned this issue Dec 8, 2020
Merged
mvo5 added a commit to mvo5/snappy that referenced this issue Dec 8, 2020
@mvo5
secboot: hack out systemd-run, see canonical/core-initrd#13
df3dce4
@xnox
Copy link
Contributor

xnox commented Dec 11, 2020

@mvo what are the exact arguments use to systemd-run?

        if (arg_wait || arg_stdio != ARG_STDIO_NONE)
                r = bus_connect_transport(arg_transport, arg_host, arg_user, &bus);
        else
                r = bus_connect_transport_systemd(arg_transport, arg_host, arg_user, &bus);
        if (r < 0)
                return log_error_errno(r, "Failed to create bus connection: %m");

It looks like if wait is used; or one wants to collect output / connect to --pipe or --pty; we must ship dbus.service and start it.

Otherwise local transport (private systemd socket) is used.

@mvo5
Copy link
Contributor Author

mvo5 commented Dec 11, 2020

We use pipe, we could maybe try to avoid doing that. Need to look but we need the resulting output.

@mvo5
Copy link
Contributor Author

mvo5 commented Dec 11, 2020

@xnox in various meetings but I can try to drive this without --pipe or --pty

@xnox
Copy link
Contributor

xnox commented Dec 11, 2020

@xnox in various meetings but I can try to drive this without --pipe or --pty

$ systemd-run -p StandardOutput=append:/run/stdout hello
Running as unit: run-u166.service
$ cat /run/stdout 
Hello, world!

Most stanzas of any unit can be set via -p "universal property" option.

@xnox
Copy link
Contributor

xnox commented Dec 11, 2020

note that stnadardOutput takes a socket or fd:name too. So you could invoke this in such a way that it goes direct to socket or fd that is slurped up by snapd.

To avoid leaving any files on any filesystem.

@mvo5
Copy link
Contributor Author

mvo5 commented Dec 11, 2020

Hm, we also use "--wait" :/ so this requires some way via systemctl to wait for this in a nice way

@mvo5
Copy link
Contributor Author

mvo5 commented Dec 11, 2020

@pedronis Not being able to use "--wait" is a bummer, let's sync on Monday and talk about ditching systemd-run and doing something using seccomp-golang instead.

@xnox
Copy link
Contributor

xnox commented Dec 12, 2020

@pedronis Not being able to use "--wait" is a bummer, let's sync on Monday and talk about ditching systemd-run and doing something using seccomp-golang instead.

if you need wait, i can ship dbus, dbus.service and then it will work. will make initrd larger and have on more daemon running just to call this thing.

Instead of wait, you can do like ExecStartPost=snap fde-util-finished or some such. But it means async code, needing a goroutine and an await.

@mvo5 mvo5 mentioned this issue Dec 14, 2020
Merged
mvo5 added a commit to mvo5/snappy that referenced this issue Dec 14, 2020
@mvo5
secboot: workaround snapcore/core-initrd issue #13
21fb026 
This commit works around canonical/core-initrd#13, i.e. we cannot run
```
systemd-run --wait --pipe
```
in the initrd because there is no dbus there and those features need
dbus.

So this PR implements a workaround that uses polling to get the result
and tmp files. Not nice but workable.
mvo5 added a commit to canonical/snapd that referenced this issue Dec 15, 2020
@mvo5 @anonymouse64
secboot: add workaround for snapcore/core-initrd issue #13
4ee1ee8 
This commit works around canonical/core-initrd#13, i.e. we cannot run
```
systemd-run --wait --pipe
```
in the initrd because there is no dbus there and those features need
dbus.

So this PR implements a workaround that uses polling to get the result
and tmp files. Not nice but workable.

* secboot/secboot_tpm.go: re-add useful comment

Signed-off-by: Ian Johnson <[email protected]>

* secboot/secboot_tpm.go: increase frequency we check for hook completion

The resolution of how often we check directly affects boot speed, since we need
to wait at least one cycle if the file is not created instantly, so rather than
always hold up boot for at least one second, make the time period between checks
smaller but not too small such that we are hogging CPU cycles checking for a
file. 20 Hz seems a reasonable compromise for this.

Signed-off-by: Ian Johnson <[email protected]>

* secboot/secboot_tpm.go: manually delete and recreate the std{in,out,err} streams

We want to be robust and secure, so to guard against possible bugs where the
files weren't properly deleted and/or have the wrong permissions, we always
should delete and re-create the stream files with the right permissions before
executing the hook itself.

Also add a test with a hook implementation which verifies the files have the 
right permissions and content that we expect.

Signed-off-by: Ian Johnson <[email protected]>

* secboot/secboot_tpm_test.go: fix typo

Signed-off-by: Ian Johnson <[email protected]>

Co-authored-by: Ian Johnson <[email protected]>
mvo5 added a commit to canonical/snapd that referenced this issue Dec 15, 2020
@mvo5 @anonymouse64
secboot: add workaround for snapcore/core-initrd issue #13
c500e38 
This commit works around canonical/core-initrd#13, i.e. we cannot run
```
systemd-run --wait --pipe
```
in the initrd because there is no dbus there and those features need
dbus.

So this PR implements a workaround that uses polling to get the result
and tmp files. Not nice but workable.

* secboot/secboot_tpm.go: re-add useful comment

Signed-off-by: Ian Johnson <[email protected]>

* secboot/secboot_tpm.go: increase frequency we check for hook completion

The resolution of how often we check directly affects boot speed, since we need
to wait at least one cycle if the file is not created instantly, so rather than
always hold up boot for at least one second, make the time period between checks
smaller but not too small such that we are hogging CPU cycles checking for a
file. 20 Hz seems a reasonable compromise for this.

Signed-off-by: Ian Johnson <[email protected]>

* secboot/secboot_tpm.go: manually delete and recreate the std{in,out,err} streams

We want to be robust and secure, so to guard against possible bugs where the
files weren't properly deleted and/or have the wrong permissions, we always
should delete and re-create the stream files with the right permissions before
executing the hook itself.

Also add a test with a hook implementation which verifies the files have the 
right permissions and content that we expect.

Signed-off-by: Ian Johnson <[email protected]>

* secboot/secboot_tpm_test.go: fix typo

Signed-off-by: Ian Johnson <[email protected]>

Co-authored-by: Ian Johnson <[email protected]>
@mvo5
Copy link
Contributor Author

mvo5 commented Dec 15, 2020

We implemented a workaround now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@xnox @mvo5