[PATCH 0/6] Add Hygon SEV support

[PATCH 0/6] Add Hygon SEV support

[Hao Feng]

Hygon SEV follows AMD SEV work flow, but uses China national standard
cryptographic algorithms SM2/SM3/SM4 instead of (RSA, ECDSA,
ECDH)/SHA/AES. Reuse most AMD SEV code path to support Hygon SEV,
also adds 3 new commands(GM_PUBKEY_GEN, GM_GET_DIGEST,
GM_VERIFY_DIGEST) to support SM2 key exchange protocol.

SM2 is based on ECC(Elliptic Curve Cryptography), and uses a special
curve. It can be used in digital signature, key exchange and
asymmetric cryptography. For key exchange, SM2 is similar to ECDH, but
involves new random key, meaning the two sides need to exchange extra
random public key besides their public key, that's why additional APIs
are needed to support Hygon SEV.
SM3 is a hash algorithm, similar to SHA-256.
SM4 is a block cipher algorithm, similar to AES-128.

1. GM_PUBKEY_GEN
----------------
The command is used to get SM2 random public key from SEV firmware to
compute share key.

Parameters:
* GM_KEY_ID_PADDR (in) - Address of key ID for the random public key.
* GM_KEY_ID_LEN (in) - Length of key ID.
* GM_PUBKEY_PADDR (in) - Address of the random public key.
* GM_PUBKEY_LEN (in,out) - Length of the random public key.

2. GM_GET_DIGEST
----------------
The command is used to get key digest from SEV firmware during SM2 key
exchange, guest owner can check the digest to see if the key
negotiation is successful or not.

Parameters:
* HANDLE (in) - Guest handle
* DIGEST_PADDR (in) - Address of the key digest
* DIGEST_LEN (in, out) - Length of the key digest

3. GM_VERIFY_DIGEST
-------------------
The command is used to send guest owner's key digest to SEV firmware
during SM2 key exchange, firmware can check the digest to see if the
negotiation is successful or not.

Parameters:
* HANDLE (in) - Guest handle
* DIGEST_PADDR (in) - Address of the key digest
* DIGEST_LEN (in) - Length of the key digest

Already tested successfully on Hygon DhyanaPlus processor, also tested
successfully on AMD EPYC processor, results show no side effect on
current AMD SEV implementation.

Hao Feng (6):
  crypto: ccp: Add Hygon Dhyana support
  crypto: ccp: Define Hygon SEV commands
  crypto: ccp: Implement SEV_GM_PUBKEY_GEN ioctl command
  KVM: Define Hygon SEV commands
  KVM: SVM: Add support for KVM_SEV_GM_GET_DIGEST command
  KVM: SVM: Add KVM_SEV_GM_VERIFY_DIGEST command

 arch/x86/kvm/svm.c           | 119 +++++++++++++++++++++++++++++++++++++++++++
 drivers/crypto/ccp/psp-dev.c |  86 +++++++++++++++++++++++++++++++
 drivers/crypto/ccp/sp-pci.c  |   2 +
 include/linux/psp-sev.h      |  49 ++++++++++++++++++
 include/uapi/linux/kvm.h     |  14 +++++
 include/uapi/linux/psp-sev.h |  17 +++++++
 6 files changed, 287 insertions(+)

-- 
2.7.4

[PATCH 1/6] crypto: ccp: Add Hygon Dhyana support

[Hao Feng]

The Hygon Dhyana CPU has 2 CCP devices, the device IDs are 0x1456 and
0x1468, Hygon uses the same device interface as AMD, so enable the CCP
driver to support Hygon CCP devices.

Signed-off-by: Hao Feng <fenghao@hygon.cn>
---
 drivers/crypto/ccp/sp-pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/crypto/ccp/sp-pci.c b/drivers/crypto/ccp/sp-pci.c
index 41bce0a..363ad9f 100644
--- a/drivers/crypto/ccp/sp-pci.c
+++ b/drivers/crypto/ccp/sp-pci.c
@@ -321,6 +321,8 @@ static const struct pci_device_id sp_pci_table[] = {
 	{ PCI_VDEVICE(AMD, 0x1456), (kernel_ulong_t)&dev_vdata[1] },
 	{ PCI_VDEVICE(AMD, 0x1468), (kernel_ulong_t)&dev_vdata[2] },
 	{ PCI_VDEVICE(AMD, 0x1486), (kernel_ulong_t)&dev_vdata[3] },
+	{ PCI_VDEVICE(HYGON, 0x1456), (kernel_ulong_t)&dev_vdata[1] },
+	{ PCI_VDEVICE(HYGON, 0x1468), (kernel_ulong_t)&dev_vdata[2] },
 	/* Last entry must be zero */
 	{ 0, }
 };
-- 
2.7.4

[PATCH 2/6] crypto: ccp: Define Hygon SEV commands

[Hao Feng]

1. SEV_CMD_GM_PUBKEY_GEN - Get SM2 random public key from SEV firmware
to start SM2 key exchange.

2. SEV_CMD_GM_GET_DIGEST - Get key digest from SEV firmware during SM2
key exchange.

3. SEV_CMD_GM_VERIFY_DIGEST - Verify guest owner's key digest during
SM2 key exchange.

Signed-off-by: Hao Feng <fenghao@hygon.cn>
---
 drivers/crypto/ccp/psp-dev.c |  3 +++
 include/linux/psp-sev.h      | 49 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index fadf859..fafebf4 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -139,6 +139,9 @@ static int sev_cmd_buffer_len(int cmd)
 	case SEV_CMD_LAUNCH_UPDATE_SECRET:	return sizeof(struct sev_data_launch_secret);
 	case SEV_CMD_DOWNLOAD_FIRMWARE:		return sizeof(struct sev_data_download_firmware);
 	case SEV_CMD_GET_ID:			return sizeof(struct sev_data_get_id);
+	case SEV_CMD_GM_PUBKEY_GEN:		return sizeof(struct sev_data_gm_pubkey_gen);
+	case SEV_CMD_GM_GET_DIGEST:		return sizeof(struct sev_data_gm_get_digest);
+	case SEV_CMD_GM_VERIFY_DIGEST:		return sizeof(struct sev_data_gm_verify_digest);
 	default:				return 0;
 	}
 
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index 827c601..0171849 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -87,6 +87,11 @@ enum sev_cmd {
 	SEV_CMD_DBG_DECRYPT		= 0x060,
 	SEV_CMD_DBG_ENCRYPT		= 0x061,
 
+	/* GM specific commands */
+	SEV_CMD_GM_PUBKEY_GEN		= 0x070,
+	SEV_CMD_GM_GET_DIGEST		= 0x071,
+	SEV_CMD_GM_VERIFY_DIGEST	= 0x072,
+
 	SEV_CMD_MAX,
 };
 
@@ -485,6 +490,50 @@ struct sev_data_dbg {
 	u32 len;				/* In */
 } __packed;
 
+/**
+ * struct sev_data_gm_pubkey_gen - GM_PUBKEY_GEN command parameters
+ *
+ * @key_id_address: physical address containing key id
+ * @key_id_len: len of key id
+ * @pubkey_address: physical address containing GM public key
+ * @pubkey_len: len of GM public key
+ */
+struct sev_data_gm_pubkey_gen {
+	u64 key_id_address;		/* In */
+	u32 key_id_len;			/* In */
+	u32 reserved;
+	u64 pubkey_address;		/* In */
+	u32 pubkey_len;			/* In/Out */
+} __packed;
+
+/**
+ * struct sev_data_gm_get_digest - GM_GET_DIGEST command parameters
+ *
+ * @handle: handle of the VM to process
+ * @address: physical address containing the digest blob
+ * @len: len of digest blob
+ */
+struct sev_data_gm_get_digest {
+	u32 handle;				/* In */
+	u32 reserved;
+	u64 address;			/* In */
+	u32 len;				/* In/Out */
+} __packed;
+
+/**
+ * struct sev_data_gm_verify_digest - GM_VERIFY_DIGEST command parameters
+ *
+ * @handle: handle of the VM to verify
+ * @address: physical address containing the digest blob
+ * @len: len of digest blob
+ */
+struct sev_data_gm_verify_digest {
+	u32 handle;		/* In */
+	u32 reserved;
+	u64 address;	/* In */
+	u32 len;		/* In */
+};
+
 #ifdef CONFIG_CRYPTO_DEV_SP_PSP
 
 /**
-- 
2.7.4

[PATCH 3/6] crypto: ccp: Implement SEV_GM_PUBKEY_GEN ioctl command

[Hao Feng]

The SEV_GM_PUBKEY_GEN command is used to get SM2 random public key
from SEV firmware to start SM2 key exchange, guest owner will use the
random public key to compute share key.

Signed-off-by: Hao Feng <fenghao@hygon.cn>
---
 drivers/crypto/ccp/psp-dev.c | 83 ++++++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/psp-sev.h | 17 +++++++++
 2 files changed, 100 insertions(+)

diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index fafebf4..c165847c 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -720,6 +720,86 @@ static int sev_ioctl_do_pdh_export(struct sev_issue_cmd *argp)
 	return ret;
 }
 
+static int sev_ioctl_do_gm_pubkey_gen(struct sev_issue_cmd *argp)
+{
+	struct sev_user_data_gm_pubkey_gen input;
+	void *key_id_blob = NULL, *pubkey_blob = NULL;
+	struct sev_data_gm_pubkey_gen *data;
+	int ret;
+
+	if (copy_from_user(&input, (void __user *)argp->data, sizeof(input)))
+		return -EFAULT;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	/* Userspace wants to query the public key length. */
+	if (!input.pubkey_address ||
+	    !input.pubkey_len)
+		goto cmd;
+
+	/* Copy key id blob from userspace. */
+	key_id_blob = psp_copy_user_blob(input.key_id_address, input.key_id_len);
+	if (IS_ERR(key_id_blob)) {
+		ret = PTR_ERR(key_id_blob);
+		goto e_free;
+	}
+
+	data->key_id_address = __psp_pa(key_id_blob);
+	data->key_id_len = input.key_id_len;
+
+	/* Allocate a physically contiguous buffer to store the public key blob. */
+	if ((input.pubkey_len > SEV_FW_BLOB_MAX_SIZE) ||
+	    !access_ok(input.pubkey_address, input.pubkey_len)) {
+		ret = -EFAULT;
+		goto e_free_key_id;
+	}
+
+	pubkey_blob = kmalloc(input.pubkey_len, GFP_KERNEL);
+	if (!pubkey_blob) {
+		ret = -ENOMEM;
+		goto e_free_key_id;
+	}
+
+	data->pubkey_address = __psp_pa(pubkey_blob);
+	data->pubkey_len = input.pubkey_len;
+
+cmd:
+	/* If platform is not in INIT state then transition it to INIT. */
+	if (psp_master->sev_state != SEV_STATE_INIT) {
+		ret = __sev_platform_init_locked(&argp->error);
+		if (ret)
+			goto e_free_pubkey;
+	}
+
+	ret = __sev_do_cmd_locked(SEV_CMD_GM_PUBKEY_GEN, data, &argp->error);
+
+	/* If we query the length, FW responded with expected data. */
+	input.pubkey_len = data->pubkey_len;
+
+	if (copy_to_user((void __user *)argp->data, &input, sizeof(input))) {
+		ret = -EFAULT;
+		goto e_free_pubkey;
+	}
+
+	if (pubkey_blob) {
+		if (copy_to_user((void __user *)input.pubkey_address,
+				 pubkey_blob, input.pubkey_len)) {
+			ret = -EFAULT;
+			goto e_free_pubkey;
+		}
+	}
+
+e_free_pubkey:
+	kfree(pubkey_blob);
+e_free_key_id:
+	kfree(key_id_blob);
+e_free:
+	kfree(data);
+	return ret;
+}
+
 static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg)
 {
 	void __user *argp = (void __user *)arg;
@@ -766,6 +846,9 @@ static long sev_ioctl(struct file *file, unsigned int ioctl, unsigned long arg)
 	case SEV_GET_ID:
 		ret = sev_ioctl_do_get_id(&input);
 		break;
+	case SEV_GM_PUBKEY_GEN:
+		ret = sev_ioctl_do_gm_pubkey_gen(&input);
+		break;
 	default:
 		ret = -EINVAL;
 		goto out;
diff --git a/include/uapi/linux/psp-sev.h b/include/uapi/linux/psp-sev.h
index ac8c60b..7482cbd 100644
--- a/include/uapi/linux/psp-sev.h
+++ b/include/uapi/linux/psp-sev.h
@@ -32,6 +32,8 @@ enum {
 	SEV_PEK_CERT_IMPORT,
 	SEV_GET_ID,
 
+	SEV_GM_PUBKEY_GEN,
+
 	SEV_MAX,
 };
 
@@ -136,6 +138,21 @@ struct sev_user_data_get_id {
 } __packed;
 
 /**
+ * struct sev_user_data_gm_pubkey_gen - GM_PUBKEY_GEN command parameters
+ *
+ * @key_id_address: address of key id
+ * @key_id_len: len of key id
+ * @pubkey_address: address of GM public key
+ * @pubkey_len: len of GM public key
+ */
+struct sev_user_data_gm_pubkey_gen {
+	__u64 key_id_address;		/* In */
+	__u32 key_id_len;			/* In */
+	__u64 pubkey_address;		/* In */
+	__u32 pubkey_len;			/* In/Out */
+} __packed;
+
+/**
  * struct sev_issue_cmd - SEV ioctl parameters
  *
  * @cmd: SEV commands to execute
-- 
2.7.4

[PATCH 4/6] KVM: Define Hygon SEV commands

[Hao Feng]

1. KVM_SEV_GM_GET_DIGEST
------------------------
The command is used to get the key digest from SEV firmware, guest
owner will check the key digest to see if the key negotiation is
successful or not.

2. KVM_SEV_GM_VERIFY_DIGEST
---------------------------
The command is used to send guest owner's key digest to SEV firmware,
firmware will check the key digest to see if the key negotiation is
successful or not.

Signed-off-by: Hao Feng <fenghao@hygon.cn>
---
 include/uapi/linux/kvm.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 6d4ea4b..3eb8858 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1469,6 +1469,10 @@ enum sev_cmd_id {
 	/* Guest certificates commands */
 	KVM_SEV_CERT_EXPORT,
 
+	/* GM specific commands */
+	KVM_SEV_GM_GET_DIGEST,
+	KVM_SEV_GM_VERIFY_DIGEST,
+
 	KVM_SEV_NR_MAX,
 };
 
@@ -1520,6 +1524,16 @@ struct kvm_sev_dbg {
 	__u32 len;
 };
 
+struct kvm_sev_gm_get_digest {
+	__u64 uaddr;
+	__u32 len;
+};
+
+struct kvm_sev_gm_verify_digest {
+	__u64 uaddr;
+	__u32 len;
+};
+
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 #define KVM_DEV_ASSIGN_PCI_2_3		(1 << 1)
 #define KVM_DEV_ASSIGN_MASK_INTX	(1 << 2)
-- 
2.7.4

[PATCH 5/6] KVM: SVM: Add support for KVM_SEV_GM_GET_DIGEST command

[Hao Feng]

The command is used to get the key digest from SEV firmware, guest
owner will check the key digest to see if the key negotiation is
successful or not.

Signed-off-by: Hao Feng <fenghao@hygon.cn>
Signed-off-by: Pu Wen <puwen@hygon.cn>
---
 arch/x86/kvm/svm.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index e0a791c..f8e7042 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -6946,6 +6946,75 @@ static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
+static int sev_gm_get_digest(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+	void __user *digest = (void __user *)(uintptr_t)argp->data;
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_gm_get_digest *data;
+	struct kvm_sev_gm_get_digest params;
+	void __user *p = NULL;
+	void *blob = NULL;
+	int ret;
+
+	if (!sev_guest(kvm))
+		return -ENOTTY;
+
+	if (copy_from_user(&params, digest, sizeof(params)))
+		return -EFAULT;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	/* User wants to query the blob length */
+	if (!params.len)
+		goto cmd;
+
+	p = (void __user *)(uintptr_t)params.uaddr;
+	if (p) {
+		if (params.len > SEV_FW_BLOB_MAX_SIZE) {
+			ret = -EINVAL;
+			goto e_free;
+		}
+
+		ret = -ENOMEM;
+		blob = kmalloc(params.len, GFP_KERNEL);
+		if (!blob)
+			goto e_free;
+
+		data->address = __psp_pa(blob);
+		data->len = params.len;
+	}
+
+cmd:
+	data->handle = sev->handle;
+	ret = sev_issue_cmd(kvm, SEV_CMD_GM_GET_DIGEST, data, &argp->error);
+
+	/*
+	 * If we query the session length, FW responded with expected data.
+	 */
+	if (!params.len)
+		goto done;
+
+	if (ret)
+		goto e_free_blob;
+
+	if (blob) {
+		if (copy_to_user(p, blob, params.len))
+			ret = -EFAULT;
+	}
+
+done:
+	params.len = data->len;
+	if (copy_to_user(digest, &params, sizeof(params)))
+		ret = -EFAULT;
+e_free_blob:
+	kfree(blob);
+e_free:
+	kfree(data);
+	return ret;
+}
+
 static int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;
@@ -6987,6 +7056,9 @@ static int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 	case KVM_SEV_LAUNCH_SECRET:
 		r = sev_launch_secret(kvm, &sev_cmd);
 		break;
+	case KVM_SEV_GM_GET_DIGEST:
+		r = sev_gm_get_digest(kvm, &sev_cmd);
+		break;
 	default:
 		r = -EINVAL;
 		goto out;
-- 
2.7.4

[PATCH 6/6] KVM: SVM: Add support for KVM_SEV_GM_VERIFY_DIGEST command

[Hao Feng]

The command is used to send guest owner's key digest to SEV firmware,
firmware will check the key digest to see if the key negotiation is
successful or not.

Signed-off-by: Hao Feng <fenghao@hygon.cn>
Signed-off-by: Pu Wen <puwen@hygon.cn>
---
 arch/x86/kvm/svm.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index f8e7042..4dbdccf 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -7015,6 +7015,50 @@ static int sev_gm_get_digest(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
+static int sev_gm_verify_digest(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+	void __user *digest = (void __user *)(uintptr_t)argp->data;
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_gm_verify_digest *data = NULL;
+	struct kvm_sev_gm_verify_digest params;
+	void *digest_blob = NULL;
+	int *error = &argp->error;
+	int ret;
+
+	if (!sev_guest(kvm))
+		return -ENOTTY;
+
+	if (copy_from_user(&params, digest, sizeof(params)))
+		return -EFAULT;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	if (params.uaddr) {
+		digest_blob = psp_copy_user_blob(params.uaddr, params.len);
+		if (IS_ERR(digest_blob)) {
+			ret = PTR_ERR(digest_blob);
+			goto e_free;
+		}
+
+		data->address = __psp_pa(digest_blob);
+		data->len = params.len;
+	}
+
+	data->handle = sev->handle;
+	ret = sev_issue_cmd(kvm, SEV_CMD_GM_VERIFY_DIGEST, data, error);
+	if (ret)
+		goto e_free_digest;
+
+e_free_digest:
+	kfree(digest_blob);
+e_free:
+	kfree(data);
+
+	return ret;
+}
+
 static int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;
@@ -7059,6 +7103,9 @@ static int svm_mem_enc_op(struct kvm *kvm, void __user *argp)
 	case KVM_SEV_GM_GET_DIGEST:
 		r = sev_gm_get_digest(kvm, &sev_cmd);
 		break;
+	case KVM_SEV_GM_VERIFY_DIGEST:
+		r = sev_gm_verify_digest(kvm, &sev_cmd);
+		break;
 	default:
 		r = -EINVAL;
 		goto out;
-- 
2.7.4

Re: [PATCH 5/6] KVM: SVM: Add support for KVM_SEV_GM_GET_DIGEST command

[Borislav Petkov]

On Mon, Apr 15, 2019 at 08:04:27PM +0800, Hao Feng wrote:
> The command is used to get the key digest from SEV firmware, guest
> owner will check the key digest to see if the key negotiation is
> successful or not.
> 
> Signed-off-by: Hao Feng <fenghao@hygon.cn>
> Signed-off-by: Pu Wen <puwen@hygon.cn>

Your both SOB chains are wrong. See:

  11) Sign your work - the Developer's Certificate of Origin
  12) When to use Acked-by:, Cc:, and Co-developed-by:
  13) Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:

sections in

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH 0/6] Add Hygon SEV support

[Lendacky, Thomas]

On 4/15/19 7:04 AM, Hao Feng wrote:
> Hygon SEV follows AMD SEV work flow, but uses China national standard
> cryptographic algorithms SM2/SM3/SM4 instead of (RSA, ECDSA,
> ECDH)/SHA/AES. Reuse most AMD SEV code path to support Hygon SEV,
> also adds 3 new commands(GM_PUBKEY_GEN, GM_GET_DIGEST,
> GM_VERIFY_DIGEST) to support SM2 key exchange protocol.
> 
> SM2 is based on ECC(Elliptic Curve Cryptography), and uses a special
> curve. It can be used in digital signature, key exchange and
> asymmetric cryptography. For key exchange, SM2 is similar to ECDH, but
> involves new random key, meaning the two sides need to exchange extra
> random public key besides their public key, that's why additional APIs
> are needed to support Hygon SEV.
> SM3 is a hash algorithm, similar to SHA-256.
> SM4 is a block cipher algorithm, similar to AES-128.
> 
> 1. GM_PUBKEY_GEN
> ----------------
> The command is used to get SM2 random public key from SEV firmware to
> compute share key.
> 
> Parameters:
> * GM_KEY_ID_PADDR (in) - Address of key ID for the random public key.
> * GM_KEY_ID_LEN (in) - Length of key ID.
> * GM_PUBKEY_PADDR (in) - Address of the random public key.
> * GM_PUBKEY_LEN (in,out) - Length of the random public key.
> 
> 2. GM_GET_DIGEST
> ----------------
> The command is used to get key digest from SEV firmware during SM2 key
> exchange, guest owner can check the digest to see if the key
> negotiation is successful or not.
> 
> Parameters:
> * HANDLE (in) - Guest handle
> * DIGEST_PADDR (in) - Address of the key digest
> * DIGEST_LEN (in, out) - Length of the key digest
> 
> 3. GM_VERIFY_DIGEST
> -------------------
> The command is used to send guest owner's key digest to SEV firmware
> during SM2 key exchange, firmware can check the digest to see if the
> negotiation is successful or not.
> 
> Parameters:
> * HANDLE (in) - Guest handle
> * DIGEST_PADDR (in) - Address of the key digest
> * DIGEST_LEN (in) - Length of the key digest
> 
> Already tested successfully on Hygon DhyanaPlus processor, also tested
> successfully on AMD EPYC processor, results show no side effect on
> current AMD SEV implementation.

You can't just randomly add commands to the SEV API.  You will need to
work with the SEV API specification owner to be sure there are not any
conflicts or other issues with what you are trying to do. Please work
with Richard Relph (Richard.Relph@amd.com) on this.

For now, NAK on all of this.

Thanks,
Tom

> 
> Hao Feng (6):
>   crypto: ccp: Add Hygon Dhyana support
>   crypto: ccp: Define Hygon SEV commands
>   crypto: ccp: Implement SEV_GM_PUBKEY_GEN ioctl command
>   KVM: Define Hygon SEV commands
>   KVM: SVM: Add support for KVM_SEV_GM_GET_DIGEST command
>   KVM: SVM: Add KVM_SEV_GM_VERIFY_DIGEST command
> 
>  arch/x86/kvm/svm.c           | 119 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/crypto/ccp/psp-dev.c |  86 +++++++++++++++++++++++++++++++
>  drivers/crypto/ccp/sp-pci.c  |   2 +
>  include/linux/psp-sev.h      |  49 ++++++++++++++++++
>  include/uapi/linux/kvm.h     |  14 +++++
>  include/uapi/linux/psp-sev.h |  17 +++++++
>  6 files changed, 287 insertions(+)
> 

Re: [PATCH 0/6] Add Hygon SEV support

[Paolo Bonzini]

On 15/04/19 14:04, Hao Feng wrote:
> Hygon SEV follows AMD SEV work flow, but uses China national standard
> cryptographic algorithms SM2/SM3/SM4 instead of (RSA, ECDSA,
> ECDH)/SHA/AES. Reuse most AMD SEV code path to support Hygon SEV,
> also adds 3 new commands(GM_PUBKEY_GEN, GM_GET_DIGEST,
> GM_VERIFY_DIGEST) to support SM2 key exchange protocol.
> 
> SM2 is based on ECC(Elliptic Curve Cryptography), and uses a special
> curve. It can be used in digital signature, key exchange and
> asymmetric cryptography. For key exchange, SM2 is similar to ECDH, but
> involves new random key, meaning the two sides need to exchange extra
> random public key besides their public key, that's why additional APIs
> are needed to support Hygon SEV.
> SM3 is a hash algorithm, similar to SHA-256.
> SM4 is a block cipher algorithm, similar to AES-128.

I don't see SM2 and SM3 implemented elsewhere in Linux.  SM4 is only
supported by a few wireless drivers.

Apart from the concerns that Thomas mentioned, you have to convince the
rest of the community that these primitives are secure (for example by
contributing support for them to drivers/crypto), and then KVM will
start using them.

Because as far as I know, they could be just as secure as double rot13.

Paolo

RE: [PATCH 0/6] Add Hygon SEV support

[Pascal Van Leeuwen]

> -----Original Message-----
> From: linux-crypto-owner@vger.kernel.org [mailto:linux-crypto-
> owner@vger.kernel.org] On Behalf Of Paolo Bonzini
> Sent: Monday, April 15, 2019 5:38 PM
> To: Hao Feng <fenghao@hygon.cn>; 'Tom Lendacky '
> <thomas.lendacky@amd.com>; 'Gary Hook ' <gary.hook@amd.com>; 'Herbert
> Xu ' <herbert@gondor.apana.org.au>; ' David S. Miller '
> <davem@davemloft.net>; 'Janakarajan Natarajan '
> <Janakarajan.Natarajan@amd.com>; 'Joerg Roedel ' <joro@8bytes.org>; '
> Radim Krčmář ' <rkrcmar@redhat.com>; 'Thomas Gleixner '
> <tglx@linutronix.de>; 'Ingo Molnar ' <mingo@redhat.com>; 'Borislav
> Petkov ' <bp@alien8.de>; ' H. Peter Anvin ' <hpa@zytor.com>
> Cc: 'Zhaohui Du ' <duzhaohui@hygon.cn>; 'Zhiwei Ying '
> <yingzhiwei@hygon.cn>; 'Wen Pu ' <puwen@hygon.cn>; x86@kernel.org;
> linux-crypto@vger.kernel.org; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 0/6] Add Hygon SEV support
>
> On 15/04/19 14:04, Hao Feng wrote:
> > Hygon SEV follows AMD SEV work flow, but uses China national standard
> > cryptographic algorithms SM2/SM3/SM4 instead of (RSA, ECDSA,
> > ECDH)/SHA/AES. Reuse most AMD SEV code path to support Hygon SEV,
> > also adds 3 new commands(GM_PUBKEY_GEN, GM_GET_DIGEST,
> > GM_VERIFY_DIGEST) to support SM2 key exchange protocol.
> >
> > SM2 is based on ECC(Elliptic Curve Cryptography), and uses a special
> > curve. It can be used in digital signature, key exchange and
> > asymmetric cryptography. For key exchange, SM2 is similar to ECDH,
> but
> > involves new random key, meaning the two sides need to exchange extra
> > random public key besides their public key, that's why additional
> APIs
> > are needed to support Hygon SEV.
> > SM3 is a hash algorithm, similar to SHA-256.
> > SM4 is a block cipher algorithm, similar to AES-128.
>
> I don't see SM2 and SM3 implemented elsewhere in Linux.  SM4 is only
> supported by a few wireless drivers.
>
> Apart from the concerns that Thomas mentioned, you have to convince the
> rest of the community that these primitives are secure (for example by
> contributing support for them to drivers/crypto), and then KVM will
> start using them.
>

I don't know about SM2, but both SM3 and SM4 are already implemented in
the kernel tree as generic C code and covered by the testmgr.

