Skip to content

STABLE-9: compat error checking additions#103

Merged
jean-edouard merged 3 commits intoOpenXT:stable-9from
jandryuk:stable-9-compat-fixes
Apr 22, 2019
Merged

STABLE-9: compat error checking additions#103
jean-edouard merged 3 commits intoOpenXT:stable-9from
jandryuk:stable-9-compat-fixes

Conversation

@jandryuk
Copy link
Copy Markdown
Contributor

@jandryuk jandryuk commented Apr 17, 2019

The compat codepaths were missing some error checking which this PR adds.

mount_upgrade_compat wasn't exiting on failure, so execution continued. Return an error for that case.

mount_upgrade_compat can fail if it wasn't written into a logical volume. We need create_lv to return an error when it cannot create an LV.

While touching the seal_system, prefix the seal commands with do_cmd so they are logged.

NOTE: Regardless of this PR, we don't abort the installer on forward sealing failure. This means we'll reboot and fail the measured launch. At the time seal_system is called, all the files have been updated, so there isn't a way to fail gracefully.

"install-main: Return failure from create_lv " is a cherry pick from the master PR.
"install-main: Add do_cmd to seal_system" is a modified version of the master PR to include the compat code paths.

Master PR #104

When lvcreate fails we should bubble the error up to the caller.

During development, I had a huge debugging dom0 which left only 24MB
free in the volume group after being written to /dev/xenclient/root.new.
This wasn't enough for the 143MB upgrade-compat image, so its LV could
not be created.  The error was not caught, so execution continued until
install-bootloader eventually failed and failed the OTA.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
(cherry picked from commit a569cc6)
Use do_cmd so we have better insight into seal_system's operation.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>

This is a stable-9 version of ec21df6 to include the compat
upgrade case.
If we can't mount upgrade-compat, we should error our before trying to
call additional commands.  This better indicates where the error lies.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
@jandryuk
Copy link
Copy Markdown
Contributor Author

There may be an issue with TPM 1.2. I'm investigating.

@jean-edouard
Copy link
Copy Markdown
Member

@jean-edouard
Copy link
Copy Markdown
Member

jean-edouard commented Apr 17, 2019

LGTM, merging soon.

@jandryuk
Copy link
Copy Markdown
Contributor Author

A TPM 1.2 box failed a forward seal, but I didn't finish investigating.

There are two new AVCs for modprobe kmod_t inheriting updatemgr_t FDs. I think the should be benign.

I'll try and investigate more tomorrow, but maybe adding do_cmd to seal_system should be held off for now.

@jean-edouard
Copy link
Copy Markdown
Member

Ok, thanks!
My TPM 1.2 box successfully forward-sealed.

@jean-edouard
Copy link
Copy Markdown
Member

@jandryuk should we still hold off on this?

@jandryuk
Copy link
Copy Markdown
Contributor Author

I think this PR is fine to go in. I've been running it a bunch and it doesn't seem to be a problem. This morning I saw a failed forward seal of a TPM 1.2 legacy boot system, but I OTA-ed it with the same installer and that one successfully forward sealed. Something is going on, but it's not this PR.

The kmod_t AVCs I noted above aren't from do_cmd or the >&2 redirects. I don't know if they are new or if they've always been there.

@jean-edouard
Copy link
Copy Markdown
Member

Cool, thanks, merging soon then.
I've seen the kmod AVCs for a while now, they're really weird... It's like kmod_t is trying to load a kmod_t module, which doesn't exist...

@jandryuk
Copy link
Copy Markdown
Contributor Author

I was referring to these two:

kernel:[ 1632.053192] audit: type=1400 audit(1555512417.960:5): avc: denied { use } for pid=20149 comm="modprobe" path="pipe:[142749]" dev="pipefs" ino=142749 scontext=system_u:system_r:kmod_t:s0-s0:c0.c1023 tcontext=system_u:system_r:updatemgr_t:s0-s0:c0.c1023 tclass=fd permissive=0
kernel:[ 1632.055475] audit: type=1400 audit(1555512417.960:6): avc: denied { use } for pid=20149 comm="modprobe" path="socket:[14774]" dev="sockfs" ino=14774 scontext=system_u:system_r:kmod_t:s0-s0:c0.c1023 tcontext=system_u:system_r:updatemgr_t:s0-s0:c0.c1023 tclass=fd permissive=0

I think they are from modprobe txt inside ml-functions (sourced by seal-system) using the inherited file descriptors.

@jean-edouard jean-edouard merged commit 8398a42 into OpenXT:stable-9 Apr 22, 2019
@dpsmith
Copy link
Copy Markdown
Member

dpsmith commented Apr 23, 2019

@jandryuk, should we open a ticket for someone to review ml-functions for any SELinux malfeasance?

@jandryuk jandryuk deleted the stable-9-compat-fixes branch April 23, 2019 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants