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

Update eif_build command line experience #27

Merged
merged 8 commits into from
Jul 23, 2024
Merged

Update eif_build command line experience #27

merged 8 commits into from
Jul 23, 2024

Conversation

agraf
Copy link
Contributor

@agraf agraf commented Jul 9, 2024

The eif_build command line utility was pretty dormant: No support for aarch64, no simple support to override metadata such as build time, no mention of the tool in the README. This PR extends the eif_build tool to become slightly more prominent and useful:

  • Aarch64 support (Thanks to Monzo!)
  • Remove old SHA support
  • Improve the result output (JSON prettified now!)
  • New command line options to override build metadata (build_time, build_tool, build_tool_version, img_os, img_kernel)
  • Make kernel_config parameter optional
  • Move eif_build from examples into its own build target
  • Add README

Monzo extended the eif_build tool with an arch option. This commit ports
the respective code over to the official repository.

Original: monzo/aws-nitro-util@369cc7e

Signed-off-by: Alexander Graf <[email protected]>
@agraf
Copy link
Contributor Author

agraf commented Jul 9, 2024

CC @Conan-Kudo 😄

@Conan-Kudo
Copy link

Does this now get built with cargo?

@Conan-Kudo
Copy link

Why is this an example if this is a reference tool to use?

@agraf
Copy link
Contributor Author

agraf commented Jul 10, 2024

Does this now get built with cargo?

Yes, if you build with cargo build --all or cargo build --workspace or cargo build -p eif_build, you get the eif_build tool as output.

Why is this an example if this is a reference tool to use?

I agree that it's awkward. I updated the Cargo.toml file to make it a separate stand alone target.

Copy link

@foersleo foersleo left a comment

Choose a reason for hiding this comment

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

Great improvements! One little comment on the arch option that I would like to hear your thoughts on.

examples/eif_build.rs Outdated Show resolved Hide resolved
examples/eif_build.rs Show resolved Hide resolved
examples/eif_build.rs Outdated Show resolved Hide resolved
examples/eif_build.rs Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
eif_build/src/main.rs Show resolved Hide resolved
eif_build/src/main.rs Outdated Show resolved Hide resolved
eif_build/src/main.rs Outdated Show resolved Hide resolved
eif_build/src/main.rs Outdated Show resolved Hide resolved
eif_build/src/main.rs Outdated Show resolved Hide resolved
agraf added 7 commits July 19, 2024 09:15
The ei_build command line considered the kernel config file as
mandatory. It really isn't. Let's make it optional.

Also, let's provide a way for the user to override any of the fields of
the build metadata using command line options.

And while at it, make the arch option optional.

Signed-off-by: Alexander Graf <[email protected]>
The EIF file format only supports SHA386. Let's not give the user even
an idea that they could use any other hash length. Remove all command
line options that allow setting of sha hash sizes.

Signed-off-by: Alexander Graf <[email protected]>
The build output is difficult to read in a single line. Let's use
JSON pretty print logic to make it easier to parse.

Signed-off-by: Alexander Graf <[email protected]>
Some of the formatting of previous changes was off compared to what
rustfmt expects. Run cargo fmt on the code to resolve it.

Signed-off-by: Alexander Graf <[email protected]>
The original arch implementation allows any value as argument. Limit the
argument space to only either "aarch64" or "x86_64".

Signed-off-by: Alexander Graf <[email protected]>
We haven't documented the eif_build tool properly so far. Let's add some
hints to it in the documentation, so people know it's an official tool.

Signed-off-by: Alexander Graf <[email protected]>
Now that we've extended the eif_build tool to be very useful outside of
example use cases, let's make it its own stand alone target. You can
easily compile it with

  $ cargo build -p eif_build

Signed-off-by: Alexander Graf <[email protected]>
@agraf agraf merged commit d2025e4 into aws:main Jul 23, 2024
5 checks passed
cottand added a commit to monzo/aws-nitro-util that referenced this pull request Aug 29, 2024
…wn CLI fork (#24)

Thanks to aws/aws-nitro-enclaves-image-format#27
, we can now build EIFs via a low-level CLI that uses
`aws-nitro-enclave-image-format` directly, rather than maintaining and
developing our own CLI (which was forked from an old example in that
repo anyway).

For us, this means we can remove all the Rust code in this repository
and defer to AWS.


So this PR:
- packages AWS' `eif_build` with Nix to use as build dependency
- replaces `eif-cli` with `eif_build`, removing `eif-cli` altogether
- fixes outdated code comment and flake description
- adds a Cargo.lock to lock upstream's dependencies. This is needed to
reproducibly build `eif_build`. We can remove it in the future if
aws/aws-nitro-enclaves-image-format#34 is
closed.

**Note EIFs built after this PR are not bit-by-bit identical to the ones
built before this PR** (so they will not have the same SHA256 digests).
This is because the EIF metadata has been updated too.

PCRs, on the other hand, can be expected to remain the same (because
metadata is not included in PCR0 hashing). We assert this via our [flake
check](https://github.com/monzo/aws-nitro-util/actions/runs/10558127694/job/29246984165?pr=24)
, which builds a test EIF against a hardcoded PCR0.

As far as downstream users are concerned, this PR has no other breaking
changes.
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.

6 participants