[PATCH v2 1/2] selftests/x86: sgxsign: Do not query RSA password

[PATCH v2 1/2] selftests/x86: sgxsign: Do not query RSA password

[Jarkko Sakkinen]

Pass NULL as the value for @cb in PEM_read_RSAPrivateKey() and remove
pem_password_cb().  According to the man page [1], when both @cb and @u
are NULL, a default callback provided by OpenSSL will be used to query
the password. Since our key is not sealed, this is dead functionality.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 tools/testing/selftests/x86/sgx/sgxsign.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/tools/testing/selftests/x86/sgx/sgxsign.c b/tools/testing/selftests/x86/sgx/sgxsign.c
index 0b89823fc703..3d9007af40c9 100644
--- a/tools/testing/selftests/x86/sgx/sgxsign.c
+++ b/tools/testing/selftests/x86/sgx/sgxsign.c
@@ -20,8 +20,6 @@ struct sgx_sigstruct_payload {
 	struct sgx_sigstruct_body body;
 };
 
-static const char *sign_key_pass;
-
 static bool check_crypto_errors(void)
 {
 	int err;
@@ -50,18 +48,6 @@ static void exit_usage(const char *program)
 	exit(1);
 }
 
-static int pem_passwd_cb(char *buf, int size, int rwflag, void *u)
-{
-	if (!sign_key_pass)
-		return -1;
-
-	strncpy(buf, sign_key_pass, size);
-	/* no retry */
-	sign_key_pass = NULL;
-
-	return strlen(buf) >= size ? size - 1 : strlen(buf);
-}
-
 static inline const BIGNUM *get_modulus(RSA *key)
 {
 #if OPENSSL_VERSION_NUMBER < 0x10100000L
@@ -85,7 +71,7 @@ static RSA *load_sign_key(const char *path)
 		return NULL;
 	}
 	key = RSA_new();
-	if (!PEM_read_RSAPrivateKey(f, &key, pem_passwd_cb, NULL))
+	if (!PEM_read_RSAPrivateKey(f, &key, NULL, NULL))
 		return NULL;
 	fclose(f);
 
@@ -455,7 +441,6 @@ int main(int argc, char **argv)
 #endif
 	ss.body.xfrm = 3,
 