There also has been quite some analysis done on them (Google is your
friend) and they are generally considered secure. Besides that, they are
in heavy practical use in mainland China, usually as direct replacements
for SHA2-256 and AES in whatever protocol or use case you need: IPsec,
TLS, WPA2, XTS for disk encryption, you name it.

>
> Because as far as I know, they could be just as secure as double rot13.
>

You could educate yourself first instead of just making assumptions?

Regards,

Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines

Tel. : +31 (0)73 65 81 900

www.insidesecure.com

Re: [PATCH 0/6] Add Hygon SEV support

[Paolo Bonzini]

On 15/04/19 17:51, Pascal Van Leeuwen wrote:
> I don't know about SM2, but both SM3 and SM4 are already implemented in
> the kernel tree as generic C code and covered by the testmgr.

I stand corrected.

> There also has been quite some analysis done on them (Google is your
> friend) and they are generally considered secure.

Good.

> Besides that, they are
> in heavy practical use in mainland China, usually as direct replacements
> for SHA2-256 and AES in whatever protocol or use case you need: IPsec,
> TLS, WPA2, XTS for disk encryption, you name it.

How should that mean anything?

>> Because as far as I know, they could be just as secure as double rot13.
> 
> You could educate yourself first instead of just making assumptions?
I did educate myself a bit, but I'm not an expert in cryptography, so I
would like to be sure that these are not another Speck or DUAL-EC-DRBG.
 "SM2 is based on ECC(Elliptic Curve Cryptography), and uses a special
curve" is enough for me to see warning signs, at least without further
explanations, and so does the fact that the initial SM3 values were
changed from SHA-2 and AFAICT there is no public justification for that.

Paolo

RE: [PATCH 0/6] Add Hygon SEV support

[Pascal Van Leeuwen]

> > Besides that, they are
> > in heavy practical use in mainland China, usually as direct
> replacements
> > for SHA2-256 and AES in whatever protocol or use case you need:
> IPsec,
> > TLS, WPA2, XTS for disk encryption, you name it.
>
> How should that mean anything?
>
Uhm ... no, the fact that something is actually *useful* to potentially
a billion plus people doesn't mean anything ...

> I did educate myself a bit, but I'm not an expert in cryptography, so I
> would like to be sure that these are not another Speck or DUAL-EC-DRBG.
>
Innocent until proven guilty mean anything to you?

>  "SM2 is based on ECC(Elliptic Curve Cryptography), and uses a special
> curve" is enough for me to see warning signs, at least without further
> explanations,
>
The specification is public (if you can read Chinese, anyway), so open to
analysis. Either way, it's quite irrelevant to Chinese organisations that
HAVE to use SM2. And anyone else can just decide NOT to use it, you don't
even have to compile it into your kernel. It's called freedom.

> and so does the fact that the initial SM3 values were
> changed from SHA-2 and AFAICT there is no public justification for
> that.
>
Actually, SM3 is an *improvement* on SHA-2, and there has been ample
analysis done on that to, in fact, confirm it's (slightly) better.
So there IS public justification. Don't shout if you don't know the
facts.

Regards,
Pascal

Re: [PATCH 0/6] Add Hygon SEV support

[Paolo Bonzini]

On 16/04/19 08:58, Pascal Van Leeuwen wrote:
>>> Besides that, they are in heavy practical use in mainland China, 
>>> usually as direct replacements for SHA2-256 and AES in whatever 
>>> protocol or use case you need: IPsec, TLS, WPA2, XTS for disk encryption,
>>> you name it.
>>
>> How should that mean anything?
>
> Uhm ... no, the fact that something is actually *useful* to potentially
> a billion plus people doesn't mean anything ...

Useful does not mean secure, does it?  PKZIP encryption was certainly
useful back in the day, but it was not secure.

>> I did educate myself a bit, but I'm not an expert in cryptography, so I
>> would like to be sure that these are not another Speck or DUAL-EC-DRBG.
>
> Innocent until proven guilty mean anything to you?

This is not a court of justice, it's a software project.  For that
matter "certainty beyond reasonable doubt" is not a thing either in this
context.

>>  "SM2 is based on ECC(Elliptic Curve Cryptography), and uses a special
>> curve" is enough for me to see warning signs, at least without further
>> explanations,
>>
> The specification is public (if you can read Chinese, anyway), so open to
> analysis. Either way, it's quite irrelevant to Chinese organisations that
> HAVE to use SM2. And anyone else can just decide NOT to use it, you don't
> even have to compile it into your kernel. It's called freedom.

"Freedom" didn't apply when Speck was proposed for inclusion in Linux,
and I would like to make sure I don't make a mistake when adding crypto
interfaces.  If SM2/3/4 were broken, I couldn't care less if someone HAS
to use them, they can patch their kernel.  But if they're not then I
appreciate that you wrote to correct me, it's helpful.  Please
understand that 99% of the community has not ever heard of anything but
SHA-{1,2,3}, ECDSA, Ed25519, AES.  If somebody comes up with a patch
with "strange" crypto, it's up to them to say that they are secure---and
again, the key word is secure, not useful.

