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

s390x: generate GPG keys for Ignition config protection #3055

Merged
merged 3 commits into from
Feb 16, 2023

Conversation

nikita-dubrovskii
Copy link
Contributor

@nikita-dubrovskii nikita-dubrovskii commented Aug 30, 2022

This is a proof-of-concept, part of coreos/fedora-coreos-config#1939.

During cosa buildextend-secex a pair of GPG keys is randomly generated,
where private key becomes part of sdboot image, and public key becomes
part of build artifacts.

User than can encrypt his Ignition config:

gpg --recipient-file /path/to/ignition.gpg.pub --output /path/to/config.ign.gpg --armor --encrypt /path/to/config.ign

And attach it to qemu-kvm as a disk:

-drive if=none,id=ignition,format=raw,file=/path/to/config.ign.gpg,readonly=on \
-device virtio-blk,serial=ignition.gpg,iommu_platform=on,drive=ignition

@nikita-dubrovskii nikita-dubrovskii force-pushed the ignition_protection branch 2 times, most recently from 0abed72 to e365bf0 Compare August 30, 2022 14:28
@nikita-dubrovskii nikita-dubrovskii changed the title WIP: s390x: generate RSA keys for ignition protection WIP: s390x: generate GPG keys for Ignition config protection Sep 27, 2022
@nikita-dubrovskii nikita-dubrovskii changed the title WIP: s390x: generate GPG keys for Ignition config protection s390x: generate GPG keys for Ignition config protection Sep 29, 2022
@nikita-dubrovskii nikita-dubrovskii force-pushed the ignition_protection branch 2 times, most recently from 84245e0 to 4addcbc Compare October 27, 2022 12:33
src/create_disk.sh Outdated Show resolved Hide resolved
src/create_disk.sh Outdated Show resolved Hide resolved
src/create_disk.sh Show resolved Hide resolved
src/create_disk.sh Show resolved Hide resolved
src/cmd-generate-release-meta Outdated Show resolved Hide resolved
src/cmd-generate-release-meta Outdated Show resolved Hide resolved
pkg/builds/schema_doc.go Outdated Show resolved Hide resolved
src/cmd-generate-release-meta Outdated Show resolved Hide resolved
src/create_disk.sh Show resolved Hide resolved
@nikita-dubrovskii nikita-dubrovskii force-pushed the ignition_protection branch 2 times, most recently from 338d019 to 7899463 Compare February 14, 2023 16:12
src/create_disk.sh Outdated Show resolved Hide resolved
src/create_disk.sh Outdated Show resolved Hide resolved
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Sorry, typed Enter too fast, will retry! :)

jlebon
jlebon previously approved these changes Feb 14, 2023
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

LGTM! Two thoughts:

  1. I wish we didn't have to support both the external genprotimg flow and the "local dev" one. Maybe we can get rid of the latter and add instructions on how to create your own genprotimg qcow2 for local testing. Though that's probably actually more complex to support than this. Let me know if you have ideas around this.
  2. How are we planning to test this? If this isn't tested by CI, we could easily regress on it. Ideally we'd have something like kola run -p qemu-secex, but maybe easier is to keep relying on the qemu platform code, but add a --qemu-secex flag which tells kola to encrypt the Ignition config and present it as the expected virtio device? I guess the hurdle there is that we need to inject the host key in the Ignition config, which depends on whatever the builder we're testing on. We could define some APIs for that to wire it through, though short-term, we could just inject a bogus key; as long as we run tests that don't require regenerating the initramfs, it should work. (Honestly, even just running basic to start would be really helpful since it still exercises the Ignition config path.)

@nikita-dubrovskii
Copy link
Contributor Author

1. I wish we didn't have to support both the external genprotimg flow and the "local dev" one. Maybe we can get rid of the latter and add instructions on how to create your own genprotimg qcow2 for local testing. Though that's probably actually more complex to support than this. Let me know if you have ideas around this.

I was thinking about it, but removing external VM seems more attractive (rdcore is awesome). I'd say we can wait til 4.13/4.14 and see how it goes with SE+OCP, maybe some build policies would change (i hope they would). If not - than i'll create test-vm/docker container to imitate genprotimg-vm.

2. How are we planning to test this? If this isn't tested by CI, we could easily regress on it. 

That's smth to discuss with IBM team. As far as i remember, we were discussing how-to add CI/CD on our side. Will open this question again

P.S.: thank you for the help and review )

During `cosa buildextend-secex` a pair of GPG keys is randomly generated,
where private key becomes part of `sdboot` image, and public key becomes
part of build artifacts.

User than can encrypt his Ignition config:
```
gpg --recipient-file /path/to/ignition.gpg.pub --output /path/to/config.ign.gpg --armor --encrypt /path/to/config.ign
```

And attach it to `qemu-kvm` as a disk:
```
-drive if=none,id=ignition,format=raw,file=/path/to/config.ign.gpg,readonly=on \
-device virtio-blk,serial=ignition.gpg,iommu_platform=on,drive=ignition
```
Signed-off-by: Nikita Dubrovskii <nikita@linux.ibm.com>
Signed-off-by: Nikita Dubrovskii <nikita@linux.ibm.com>
@jschintag
Copy link
Contributor

I was thinking about it, but removing external VM seems more attractive (rdcore is awesome).

I agree that doing it via rdcore would be awesome. But i don't think the restrictions on the universal hostkey are going to be lifted, so the genprotimgvm is not going away...

@jschintag
Copy link
Contributor

LGTM

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

To be clear, I don't think we should consider this feature completed until we've fleshed out the testing story. But I think this is in a good state now to get in and we can iterate from there.

Nice work!

@jlebon jlebon merged commit f35e2c8 into coreos:main Feb 16, 2023
@jlebon
Copy link
Member

jlebon commented Feb 16, 2023

Opened a follow-up in #3362.

@nikita-dubrovskii nikita-dubrovskii deleted the ignition_protection branch February 17, 2023 07:51
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.

None yet

4 participants