-	sign_key_pass = getenv("KBUILD_SGX_SIGN_PIN");
 	program = argv[0];
 
 	do {
-- 
2.20.1

[PATCH v2 2/2] selftests/x86/sgx: Read encl.bin and encl.ss from the file system

[Jarkko Sakkinen]

Do not link encl.bin and encl.ss to the test application binary. Linking
data files directly to the ELF are legacy from in-kernel LE
implementation.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
Add encl.ss as target so that it gets packaged
 tools/testing/selftests/x86/sgx/Makefile     |  12 +--
 tools/testing/selftests/x86/sgx/encl_piggy.S |  19 ----
 tools/testing/selftests/x86/sgx/main.c       | 101 +++++++++++++++----
 3 files changed, 88 insertions(+), 44 deletions(-)
 delete mode 100644 tools/testing/selftests/x86/sgx/encl_piggy.S

diff --git a/tools/testing/selftests/x86/sgx/Makefile b/tools/testing/selftests/x86/sgx/Makefile
index 4310a5b6ecc7..a09ef5f965dc 100644
--- a/tools/testing/selftests/x86/sgx/Makefile
+++ b/tools/testing/selftests/x86/sgx/Makefile
@@ -10,11 +10,11 @@ HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC -z noexecstack
 ENCL_CFLAGS := -Wall -Werror -static -nostdlib -nostartfiles -fPIC \
 	       -fno-stack-protector -mrdrnd $(INCLUDES)
 
-TEST_CUSTOM_PROGS := $(OUTPUT)/test_sgx
+TEST_CUSTOM_PROGS := $(OUTPUT)/test_sgx $(OUTPUT)/encl.bin $(OUTPUT)/encl.ss
+
 all: $(TEST_CUSTOM_PROGS)
 
-$(TEST_CUSTOM_PROGS): $(OUTPUT)/main.o $(OUTPUT)/sgx_call.o \
-		      $(OUTPUT)/encl_piggy.o
+$(OUTPUT)/test_sgx: $(OUTPUT)/main.o $(OUTPUT)/sgx_call.o
 	$(CC) $(HOST_CFLAGS) -o $@ $^
 
 $(OUTPUT)/main.o: main.c
@@ -23,16 +23,13 @@ $(OUTPUT)/main.o: main.c
 $(OUTPUT)/sgx_call.o: sgx_call.S
 	$(CC) $(HOST_CFLAGS) -c $< -o $@
 
-$(OUTPUT)/encl_piggy.o: $(OUTPUT)/encl.bin $(OUTPUT)/encl.ss
-	$(CC) $(HOST_CFLAGS) -I$(OUTPUT) -c encl_piggy.S -o $@
-
 $(OUTPUT)/encl.bin: $(OUTPUT)/encl.elf $(OUTPUT)/sgxsign
 	$(OBJCOPY) -O binary $< $@
 
 $(OUTPUT)/encl.elf: encl.lds encl.c encl_bootstrap.S
 	$(CC) $(ENCL_CFLAGS) -T $^ -o $@
 
-$(OUTPUT)/encl.ss: $(OUTPUT)/encl.bin  $(OUTPUT)/sgxsign
+$(OUTPUT)/encl.ss: $(OUTPUT)/encl.bin
 	$(OUTPUT)/sgxsign signing_key.pem $(OUTPUT)/encl.bin $(OUTPUT)/encl.ss
 
 $(OUTPUT)/sgxsign: sgxsign.c
@@ -40,7 +37,6 @@ $(OUTPUT)/sgxsign: sgxsign.c
 
 EXTRA_CLEAN := \
 	$(OUTPUT)/encl.bin \
-	$(OUTPUT)/encl_piggy.o \
 	$(OUTPUT)/encl.elf \
 	$(OUTPUT)/encl.ss \
 	$(OUTPUT)/sgx_call.o \
diff --git a/tools/testing/selftests/x86/sgx/encl_piggy.S b/tools/testing/selftests/x86/sgx/encl_piggy.S
deleted file mode 100644
index a7f6447abbba..000000000000
--- a/tools/testing/selftests/x86/sgx/encl_piggy.S
+++ /dev/null
@@ -1,19 +0,0 @@
-/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
-/*
- * Copyright(c) 2016-18 Intel Corporation.
- */
-
-	.section ".rodata", "a"
-	.balign 4096
-
-encl_bin:
-	.globl encl_bin
-	.incbin	"encl.bin"
-encl_bin_end:
-	.globl encl_bin_end
-
-encl_ss:
-	.globl encl_ss
-	.incbin	"encl.ss"
-encl_ss_end:
-	.globl encl_ss_end
diff --git a/tools/testing/selftests/x86/sgx/main.c b/tools/testing/selftests/x86/sgx/main.c
index 68a22ef3f05c..2160bcd0ccd9 100644
--- a/tools/testing/selftests/x86/sgx/main.c
+++ b/tools/testing/selftests/x86/sgx/main.c
@@ -14,6 +14,7 @@
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/time.h>
+#include <sys/types.h>
 #include "encl_piggy.h"
 #include "defines.h"
 #include "../../../../../arch/x86/kernel/cpu/sgx/arch.h"
@@ -189,7 +190,8 @@ static bool encl_add_page(int dev_fd, unsigned long addr, void *data,
 	return true;
 }
 
-static bool encl_load(struct sgx_secs *secs, unsigned long bin_size)
+static bool encl_build(struct sgx_secs *secs, void *bin,
+		       unsigned long bin_size, struct sgx_sigstruct *sigstruct)
 {
 	struct sgx_enclave_init ioc;
 	uint64_t offset;
@@ -215,11 +217,11 @@ static bool encl_load(struct sgx_secs *secs, unsigned long bin_size)
 				SGX_SECINFO_W | SGX_SECINFO_X;
 
 		if (!encl_add_page(dev_fd, secs->base + offset,
-				   encl_bin + offset, flags))
+				   bin + offset, flags))
 			goto out_map;
 	}
 
-	ioc.sigstruct = (uint64_t)&encl_ss;
+	ioc.sigstruct = (uint64_t)sigstruct;
 	rc = ioctl(dev_fd, SGX_IOC_ENCLAVE_INIT, &ioc);
 	if (rc) {
 		printf("EINIT failed rc=%d\n", rc);
@@ -241,7 +243,6 @@ static bool encl_load(struct sgx_secs *secs, unsigned long bin_size)
 		return false;
 	}
 
-
 	close(dev_fd);
 	return true;
 out_map:
@@ -251,20 +252,95 @@ static bool encl_load(struct sgx_secs *secs, unsigned long bin_size)
 	return false;
 }
 
+bool get_file_size(const char *path, off_t *bin_size)
+{
+	struct stat sb;
+	int ret;
+
+	ret = stat(path, &sb);
+	if (ret) {
+		perror("stat");
+		return false;
+	}
+
+	if (!sb.st_size || sb.st_size & 0xfff) {
+		fprintf(stderr, "Invalid blob size %lu\n", sb.st_size);
+		return false;
+	}
+
+	*bin_size = sb.st_size;
+	return true;
+}
+
+bool encl_data_map(const char *path, void **bin, off_t *bin_size)
+{
+	int fd;
+
+	fd = open(path, O_RDONLY);
+	if (fd == -1)  {
+		fprintf(stderr, "open() %s failed, errno=%d.\n", path, errno);
+		return false;
+	}
+
+	if (!get_file_size(path, bin_size))
+		goto err_out;
+
+	*bin = mmap(NULL, *bin_size, PROT_READ, MAP_PRIVATE, fd, 0);
+	if (*bin == MAP_FAILED) {
+		fprintf(stderr, "mmap() %s failed, errno=%d.\n", path, errno);
+		goto err_out;
+	}
+
+	close(fd);
+	return true;
+
+err_out:
+	close(fd);
+	return false;
+}
+
+bool load_sigstruct(const char *path, void *sigstruct)
+{
+	int fd;
+
+	fd = open(path, O_RDONLY);
+	if (fd == -1)  {
+		fprintf(stderr, "open() %s failed, errno=%d.\n", path, errno);
+		return false;
+	}
+
+	if (read(fd, sigstruct, sizeof(struct sgx_sigstruct)) !=
+	    sizeof(struct sgx_sigstruct)) {
+		fprintf(stderr, "read() %s failed, errno=%d.\n", path, errno);
+		close(fd);
+		return false;
+	}
+
+	close(fd);
+	return true;
+}
+
 int sgx_call(void *rdi, void *rsi, long rdx, void *rcx, void *r8, void *r9,
 	     void *tcs, struct sgx_enclave_exception *ei, void *cb);
 
 int main(int argc, char *argv[], char *envp[])
 {
-	unsigned long bin_size = encl_bin_end - encl_bin;
-	unsigned long ss_size = encl_ss_end - encl_ss;
 	struct sgx_enclave_exception exception;
-	Elf64_Sym *eenter_sym;
+	struct sgx_sigstruct sigstruct;
 	struct vdso_symtab symtab;
+	Elf64_Sym *eenter_sym;
 	struct sgx_secs secs;
 	uint64_t result = 0;
+	off_t bin_size;
+	void *bin;
 	void *addr;
 
+	if (!encl_data_map("encl.bin", &bin, &bin_size))
+		exit(1);
+
+	if (!load_sigstruct("encl.ss", &sigstruct))
+		exit(1);
+
 	memset(&exception, 0, sizeof(exception));
 
 	addr = vdso_get_base_addr(envp);
@@ -279,16 +355,7 @@ int main(int argc, char *argv[], char *envp[])
 		exit(1);
 	eenter = addr + eenter_sym->st_value;
 
-	printf("Binary size %lu (0x%lx), SIGSTRUCT size %lu\n", bin_size,
-	       bin_size, ss_size);
-	if (ss_size != sizeof(struct sgx_sigstruct)) {
-		fprintf(stderr, "The size of SIGSTRUCT should be %lu\n",
-			sizeof(struct sgx_sigstruct));
-		exit(1);
-	}
-
-	printf("Loading the enclave.\n");
-	if (!encl_load(&secs, bin_size))
+	if (!encl_build(&secs, bin, bin_size, &sigstruct))
 		exit(1);
 
 	printf("Input: 0x%lx\n", MAGIC);
-- 
2.20.1

Re: [PATCH v2 2/2] selftests/x86/sgx: Read encl.bin and encl.ss from the file system

[Jarkko Sakkinen]

On Fri, Aug 30, 2019 at 01:15:19AM +0300, Jarkko Sakkinen wrote:
> Do not link encl.bin and encl.ss to the test application binary. Linking
> data files directly to the ELF are legacy from in-kernel LE
> implementation.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

I squashed both.

After Sean's changes have been integrated I'll send v22 (hopefully
tomorrow).

/Jarkko