Paolo

>> and so does the fact that the initial SM3 values were
>> changed from SHA-2 and AFAICT there is no public justification for
>> that.
>>
> Actually, SM3 is an *improvement* on SHA-2, and there has been ample
> analysis done on that to, in fact, confirm it's (slightly) better.
> So there IS public justification. Don't shout if you don't know the
> facts.

Re: [PATCH 5/6] KVM: SVM: Add support for KVM_SEV_GM_GET_DIGEST command

[Borislav Petkov]

On Tue, Apr 16, 2019 at 07:54:40AM +0000, Hao Feng wrote:
> Thanks for the comments, I thought there could be multiple signers...

There can be multiple signers but as you've read in the sections above,
those SOBs all have a meaning and in your case it wasn't clear what that
meaning is.

> After reading through the guidelines again, seems I should use the
> following statements instead?
> 
> Co-developed-by: Pu Wen <puwen@hygon.cn><mailto:puwen@hygon.cn>
> Signed-off-by: Pu Wen <puwen@hygon.cn><mailto:puwen@hygon.cn>
> Signed-off-by: Hao Feng <fenghao@hygon.cn><mailto:fenghao@hygon.cn>

Yes, as long as you realize that having the From: you makes you the
author of the patch and Pu Wen is fine with that.

Btw, you should fix your mailer to send plain text only and not add
those <mailto: > crap tags above.

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

RE: [PATCH 0/6] Add Hygon SEV support

[Pascal Van Leeuwen]

> >
> > Uhm ... no, the fact that something is actually *useful* to
> potentially
> > a billion plus people doesn't mean anything ...
>
> Useful does not mean secure, does it?  PKZIP encryption was certainly
> useful back in the day, but it was not secure.
>
"Secure" is a relative term anyway. There's always a trade-off between
performance, cost, power consumption and security. Different use cases
require different levels of security. IMHO that decision should be up
to the application / user / market, and not up to some software
engineers that are not experts on the subject matter anyway (but I am
hopeful that some people here are, in fact, experts to some extent).

> "Freedom" didn't apply when Speck was proposed for inclusion in Linux,
> and I would like to make sure I don't make a mistake when adding crypto
> interfaces.  If SM2/3/4 were broken, I couldn't care less if someone
> HAS to use them, they can patch their kernel.
>
Is Speck actually used in any real-life protocol or application?
I did not follow the Speck discussion but I have a hunch that was a far
more important reason not to include it than it being a weak cipher or
it's shady NSA origins ...

And yes, they can always fork the kernel and do their own stuff with it,
but that's going to be a support nightmare for people - like us - wanting
to add HW acceleration on top of that. And yes, "we" can do SM3 & SM4.
Full disclosure: it is in my/our interest to keep SM3 & SM4 in the tree.

>  But if they're not then I appreciate that you wrote to correct me,
> it's helpful.  Please
> understand that 99% of the community has not ever heard of anything but
> SHA-{1,2,3}, ECDSA, Ed25519, AES.  If somebody comes up with a patch
> with "strange" crypto, it's up to them to say that they are secure---
> and again, the key word is secure, not useful.
>
I recognise the fact that most people are not experts on the subject
matter. However, there's a lot you can find out in a short Google session
before you start a discussion on incorrect assumptions ...
Anyway, always happy to educate people a bit.

Regards,

Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Inside Secure

Re: [PATCH 0/6] Add Hygon SEV support

[Hao Feng]

On 4/16/19 4:09 PM, Paolo Bonzini wrote:
> Useful does not mean secure, does it?  PKZIP encryption was certainly
> useful back in the day, but it was not secure.
> 

Thanks for the comments, quite understand the concern about security
because it is indeed so important. Many people may not hear about
SM2, so it is natural to doubt its security. Actually SM2 is not just China
standard, it is also ISO/IEC standard, this fact should give us some
confidence about its security.

You can get more information about SM2 from below ISO/IEC specification.
https://www.iso.org/obp/ui/#iso:std:iso-iec:14888:-3:ed-4:v1:en

-- 
Thanks
Harry

Re: [PATCH 5/6] KVM: SVM: Add support for KVM_SEV_GM_GET_DIGEST command

[Hao Feng]

On 4/16/19 4:15 PM, Borislav Petkov wrote:
> Yes, as long as you realize that having the From: you makes you the
> author of the patch and Pu Wen is fine with that.

Ok, will modify in next patch series.

> 
> Btw, you should fix your mailer to send plain text only and not add
> those <mailto: > crap tags above.
> 

Sorry about that, there was some incorrect configuration in my email
client, already corrected it, thanks.


-- 
Thanks
Harry