2019-04-15 12:37:11

by Hao Feng

[permalink] [raw]
Subject: [PATCH 0/6] Add Hygon SEV support

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



2019-04-15 12:35:37

by Hao Feng

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

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 <[email protected]>
---
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


2019-04-15 12:36:12

by Hao Feng

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

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 <[email protected]>
---
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


2019-04-15 12:36:31

by Hao Feng

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

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 <[email protected]>
---
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


2019-04-15 15:32:27

by Tom Lendacky

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

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 ([email protected]) 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(+)
>

2019-04-15 15:38:26

by Paolo Bonzini

[permalink] [raw]
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.

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

Paolo

2019-04-15 15:52:07

by Pascal Van Leeuwen

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

> -----Original Message-----
> From: [email protected] [mailto:linux-crypto-
> [email protected]] On Behalf Of Paolo Bonzini
> Sent: Monday, April 15, 2019 5:38 PM
> To: Hao Feng <[email protected]>; 'Tom Lendacky '
> <[email protected]>; 'Gary Hook ' <[email protected]>; 'Herbert
> Xu ' <[email protected]>; ' David S. Miller '
> <[email protected]>; 'Janakarajan Natarajan '
> <[email protected]>; 'Joerg Roedel ' <[email protected]>; '
> Radim Krčmář ' <[email protected]>; 'Thomas Gleixner '
> <[email protected]>; 'Ingo Molnar ' <[email protected]>; 'Borislav
> Petkov ' <[email protected]>; ' H. Peter Anvin ' <[email protected]>
> Cc: 'Zhaohui Du ' <[email protected]>; 'Zhiwei Ying '
> <[email protected]>; 'Wen Pu ' <[email protected]>; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]
> 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

http://www.insidesecure.com

2019-04-15 16:05:08

by Paolo Bonzini

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

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

2019-04-16 06:58:28

by Pascal Van Leeuwen

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

> > 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

2019-04-16 08:10:03

by Paolo Bonzini

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

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.

2019-04-16 09:09:04

by Pascal Van Leeuwen

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

> >
> > 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



2019-04-16 10:35:25

by Hao Feng

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

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