diff mbox series

[kvm-unit-tests,v1,2/2] s390x: create persistent comm-key

Message ID 20220721132647.552298-3-nrb@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: dump support for PV tests | expand

Commit Message

Nico Boehr July 21, 2022, 1:26 p.m. UTC
To decrypt the dump of a PV guest, the comm-key (CCK) is required. Until
now, no comm-key was provided to genprotimg, therefore decrypting the
dump of a kvm-unit-test under PV was not possible.

This patch makes sure that we create a random CCK if there's no
$(TEST_DIR)/comm.key file.

Also allow dumping of PV tests by passing the appropriate PCF to
genprotimg (bit 34). --x-pcf is used to be compatible with older
genprotimg versions, which don't support --enable-dump. 0xe0 is the
default PCF value and only bit 34 is added.

Unfortunately, recent versions of genprotimg removed the --x-comm-key
argument which was used by older versions to specify the CCK. To support
these versions, we need to parse the genprotimg help output and decide
which argument to use.

Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
---
 s390x/Makefile | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

Comments

Claudio Imbrenda July 21, 2022, 4:32 p.m. UTC | #1
On Thu, 21 Jul 2022 15:26:47 +0200
Nico Boehr <nrb@linux.ibm.com> wrote:

> To decrypt the dump of a PV guest, the comm-key (CCK) is required. Until
> now, no comm-key was provided to genprotimg, therefore decrypting the
> dump of a kvm-unit-test under PV was not possible.
> 
> This patch makes sure that we create a random CCK if there's no
> $(TEST_DIR)/comm.key file.
> 
> Also allow dumping of PV tests by passing the appropriate PCF to
> genprotimg (bit 34). --x-pcf is used to be compatible with older
> genprotimg versions, which don't support --enable-dump. 0xe0 is the
> default PCF value and only bit 34 is added.
> 
> Unfortunately, recent versions of genprotimg removed the --x-comm-key
> argument which was used by older versions to specify the CCK. To support
> these versions, we need to parse the genprotimg help output and decide
> which argument to use.

I wonder if we can simply support only the newest version?
would make the code cleaner, and updating genprotimg is not too
complicated

[...]
Nico Boehr July 22, 2022, 7:31 a.m. UTC | #2
Quoting Claudio Imbrenda (2022-07-21 18:32:45)
> On Thu, 21 Jul 2022 15:26:47 +0200
> Nico Boehr <nrb@linux.ibm.com> wrote:
> 
> > To decrypt the dump of a PV guest, the comm-key (CCK) is required. Until
> > now, no comm-key was provided to genprotimg, therefore decrypting the
> > dump of a kvm-unit-test under PV was not possible.
> > 
> > This patch makes sure that we create a random CCK if there's no
> > $(TEST_DIR)/comm.key file.
> > 
> > Also allow dumping of PV tests by passing the appropriate PCF to
> > genprotimg (bit 34). --x-pcf is used to be compatible with older
> > genprotimg versions, which don't support --enable-dump. 0xe0 is the
> > default PCF value and only bit 34 is added.
> > 
> > Unfortunately, recent versions of genprotimg removed the --x-comm-key
> > argument which was used by older versions to specify the CCK. To support
> > these versions, we need to parse the genprotimg help output and decide
> > which argument to use.
> 
> I wonder if we can simply support only the newest version?
> would make the code cleaner, and updating genprotimg is not too
> complicated

I would be annoyed by having to compile s390-tools every time I want to run PV tests on older distros.

If we want to avoid the --help parsing stuff, we could add a configure option to disable PV dump support. Not sure if it's a good idea.
Janosch Frank Aug. 24, 2022, 8:15 a.m. UTC | #3
On 7/21/22 15:26, Nico Boehr wrote:
> To decrypt the dump of a PV guest, the comm-key (CCK) is required. Until
> now, no comm-key was provided to genprotimg, therefore decrypting the
> dump of a kvm-unit-test under PV was not possible.
> 
> This patch makes sure that we create a random CCK if there's no
> $(TEST_DIR)/comm.key file.
> 
> Also allow dumping of PV tests by passing the appropriate PCF to
> genprotimg (bit 34). --x-pcf is used to be compatible with older
> genprotimg versions, which don't support --enable-dump. 0xe0 is the
> default PCF value and only bit 34 is added.
> 
> Unfortunately, recent versions of genprotimg removed the --x-comm-key
> argument which was used by older versions to specify the CCK. To support
> these versions, we need to parse the genprotimg help output and decide
> which argument to use.
> 
> Signed-off-by: Nico Boehr <nrb@linux.ibm.com>
> ---
>   s390x/Makefile | 21 +++++++++++++++++----
>   1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/s390x/Makefile b/s390x/Makefile
> index 34de233d09b8..5e3cb5a47bc2 100644
> --- a/s390x/Makefile
> +++ b/s390x/Makefile
> @@ -162,14 +162,27 @@ $(SNIPPET_DIR)/c/%.hdr: $(SNIPPET_DIR)/c/%.gbin $(HOST_KEY_DOCUMENT)
>   	$(RM) $(@:.elf=.aux.o)
>   	@chmod a-x $@
>   

Add comment along the lines of:
Secure Execution Customer Communication Key file
32 bytes of key material, uses existing one if available

