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

Make emulation-check rule is broken #1865

Closed
gendx opened this issue May 19, 2020 · 12 comments
Closed

Make emulation-check rule is broken #1865

gendx opened this issue May 19, 2020 · 12 comments
Assignees

Comments

@gendx
Copy link
Contributor

gendx commented May 19, 2020

As mentioned in #1861 (comment), the make emulation-check is broken when relevant Debian packages are missing.

As a newcomer, it's unclear what to do after the following steps.

$ make emulation-check 
Cloning into 'qemu'...
remote: Enumerating objects: 19007, done.
remote: Counting objects: 100% (19007/19007), done.
remote: Compressing objects: 100% (6200/6200), done.
remote: Total 482048 (delta 14287), reused 16209 (delta 12647), pack-reused 463041
Receiving objects: 100% (482048/482048), 195.57 MiB | 27.38 MiB/s, done.
Resolving deltas: 100% (394231/394231), done.
Checking connectivity... done.

ERROR: pkg-config binary 'pkg-config' not found

Makefile:219: recipe for target 'emulation-setup' failed
make: *** [emulation-setup] Error 1

$ sudo apt install pkg-config
...

$ make emulation-check 
make[1]: *** [config-host.mak] Error 1
Makefile:219: recipe for target 'emulation-setup' failed
make: *** [emulation-setup] Error 2
@alistair23
Copy link
Contributor

The solution seems to be just to remove the pipe to /dev/null. Then you will get a nice error that can be fixed.

I'll send a patch

@gendx
Copy link
Contributor Author

gendx commented May 19, 2020

Replying here to #1861 (comment), given that it's not on-topic for that pull request anymore.

A few things here:

1. I find it surprising that it automatically tries to install qemu "behind my back".

QEMU is never installed. There is a local clone, but never an install. I never want random scripts to install things on my machine.

I don't think we have the same definition of "install". Sure, this isn't installed as root in a system folder like /usr/bin, but it's still some installation on my current folder with my user's privileges. In particular, the ./configure script could do anything out of my control.

To me, the following facts are problematic.

  • This qemu repository contains hundreds of MB. I consider this amount to be an installation. I don't consider it essential to regular Tock development, so there should be a prompt for me to confirm that I want to install it.
  • It doesn't come from an "official" source such as https://github.com/qemu/qemu. Don't take it personally (my reasoning would be the same for any https://github.com/username/qemu repository), but from a security perspective an entity like https://github.com/qemu is more to be trusted than https://github.com/username. Similarly, I trust an entity like https://github.com/tock to have some review policy before adding commits to master, whereas there's no such policy in https://github.com/username (username can add whatever commits they wish without oversight from anyone else).
  • The cloning process doesn't pin any commit vetted by Tock (cloning the current HEAD of the repository without verification, rather than using a git submodule pinned to a commit for example). Any code pushed on https://github.com/username/qemu is immediately picked up by Tock.
  • The installation runs ./configure on this unvetted (by Tock) HEAD of https://github.com/username/qemu. You never want random scripts to install things on my machine, but it's exactly what it is to my machine.

There's no form of cryptographic verification anywhere, so if username (or someone hacking username's account) pushes malicious code in username's fork, then it's arbitrary code execution on anyone running make ci.

All in all, I'd trust more some apt-get install qemu (because there is the support of the Debian organisation behind it) than a "non installation" cloning of https://github.com/username/qemu followed by arbitrary code execution.

Don't get me wrong, I definitely support your work in adding QEMU-based tests to Tock. But there needs to be safeguards on Tock's side. Otherwise, Tock fails at its mission of providing a secure OS.

The emulation-setup will instead clone and build a local QEMU isntance. Without it the emulation-check won't work so it depends on the setup. Also, that setup should only happen once and until you run a git clean it won't be rebuilt.

  • It anyway doesn't work out of the box without installing some other Debian packages.

I'm sorry you are missing some development packages, but there isn't really a good fix here. I don't think a Makefile should install packages so I don't want to install anything. We could document that you need to install build-essentials in order to build QEMU if that helps?

I don't think the Makefile should install these packages either (which would require root privileges anyway). But it should detect missing packages.

And it would be useful to document what packages are needed, for example starting from a minimal Docker image with buster-slim (minimal image on the latest Debian stable), so that we know if build-essentials is the only required dependency.

@alistair23
Copy link
Contributor

I agree that cloning from some strange person's git repo is not ideal.

Having the patches merged into mainline (no one has reviewed them) and git submodules would fix this. Git submodules are a bit of pain though, but would that be a better solution?

As for package detection that is really hard. There are lots of different distros that you have to support. QEMU's configure script does this for you already, I don't think we should re-implement that.