> +comm-key = $(TEST_DIR)/comm.key
> +$(comm-key):
> +	dd if=/dev/urandom of=$@ bs=32 count=1 status=none
> +
>   %.bin: %.elf
>   	$(OBJCOPY) -O binary  $< $@
>   
> -genprotimg_args = --host-key-document $(HOST_KEY_DOCUMENT) --no-verify
> -%selftest.pv.bin: %selftest.bin $(HOST_KEY_DOCUMENT) $(patsubst %.pv.bin,%.parmfile,$@)
> +GENPROTIMG_HAS_COMM_KEY = $(shell $(GENPROTIMG) --help | grep -q -- --comm-key && echo yes)
> +ifeq ($(GENPROTIMG_HAS_COMM_KEY),yes)
> +	GENPROTIMG_COMM_KEY = --comm-key $(comm-key)
> +else
> +	GENPROTIMG_COMM_KEY = --x-comm-key $(comm-key)
> +endif
> +# use x-pcf to be compatible with old genprotimg versions
> +# allow dumping + PCKMO
> +genprotimg_pcf = 0x200000e0
> +genprotimg_args = --host-key-document $(HOST_KEY_DOCUMENT) --no-verify $(GENPROTIMG_COMM_KEY) --x-pcf $(genprotimg_pcf)

\n
We might need to fix that in patch #1.

> +%selftest.pv.bin: %selftest.bin $(HOST_KEY_DOCUMENT) $(patsubst %.pv.bin,%.parmfile,$@) $(comm-key)
>   	$(GENPROTIMG) $(genprotimg_args) --parmfile $(patsubst %.pv.bin,%.parmfile,$@) --image $< -o $@
>   
> -%.pv.bin: %.bin $(HOST_KEY_DOCUMENT)
> +%.pv.bin: %.bin $(HOST_KEY_DOCUMENT) $(comm-key)
>   	$(GENPROTIMG) $(genprotimg_args) --image $< -o $@
>   
>   $(snippet_asmlib): $$(patsubst %.o,%.S,$$@) $(asm-offsets)
> @@ -177,7 +190,7 @@ $(snippet_asmlib): $$(patsubst %.o,%.S,$$@) $(asm-offsets)
>   
>   
>   arch_clean: asm_offsets_clean
> -	$(RM) $(TEST_DIR)/*.{o,elf,bin} $(SNIPPET_DIR)/*/*.{o,elf,*bin,*obj,hdr} $(SNIPPET_DIR)/asm/.*.d $(TEST_DIR)/.*.d lib/s390x/.*.d
> +	$(RM) $(TEST_DIR)/*.{o,elf,bin} $(SNIPPET_DIR)/*/*.{o,elf,*bin,*obj,hdr} $(SNIPPET_DIR)/asm/.*.d $(TEST_DIR)/.*.d lib/s390x/.*.d $(comm-key)
>   
>   generated-files = $(asm-offsets)
>   $(tests:.elf=.o) $(asmlib) $(cflatobjs): $(generated-files)
diff mbox series

Patch

diff --git a/s390x/Makefile b/s390x/Makefile
index 34de233d09b8..5e3cb5a47bc2 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -162,14 +162,27 @@  $(SNIPPET_DIR)/c/%.hdr: $(SNIPPET_DIR)/c/%.gbin $(HOST_KEY_DOCUMENT)
 	$(RM) $(@:.elf=.aux.o)
 	@chmod a-x $@
 
+comm-key = $(TEST_DIR)/comm.key
+$(comm-key):
+	dd if=/dev/urandom of=$@ bs=32 count=1 status=none
+
 %.bin: %.elf
 	$(OBJCOPY) -O binary  $< $@
 
-genprotimg_args = --host-key-document $(HOST_KEY_DOCUMENT) --no-verify
-%selftest.pv.bin: %selftest.bin $(HOST_KEY_DOCUMENT) $(patsubst %.pv.bin,%.parmfile,$@)
+GENPROTIMG_HAS_COMM_KEY = $(shell $(GENPROTIMG) --help | grep -q -- --comm-key && echo yes)
+ifeq ($(GENPROTIMG_HAS_COMM_KEY),yes)
+	GENPROTIMG_COMM_KEY = --comm-key $(comm-key)
+else
+	GENPROTIMG_COMM_KEY = --x-comm-key $(comm-key)
+endif
+# use x-pcf to be compatible with old genprotimg versions
+# allow dumping + PCKMO
+genprotimg_pcf = 0x200000e0
+genprotimg_args = --host-key-document $(HOST_KEY_DOCUMENT) --no-verify $(GENPROTIMG_COMM_KEY) --x-pcf $(genprotimg_pcf)
+%selftest.pv.bin: %selftest.bin $(HOST_KEY_DOCUMENT) $(patsubst %.pv.bin,%.parmfile,$@) $(comm-key)
 	$(GENPROTIMG) $(genprotimg_args) --parmfile $(patsubst %.pv.bin,%.parmfile,$@) --image $< -o $@
 
-%.pv.bin: %.bin $(HOST_KEY_DOCUMENT)
+%.pv.bin: %.bin $(HOST_KEY_DOCUMENT) $(comm-key)
 	$(GENPROTIMG) $(genprotimg_args) --image $< -o $@
 
 $(snippet_asmlib): $$(patsubst %.o,%.S,$$@) $(asm-offsets)
@@ -177,7 +190,7 @@  $(snippet_asmlib): $$(patsubst %.o,%.S,$$@) $(asm-offsets)
 
 
 arch_clean: asm_offsets_clean
-	$(RM) $(TEST_DIR)/*.{o,elf,bin} $(SNIPPET_DIR)/*/*.{o,elf,*bin,*obj,hdr} $(SNIPPET_DIR)/asm/.*.d $(TEST_DIR)/.*.d lib/s390x/.*.d
+	$(RM) $(TEST_DIR)/*.{o,elf,bin} $(SNIPPET_DIR)/*/*.{o,elf,*bin,*obj,hdr} $(SNIPPET_DIR)/asm/.*.d $(TEST_DIR)/.*.d lib/s390x/.*.d $(comm-key)
 
 generated-files = $(asm-offsets)
 $(tests:.elf=.o) $(asmlib) $(cflatobjs): $(generated-files)