QEMU has documentation on what is required: https://wiki.qemu.org/Hosts/Linux#Building_QEMU_for_Linux we could link to that?

Eventually I would like to say "just install it from your package manager", but I think we are a long way from that. It looks like QEMU 5.1 will have what we need, but for some of the slower moving distros that will take years to update to 5.1+ in their packages for most users.

@gendx
Copy link
Contributor Author

gendx commented May 19, 2020

Git submodules are a bit of pain though, but would that be a better solution?

I've generally used submodules with good success. I'd be curious what pain points you have with them?

@alistair23
Copy link
Contributor

You are just lucky and haven't had to deal with keeping them updated and in sync. It quickly becomes a hassle and can be hard for new users to figure out.

When the patches are merged to mainline I'll convert it over to a submodule.

bors bot added a commit that referenced this issue May 19, 2020
1866: Small improvements to QEMU Make step r=ppannuto a=alistair23

### Pull Request Overview

This PR addresses concerns raised in: #1865

### Testing Strategy

Pushed to Git.

### TODO or Help Wanted

### Documentation Updated

- [X] Updated the relevant files in `/docs`, or no updates are required.

### Formatting

- [X] Ran `make format`.
- [X] Fixed errors surfaced by `make clippy`.


Co-authored-by: Alistair Francis <alistair.francis@wdc.com>
@ppannuto
Copy link
Member

Given the containment question and the non-zero environment to set up for qemu, would it possibly make sense to put the qemu parts into a docker container? That way we don't have to maintain separate install support for the various packages and such needed to build on every local machine.

From the 'just run it' perspective, I think it would be reasonable to (a) install and use the docker image if docker is installed, or (b) print a "skipping qemu checks, please install docker to run locally" message.

@alistair23
Copy link
Contributor

I don't feel that Docker is really any easier. It's just as easy to install Docker as it is build-essentials. To me build essentials is more likely to be installed already as well.

If others disagree I'm open to moving it into Docker.

@ppannuto
Copy link
Member

I think it's less about the ease of install as it is about the sandboxing.

Putting the qemu build into a docker container means all of the possibly untrusted bits (the repo from username, the giant configure script, etc) are contained. Then you can mount just the tock repo folder -- probably even read-only -- into the docker container to run all things qemu.

@alistair23
Copy link
Contributor

Fair, but once we swap over to mainline QEMU and submodules I don't see the issue as it's from a trusted source. It would be the same as apt installing it.

@gendx
Copy link
Contributor Author

gendx commented May 20, 2020

I think it's less about the ease of install as it is about the sandboxing.

Putting the qemu build into a docker container means all of the possibly untrusted bits (the repo from username, the giant configure script, etc) are contained. Then you can mount just the tock repo folder -- probably even read-only -- into the docker container to run all things qemu.

This was my point as well. I care much more about the security, trust & sandboxing of it than the installation process.

I also think that a prompt is necessary before cloning & installing the external qemu repository (whether it comes from https://github.com/username or https://github.com/qemu).

You are just lucky and haven't had to deal with keeping them updated and in sync. It quickly becomes a hassle and can be hard for new users to figure out.

Your concerns are still not very specific. There is some extensive documentation (https://git-scm.com/book/en/v2/Git-Tools-Submodules) and StackOverflow is full of questions (with answers!) related to git in general.
Provided that sufficient tooling is put in place (e.g. the Makefile rules check that the submodule is in a "clean" state), there should be no problem.

FWIW, OpenSK has been using Tock as a submodule and I haven't seen any complaints.

Fair, but once we swap over to mainline QEMU and submodules I don't see the issue as it's from a trusted source. It would be the same as apt installing it.

How far are we to be able to swap to mainline? https://github.com/alistair23/qemu is described as "a fork" although it's not forked from the GitHub UI, which makes it hard to even see a diff with the mainline. Are there pending pull requests (or the equivalent for whatever qemu's code review is) to add what we need?


I would also support putting it all in a Docker container by default, for the sake of sandboxing. But it could be installed directly on the current system provided some flag or environment variable is present (so that e.g. we don't need to make the Travis-CI workflow even slower by having to install Docker).

Would that be a good compromise?

@alistair23
Copy link
Contributor

There are patches on the QEMU mailing list, you can see the latest version here: https://patchew.org/QEMU/cover.1589923785.git.alistair.francis@wdc.com/

I have been pushing for reviews, you could help speed up the process by reviewing the code.

I can look at adding it to Docker, but I don't use Docker often so it might take a while to figure out.

@gendx gendx mentioned this issue May 29, 2020
2 tasks
@bradjc
Copy link
Contributor

bradjc commented Jun 3, 2020

Should be fixed with #1880 merged.

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

No branches or pull requests

4 participants