All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] crypto: Introduce ECDSA algorithm
@ 2022-06-13  8:45 Lei He
  2022-06-13  8:45 ` [PATCH 1/7] crypto: Introduce ECDSA algorithm API Lei He
                   ` (6 more replies)
  0 siblings, 7 replies; 21+ messages in thread
From: Lei He @ 2022-06-13  8:45 UTC (permalink / raw)
  To: mst, arei.gonglei, berrange, qemu-devel; +Cc: helei.sig11, pizhenwei, jasowang

This patch introduced ECDSA algorithm for crypto:
1. make the built-in ASN.1 decoder support more ASN.1 types.
2. support ECDSA key and signature parsing.
3. implement the ECDSA algorithm using nettle and gcrypt respectively.

Lei He (7):
  crypto: Introduce ECDSA algorithm API
  crypto: Support more ASN.1 types
  crypto: remove "qemu/osdep.h" in rsakey.h
  crypto: Add ECDSA key parser
  crypto: Implement ECDSA algorithm by hogweed
  crypto: Implement ECDSA algorithm by gcrypt
  crypto: Add test suite for ECDSA algorithm

 crypto/akcipher-gcrypt.c.inc      | 400 ++++++++++++++++++++++++++++++++++++++
 crypto/akcipher-nettle.c.inc      | 268 +++++++++++++++++++++++++
 crypto/der.c                      | 174 +++++++++++++++--
 crypto/der.h                      | 128 +++++++++++-
 crypto/ecdsakey-builtin.c.inc     | 248 +++++++++++++++++++++++
 crypto/ecdsakey.c                 | 118 +++++++++++
 crypto/ecdsakey.h                 |  66 +++++++
 crypto/meson.build                |   1 +
 crypto/rsakey.c                   |   1 +
 crypto/rsakey.h                   |   1 -
 qapi/crypto.json                  |  28 ++-
 tests/unit/test-crypto-akcipher.c | 227 +++++++++++++++++++--
 tests/unit/test-crypto-der.c      | 126 +++++++++---
 13 files changed, 1722 insertions(+), 64 deletions(-)
 create mode 100644 crypto/ecdsakey-builtin.c.inc
 create mode 100644 crypto/ecdsakey.c
 create mode 100644 crypto/ecdsakey.h

-- 
2.11.0



^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/7] crypto: Introduce ECDSA algorithm API
  2022-06-13  8:45 [PATCH 0/7] crypto: Introduce ECDSA algorithm Lei He
@ 2022-06-13  8:45 ` Lei He
  2022-06-14  6:51   ` Philippe Mathieu-Daudé via
  2022-06-17 11:14   ` Daniel P. Berrangé
  2022-06-13  8:45 ` [PATCH 2/7] crypto: Support more ASN.1 types Lei He
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 21+ messages in thread
From: Lei He @ 2022-06-13  8:45 UTC (permalink / raw)
  To: mst, arei.gonglei, berrange, qemu-devel; +Cc: helei.sig11, pizhenwei, jasowang

Introduce ECDSA related structures to qapi-crypto.

Signed-off-by: lei he <helei.sig11@bytedance.com>
---
 qapi/crypto.json | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/qapi/crypto.json b/qapi/crypto.json
index 653e6e3f3d..c2fcdaa13a 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -545,7 +545,7 @@
 ##
 { 'enum': 'QCryptoAkCipherAlgorithm',
   'prefix': 'QCRYPTO_AKCIPHER_ALG',
-  'data': ['rsa']}
+  'data': ['rsa', 'ecdsa']}
 
 ##
 # @QCryptoAkCipherKeyType:
@@ -587,6 +587,29 @@
             'padding-alg': 'QCryptoRSAPaddingAlgorithm'}}
 
 ##
+# @QCryptoCurveID:
+#
+# The known curved for ECC algorithms.
+#
+# Since: 7.1
+##
+{ 'enum': 'QCryptoCurveID',
+  'prefix': 'QCRYPTO_CURVE_ID',
+  'data': ['nist-p192', 'nist-p256', 'nist-p384']}
+
+##
+# @QCryptoAkCipherOptionsECDSA:
+#
+# Specific parameters for ECDSA algorithm.
+#
+# @curve-id: QCryptoCurveId
+#
+# Since: 7.1
+##
+{ 'struct': 'QCryptoAkCipherOptionsECDSA',
+  'data': { 'curve-id':'QCryptoCurveID' }}
+
+##
 # @QCryptoAkCipherOptions:
 #
 # The options that are available for all asymmetric key algorithms
@@ -597,4 +620,5 @@
 { 'union': 'QCryptoAkCipherOptions',
   'base': { 'alg': 'QCryptoAkCipherAlgorithm' },
   'discriminator': 'alg',
-  'data': { 'rsa': 'QCryptoAkCipherOptionsRSA' }}
+  'data': { 'rsa': 'QCryptoAkCipherOptionsRSA',
+            'ecdsa': 'QCryptoAkCipherOptionsECDSA' }}
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 2/7] crypto: Support more ASN.1 types
  2022-06-13  8:45 [PATCH 0/7] crypto: Introduce ECDSA algorithm Lei He
  2022-06-13  8:45 ` [PATCH 1/7] crypto: Introduce ECDSA algorithm API Lei He
@ 2022-06-13  8:45 ` Lei He
  2022-06-17 11:20   ` Daniel P. Berrangé
  2022-06-13  8:45 ` [PATCH 3/7] crypto: remove "qemu/osdep.h" in rsakey.h Lei He
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Lei He @ 2022-06-13  8:45 UTC (permalink / raw)
  To: mst, arei.gonglei, berrange, qemu-devel; +Cc: helei.sig11, pizhenwei, jasowang

1. support decoding of 'bit string','octet string',
'object id', and 'context specific tag' for DER decoder.
2. support encoding of int and sequence for DER decoder.
3. add test suites for the above changes.

Signed-off-by: lei he <helei.sig11@bytedance.com>
---
 crypto/der.c                 | 174 +++++++++++++++++++++++++++++++++++++------
 crypto/der.h                 | 128 ++++++++++++++++++++++++++++++-
 tests/unit/test-crypto-der.c | 126 +++++++++++++++++++++++++------
 3 files changed, 382 insertions(+), 46 deletions(-)

diff --git a/crypto/der.c b/crypto/der.c
index f877390bbb..edf2c6c313 100644
--- a/crypto/der.c
+++ b/crypto/der.c
@@ -27,15 +27,68 @@ enum QCryptoDERTypeTag {
     QCRYPTO_DER_TYPE_TAG_INT = 0x2,
     QCRYPTO_DER_TYPE_TAG_BIT_STR = 0x3,
     QCRYPTO_DER_TYPE_TAG_OCT_STR = 0x4,
-    QCRYPTO_DER_TYPE_TAG_OCT_NULL = 0x5,
-    QCRYPTO_DER_TYPE_TAG_OCT_OID = 0x6,
+    QCRYPTO_DER_TYPE_TAG_NULL = 0x5,
+    QCRYPTO_DER_TYPE_TAG_OID = 0x6,
     QCRYPTO_DER_TYPE_TAG_SEQ = 0x10,
     QCRYPTO_DER_TYPE_TAG_SET = 0x11,
 };
 
-#define QCRYPTO_DER_CONSTRUCTED_MASK 0x20
+enum QCryptoDERTagClass {
+    QCRYPTO_DER_TAG_CLASS_UNIV = 0x0,
+    QCRYPTO_DER_TAG_CLASS_APPL = 0x1,
+    QCRYPTO_DER_TAG_CLASS_CONT = 0x2,
+    QCRYPTO_DER_TAG_CLASS_PRIV = 0x3,
+};
+
+enum QCryptoDERTagEnc {
+    QCRYPTO_DER_TAG_ENC_PRIM = 0x0,
+    QCRYPTO_DER_TAG_ENC_CONS = 0x1,
+};
+
+#define QCRYPTO_DER_TAG_ENC_MASK 0x20
+#define QCRYPTO_DER_TAG_ENC_SHIFT 5
+
+#define QCRYPTO_DER_TAG_CLASS_MASK 0xc0
+#define QCRYPTO_DER_TAG_CLASS_SHIFT 6
+
+#define QCRYPTO_DER_TAG_VAL_MASK 0x1f
 #define QCRYPTO_DER_SHORT_LEN_MASK 0x80
 
+#define QCRYPTO_DER_TAG(class, enc, val)        \
+    (((class) << QCRYPTO_DER_TAG_CLASS_SHIFT) | \
+     ((enc) << QCRYPTO_DER_TAG_ENC_SHIFT) | val)
+
+static void qcrypto_der_encode_data(const uint8_t *src, size_t src_len,
+                                    uint8_t *dst, size_t *dst_len)
+{
+    size_t max_length = 0xFF;
+    uint8_t length_bytes = 0, header_byte;
+
+    if (src_len < QCRYPTO_DER_SHORT_LEN_MASK) {
+        header_byte = src_len;
+        *dst_len = src_len + 1;
+    } else {
+        for (length_bytes = 1; max_length < src_len;) {
+            length_bytes++;
+            max_length = (max_length << 8) + max_length;
+        }
+        header_byte = length_bytes;
+        header_byte |= QCRYPTO_DER_SHORT_LEN_MASK;
+        *dst_len = src_len + length_bytes + 1;
+    }
+    if (!dst) {
+        return;
+    }
+    *dst++ = header_byte;
+    /* Bigendian length bytes */
+    while (length_bytes) {
+        *dst++ = (src_len >> (length_bytes - 1) * 8);
+        src_len >>= 8;
+        length_bytes--;
+    }
+    memcpy(dst, src, src_len);
+}
+
 static uint8_t qcrypto_der_peek_byte(const uint8_t **data, size_t *dlen)
 {
     return **data;
@@ -150,40 +203,119 @@ static int qcrypto_der_extract_data(const uint8_t **data, size_t *dlen,
     return qcrypto_der_extract_definite_data(data, dlen, cb, ctx, errp);
 }
 
-int qcrypto_der_decode_int(const uint8_t **data, size_t *dlen,
-                           QCryptoDERDecodeCb cb, void *ctx, Error **errp)
+static int qcrypto_der_decode_tlv(const uint8_t expected_tag,
+                                  const uint8_t **data, size_t *dlen,
+                                  QCryptoDERDecodeCb cb,
+                                  void *ctx, Error **errp)
 {
+    const uint8_t *saved_data = *data;
+    size_t saved_dlen = *dlen;
     uint8_t tag;
+    int data_length;
+
     if (*dlen < 1) {
         error_setg(errp, "Need more data");
         return -1;
     }
     tag = qcrypto_der_cut_byte(data, dlen);
+    if (tag != expected_tag) {
+        error_setg(errp, "Unexpected tag: expected: %u, actual: %u",
+                   expected_tag, tag);
+        goto error;
+    }
 
-    /* INTEGER must encoded in primitive-form */
-    if (tag != QCRYPTO_DER_TYPE_TAG_INT) {
-        error_setg(errp, "Invalid integer type tag: %u", tag);
-        return -1;
+    data_length = qcrypto_der_extract_data(data, dlen, cb, ctx, errp);
+    if (data_length < 0) {
+        goto error;
     }
+    return data_length;
+
+error:
+    *data = saved_data;
+    *dlen = saved_dlen;
+    return -1;
+}
 
-    return qcrypto_der_extract_data(data, dlen, cb, ctx, errp);
+int qcrypto_der_decode_int(const uint8_t **data, size_t *dlen,
+                           QCryptoDERDecodeCb cb, void *ctx, Error **errp)
+{
+    const uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                                        QCRYPTO_DER_TAG_ENC_PRIM,
+                                        QCRYPTO_DER_TYPE_TAG_INT);
+    return qcrypto_der_decode_tlv(tag, data, dlen, cb, ctx, errp);
 }
 
 int qcrypto_der_decode_seq(const uint8_t **data, size_t *dlen,
                            QCryptoDERDecodeCb cb, void *ctx, Error **errp)
 {
-    uint8_t tag;
-    if (*dlen < 1) {
-        error_setg(errp, "Need more data");
-        return -1;
-    }
-    tag = qcrypto_der_cut_byte(data, dlen);
+    uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                                  QCRYPTO_DER_TAG_ENC_CONS,
+                                  QCRYPTO_DER_TYPE_TAG_SEQ);
+    return qcrypto_der_decode_tlv(tag, data, dlen, cb, ctx, errp);
+}
 
-    /* SEQUENCE must use constructed form */
-    if (tag != (QCRYPTO_DER_TYPE_TAG_SEQ | QCRYPTO_DER_CONSTRUCTED_MASK)) {
-        error_setg(errp, "Invalid type sequence tag: %u", tag);
-        return -1;
+int qcrypto_der_decode_octet_str(const uint8_t **data, size_t *dlen,
+                                 QCryptoDERDecodeCb cb, void *ctx, Error **errp)
+{
+    uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                                  QCRYPTO_DER_TAG_ENC_PRIM,
+                                  QCRYPTO_DER_TYPE_TAG_OCT_STR);
+    return qcrypto_der_decode_tlv(tag, data, dlen, cb, ctx, errp);
+}
+
+int qcrypto_der_decode_bit_str(const uint8_t **data, size_t *dlen,
+                               QCryptoDERDecodeCb cb, void *ctx, Error **errp)
+{
+    uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                                  QCRYPTO_DER_TAG_ENC_PRIM,
+                                  QCRYPTO_DER_TYPE_TAG_BIT_STR);
+    return qcrypto_der_decode_tlv(tag, data, dlen, cb, ctx, errp);
+}
+
+int qcrypto_der_decode_oid(const uint8_t **data, size_t *dlen,
+                           QCryptoDERDecodeCb cb, void *ctx, Error **errp)
+{
+    uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                                  QCRYPTO_DER_TAG_ENC_PRIM,
+                                  QCRYPTO_DER_TYPE_TAG_OID);
+    return qcrypto_der_decode_tlv(tag, data, dlen, cb, ctx, errp);
+}
+
+int qcrypto_der_decode_ctx_tag(const uint8_t **data, size_t *dlen, int tag_id,
+                               QCryptoDERDecodeCb cb, void *ctx, Error **errp)
+{
+    uint8_t tag = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_CONT,
+                                  QCRYPTO_DER_TAG_ENC_CONS,
+                                  tag_id);
+    return qcrypto_der_decode_tlv(tag, data, dlen, cb, ctx, errp);
+}
+
+void qcrypto_der_encode_int(const uint8_t *src, size_t src_len,
+                            uint8_t *dst, size_t *dst_len)
+{
+    if (!dst) {
+        qcrypto_der_encode_data(src, src_len, NULL, dst_len);
+        *dst_len += 1;
+        return;
     }
+    *dst++ = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                             QCRYPTO_DER_TAG_ENC_PRIM,
+                             QCRYPTO_DER_TYPE_TAG_INT);
+    qcrypto_der_encode_data(src, src_len, dst, dst_len);
+    *dst_len += 1;
+}
 
-    return qcrypto_der_extract_data(data, dlen, cb, ctx, errp);
+void qcrypto_der_encode_seq(const uint8_t *src, size_t src_len,
+                            uint8_t *dst, size_t *dst_len)
+{
+    if (!dst) {
+        qcrypto_der_encode_data(src, src_len, NULL, dst_len);
+        *dst_len += 1;
+        return;
+    }
+    *dst++ = QCRYPTO_DER_TAG(QCRYPTO_DER_TAG_CLASS_UNIV,
+                             QCRYPTO_DER_TAG_ENC_CONS,
+                             QCRYPTO_DER_TYPE_TAG_SEQ);
+    qcrypto_der_encode_data(src, src_len, dst, dst_len);
+    *dst_len += 1;
 }
diff --git a/crypto/der.h b/crypto/der.h
index e3d3aeacdc..ec1436d531 100644
--- a/crypto/der.h
+++ b/crypto/der.h
@@ -47,14 +47,13 @@ typedef int (*QCryptoDERDecodeCb) (void *opaque, const uint8_t *value,
  * will be set to the rest length of data, if cb is not NULL, must
  * return 0 to make decode success, at last, the length of the data
  * part of the decoded INTEGER will be returned. Otherwise, -1 is
- * returned.
+ * returned and the valued of *data and *dlen keep unchanged.
  */
 int qcrypto_der_decode_int(const uint8_t **data,
                            size_t *dlen,
                            QCryptoDERDecodeCb cb,
                            void *opaque,
                            Error **errp);
-
 /**
  * qcrypto_der_decode_seq:
  *
@@ -70,7 +69,7 @@ int qcrypto_der_decode_int(const uint8_t **data,
  * will be set to the rest length of data, if cb is not NULL, must
  * return 0 to make decode success, at last, the length of the data
  * part of the decoded SEQUENCE will be returned. Otherwise, -1 is
- * returned.
+ * returned and the valued of *data and *dlen keep unchanged.
  */
 int qcrypto_der_decode_seq(const uint8_t **data,
                            size_t *dlen,
@@ -78,4 +77,127 @@ int qcrypto_der_decode_seq(const uint8_t **data,
                            void *opaque,
                            Error **errp);
 
+/**
+ * qcrypto_der_decode_oid:
+ *
+ * Decode OID from DER-encoded data, similar with der_decode_int.
+ *
+ * @data: pointer to address of input data
+ * @dlen: pointer to length of input data
+ * @cb: callback invoked when decode succeed, if cb equals NULL, no
+ * callback will be invoked
+ * @opaque: parameter passed to cb
+ *
+ * Returns: On success, *data points to rest data, and *dlen
+ * will be set to the rest length of data, if cb is not NULL, must
+ * return 0 to make decode success, at last, the length of the data
+ * part of the decoded OID will be returned. Otherwise, -1 is
+ * returned and the valued of *data and *dlen keep unchanged.
+ */
+int qcrypto_der_decode_oid(const uint8_t **data,
+                           size_t *dlen,
+                           QCryptoDERDecodeCb cb,
+                           void *opaque,
+                           Error **errp);
+
+/**
+ * qcrypto_der_decode_octet_str:
+ *
+ * Decode OCTET STRING from DER-encoded data, similar with der_decode_int.
+ *
+ * @data: pointer to address of input data
+ * @dlen: pointer to length of input data
+ * @cb: callback invoked when decode succeed, if cb equals NULL, no
+ * callback will be invoked
+ * @opaque: parameter passed to cb
+ *
+ * Returns: On success, *data points to rest data, and *dlen
+ * will be set to the rest length of data, if cb is not NULL, must
+ * return 0 to make decode success, at last, the length of the data
+ * part of the decoded OCTET STRING will be returned. Otherwise, -1 is
+ * returned and the valued of *data and *dlen keep unchanged.
+ */
+int qcrypto_der_decode_octet_str(const uint8_t **data,
+                                 size_t *dlen,
+                                 QCryptoDERDecodeCb cb,
+                                 void *opaque,
+                                 Error **errp);
+
+/**
+ * qcrypto_der_decode_bit_str:
+ *
+ * Decode BIT STRING from DER-encoded data, similar with der_decode_int.
+ *
+ * @data: pointer to address of input data
+ * @dlen: pointer to length of input data
+ * @cb: callback invoked when decode succeed, if cb equals NULL, no
+ * callback will be invoked
+ * @opaque: parameter passed to cb
+ *
+ * Returns: On success, *data points to rest data, and *dlen
+ * will be set to the rest length of data, if cb is not NULL, must
+ * return 0 to make decode success, at last, the length of the data
+ * part of the decoded BIT STRING will be returned. Otherwise, -1 is
+ * returned and the valued of *data and *dlen keep unchanged.
+ */
+int qcrypto_der_decode_bit_str(const uint8_t **data,
+                               size_t *dlen,
+                               QCryptoDERDecodeCb cb,
+                               void *opaque,
+                               Error **errp);
+
+
+/**
+ * qcrypto_der_decode_ctx_tag:
+ *
+ * Decode context specific tag
+ *
+ * @data: pointer to address of input data
+ * @dlen: pointer to length of input data
+ * @tag: expected value of context specific tag
+ * @cb: callback invoked when decode succeed, if cb equals NULL, no
+ * callback will be invoked
+ * @opaque: parameter passed to cb
+ *
+ * Returns: On success, *data points to rest data, and *dlen
+ * will be set to the rest length of data, if cb is not NULL, must
+ * return 0 to make decode success, at last, the length of the data
+ * part of the decoded BIT STRING will be returned. Otherwise, -1 is
+ * returned and the valued of *data and *dlen keep unchanged.
+ */
+int qcrypto_der_decode_ctx_tag(const uint8_t **data,
+                               size_t *dlen, int tag_id,
+                               QCryptoDERDecodeCb cb,
+                               void *opaque,
+                               Error **errp);
+
+/**
+ * qcrypto_der_encode_seq:
+ * @src: source data to be encoded
+ * @src_len: length of source data
+ * @dest: distination to save the encoded data, if dest is NULL, dst_len is
+ * set to expected buffer length and nothing more happends.
+ * @dst_len: output parameter, indicates how many bytes wat wrote to dest.
+ *
+ * Encode data as SEQUENCE in DER rules.
+ *
+ */
+void qcrypto_der_encode_seq(const uint8_t *src, size_t src_len,
+                            uint8_t *dst, size_t *dst_len);
+
+/**
+ * qcrypto_der_encode_int:
+ * @src: source data to be encoded
+ * @src_len: length of source data
+ * @dest: distination to save the encoded data, if dest is NULL, dst_len is
+ * set to expected buffer length and nothing more happends.
+ * @dst_len: output parameter, indicates how many bytes wat wrote to dest.
+ *
+ * Encode data as INTEGER in DER rules, the source data MUST be already
+ * encoded as two's complement in bigendian.
+ *
+ */
+void qcrypto_der_encode_int(const uint8_t *src, size_t src_len,
+                            uint8_t *dst, size_t *dst_len);
+
 #endif  /* QCRYPTO_ASN1_DECODER_H */
diff --git a/tests/unit/test-crypto-der.c b/tests/unit/test-crypto-der.c
index aed0f28d68..d218a7f170 100644
--- a/tests/unit/test-crypto-der.c
+++ b/tests/unit/test-crypto-der.c
@@ -147,13 +147,58 @@ static const uint8_t test_rsa2048_priv_key[] =
     "\x4e\x2f\x4c\xf9\xab\x97\x38\xe4\x20\x32\x32\x96\xc8\x9e\x79\xd3"
     "\x12";
 
+static const uint8_t test_ecdsa_p192_priv_key[] =
+    "\x30\x53"               /* SEQUENCE, offset 0, length 83 */
+    "\x02\x01\x01"           /* INTEGER, offset 2, length 1 */
+    "\x04\x18"               /* OCTET STRING, offset 5, length 24 */
+    "\xcb\xc8\x86\x0e\x66\x3c\xf7\x5a\x44\x13\xb8\xef\xea\x1d\x7b\xa6"
+    "\x1c\xda\xf4\x1b\xc7\x67\x6b\x35"
+    "\xa1\x34"               /* CONTEXT SPECIFIC 1, offset 31, length 52 */
+    "\x03\x32"               /* BIT STRING, offset 33, length 50 */
+    "\x00\x04\xc4\x16\xb3\xff\xac\xd5\x87\x98\xf7\xd9\x45\xfe\xd3\x5c"
+    "\x17\x9d\xb2\x36\x22\xcc\x07\xb3\x6d\x3c\x4e\x04\x5f\xeb\xb6\x52"
+    "\x58\xfb\x36\x10\x52\xb7\x01\x62\x0e\x94\x51\x1d\xe2\xef\x10\x82"
+    "\x88\x78";
+
+static const uint8_t test_ecdsa_p256_priv_key[] =
+    "\x30\x77"              /* SEQUENCE, offset 0, length 119 */
+    "\x02\x01\x01"          /* INTEGER, offset 2, length 1 */
+    "\x04\x20"              /* OCTET STRING, offset 5, length 32 */
+    "\xf6\x92\xdd\x29\x1c\x6e\xef\xb6\xb2\x73\x9f\x40\x1b\xb3\x2a\x28"
+    "\xd2\x37\xd6\x4a\x5b\xe4\x40\x4c\x6a\x95\x99\xfa\xf7\x92\x49\xbe"
+    "\xa0\x0a"              /* CONTEXT SPECIFIC 0, offset 39, length 10 */
+    "\x06\x08"              /* OID, offset 41, length 8 */
+    "\x2a\x86\x48\xce\x3d\x03\x01\x07"
+    "\xa1\x44"              /* CONTEXT SPECIFIC 1, offset 51, length 68 */
+    "\x03\x42"              /* BIT STRING, offset 53, length 66 */
+    "\x00\x04\xed\x42\x9c\x67\x79\xbe\x46\x83\x88\x3e\x8c\xc1\x33\xf3"
+    "\xc3\xf6\x2c\xf3\x13\x6a\x00\xc2\xc9\x3e\x87\x7f\x86\x39\xe6\xae"
+    "\xe3\xb9\xba\x2f\x58\x63\x32\x62\x62\x54\x07\x27\xf9\x5a\x3a\xc7"
+    "\x3a\x6b\x5b\xbc\x0d\x33\xba\xbb\xd4\xa3\xff\x4f\x9e\xdd\xf5\x59"
+    "\xc0\xf6";
+
 #define MAX_CHECKER_COUNT 32
 
+static int qcrypto_wrapped_decode_ctx_tag0(const uint8_t **data, size_t *dlen,
+                                           QCryptoDERDecodeCb cb, void *opaque,
+                                           Error **errp)
+{
+   return qcrypto_der_decode_ctx_tag(data, dlen, 0, cb, opaque, errp);
+}
+
+static int qcrypto_wrapped_decode_ctx_tag1(const uint8_t **data, size_t *dlen,
+                                           QCryptoDERDecodeCb cb, void *opaque,
+                                           Error **errp)
+{
+   return qcrypto_der_decode_ctx_tag(data, dlen, 1, cb, opaque, errp);
+}
+
 typedef struct QCryptoAns1DecoderResultChecker QCryptoAns1DecoderResultChecker;
 struct QCryptoAns1DecoderResultChecker {
     int (*action) (const uint8_t **data, size_t *dlen,
                    QCryptoDERDecodeCb cb, void *opaque, Error **errp);
     QCryptoDERDecodeCb cb;
+    bool constructed;
     const uint8_t *exp_value;
     size_t exp_vlen;
 };
@@ -204,7 +249,7 @@ static void test_ans1(const void *opaque)
         g_assert(checker->action(&c->data, &c->dlen, checker_callback,
                                  (void *)checker, &error_abort)
             == checker->exp_vlen);
-        if (checker->action == qcrypto_der_decode_seq) {
+        if (checker->constructed) {
             ++seq_depth;
             ctx[seq_depth].data = checker->exp_value;
             ctx[seq_depth].dlen = checker->exp_vlen;
@@ -225,25 +270,25 @@ static QCryptoAns1DecoderTestData test_data[] = {
     .test_data = test_rsa512_priv_key,
     .test_data_len = sizeof(test_rsa512_priv_key) - 1,
     .checker = {
-        { qcrypto_der_decode_seq, checker_callback,
+        { qcrypto_der_decode_seq, checker_callback, true,
           test_rsa512_priv_key + 4, 313 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 4 + 2, 1 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 7 + 2, 65 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 74 + 2, 3 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 79 + 2, 64 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 145 + 2, 33 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 180 + 2, 33 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 215 + 2, 32 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 249 + 2, 32 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa512_priv_key + 283 + 2, 32 },
     },
 },
@@ -252,29 +297,66 @@ static QCryptoAns1DecoderTestData test_data[] = {
     .test_data = test_rsa2048_priv_key,
     .test_data_len = sizeof(test_rsa2048_priv_key) - 1,
     .checker = {
-        { qcrypto_der_decode_seq, checker_callback,
+        { qcrypto_der_decode_seq, checker_callback, true,
           test_rsa2048_priv_key + 4, 1190 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 4 + 2, 1 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 7 + 4, 257 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 268 + 2, 3 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 273 + 4, 257 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 534 + 3, 129 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 666 + 3, 129 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 798 + 3, 129 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 930 + 3, 129 },
-        { qcrypto_der_decode_int, checker_callback,
+        { qcrypto_der_decode_int, checker_callback, false,
           test_rsa2048_priv_key + 1062 + 3, 129 },
     },
 },
-
+{
+    .path = "/crypto/der/parse-ecdsa-p192-priv-key",
+    .test_data = test_ecdsa_p192_priv_key,
+    .test_data_len = sizeof(test_ecdsa_p192_priv_key) - 1,
+    .checker = {
+        { qcrypto_der_decode_seq, checker_callback, true,
+          test_ecdsa_p192_priv_key + 2, 83 },
+        { qcrypto_der_decode_int, checker_callback, false,
+          test_ecdsa_p192_priv_key + 2 + 2, 1 },
+        { qcrypto_der_decode_octet_str, checker_callback, false,
+          test_ecdsa_p192_priv_key + 5 + 2, 24 },
+        { qcrypto_wrapped_decode_ctx_tag1, checker_callback, true,
+          test_ecdsa_p192_priv_key + 31 + 2, 52 },
+        { qcrypto_der_decode_bit_str , checker_callback, false,
+          test_ecdsa_p192_priv_key + 33 + 2, 50 },
+    },
+},
+{
+    .path = "/crypto/der/parse-ecdsa-p256-priv-key",
+    .test_data = test_ecdsa_p256_priv_key,
+    .test_data_len = sizeof(test_ecdsa_p256_priv_key) - 1,
+    .checker = {
+        { qcrypto_der_decode_seq, checker_callback, true,
+          test_ecdsa_p256_priv_key + 2, 119 },
+        { qcrypto_der_decode_int, checker_callback, false,
+          test_ecdsa_p256_priv_key + 2 + 2, 1 },
+        { qcrypto_der_decode_octet_str, checker_callback, false,
+          test_ecdsa_p256_priv_key + 5 + 2, 32 },
+        { qcrypto_wrapped_decode_ctx_tag0, checker_callback, true,
+          test_ecdsa_p256_priv_key + 39 + 2, 10 },
+        { qcrypto_der_decode_oid, checker_callback, false,
+          test_ecdsa_p256_priv_key + 41 + 2, 8 },
+        { qcrypto_wrapped_decode_ctx_tag1, checker_callback, true,
+          test_ecdsa_p256_priv_key + 51 + 2, 68 },
+        { qcrypto_der_decode_bit_str , checker_callback, false,
+          test_ecdsa_p256_priv_key + 53 + 2, 66 },
+    },
+},
 };
 
 int main(int argc, char **argv)
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 3/7] crypto: remove "qemu/osdep.h" in rsakey.h
  2022-06-13  8:45 [PATCH 0/7] crypto: Introduce ECDSA algorithm Lei He
  2022-06-13  8:45 ` [PATCH 1/7] crypto: Introduce ECDSA algorithm API Lei He
  2022-06-13  8:45 ` [PATCH 2/7] crypto: Support more ASN.1 types Lei He
@ 2022-06-13  8:45 ` Lei He
  2022-06-13 13:55   ` Philippe Mathieu-Daudé via
  2022-06-17 11:20   ` Daniel P. Berrangé
  2022-06-13  8:45 ` [PATCH 4/7] crypto: Add ECDSA key parser Lei He
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 21+ messages in thread
From: Lei He @ 2022-06-13  8:45 UTC (permalink / raw)
  To: mst, arei.gonglei, berrange, qemu-devel; +Cc: helei.sig11, pizhenwei, jasowang

Move 'include "qemu/osdep.h"' from rsakey.h to rsakey.c.

Signed-off-by: lei he <helei.sig11@bytedance.com>
---
 crypto/rsakey.c | 1 +
 crypto/rsakey.h | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/rsakey.c b/crypto/rsakey.c
index cc40e072f0..dcdbd9ec57 100644
--- a/crypto/rsakey.c
+++ b/crypto/rsakey.c
@@ -19,6 +19,7 @@
  *
  */
 
+#include "qemu/osdep.h"
 #include "rsakey.h"
 
 void qcrypto_akcipher_rsakey_free(QCryptoAkCipherRSAKey *rsa_key)
diff --git a/crypto/rsakey.h b/crypto/rsakey.h
index 974b76f659..ba88974d12 100644
--- a/crypto/rsakey.h
+++ b/crypto/rsakey.h
@@ -22,7 +22,6 @@
 #ifndef QCRYPTO_RSAKEY_H
 #define QCRYPTO_RSAKEY_H
 
-#include "qemu/osdep.h"
 #include "qemu/host-utils.h"
 #include "crypto/akcipher.h"
 
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 4/7] crypto: Add ECDSA key parser
  2022-06-13  8:45 [PATCH 0/7] crypto: Introduce ECDSA algorithm Lei He
                   ` (2 preceding siblings ...)
  2022-06-13  8:45 ` [PATCH 3/7] crypto: remove "qemu/osdep.h" in rsakey.h Lei He
@ 2022-06-13  8:45 ` Lei He
  2022-06-13 14:19   ` Philippe Mathieu-Daudé via
  2022-06-17 11:34   ` Daniel P. Berrangé
  2022-06-13  8:45 ` [PATCH 5/7] crypto: Implement ECDSA algorithm by hogweed Lei He
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 21+ messages in thread
From: Lei He @ 2022-06-13  8:45 UTC (permalink / raw)
  To: mst, arei.gonglei, berrange, qemu-devel; +Cc: helei.sig11, pizhenwei, jasowang

Add ECDSA key parser and ECDSA signautre parser.

Signed-off-by: lei he <helei.sig11@bytedance.com>
---
 crypto/ecdsakey-builtin.c.inc | 248 ++++++++++++++++++++++++++++++++++++++++++
 crypto/ecdsakey.c             | 118 ++++++++++++++++++++
 crypto/ecdsakey.h             |  66 +++++++++++
 crypto/meson.build            |   1 +
 4 files changed, 433 insertions(+)
 create mode 100644 crypto/ecdsakey-builtin.c.inc
 create mode 100644 crypto/ecdsakey.c
 create mode 100644 crypto/ecdsakey.h

diff --git a/crypto/ecdsakey-builtin.c.inc b/crypto/ecdsakey-builtin.c.inc
new file mode 100644
index 0000000000..5da317ec44
--- /dev/null
+++ b/crypto/ecdsakey-builtin.c.inc
@@ -0,0 +1,248 @@
+/*
+ * QEMU Crypto akcipher algorithms
+ *
+ * Copyright (c) 2022 Bytedance
+ * Author: lei he <helei.sig11@bytedance.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "der.h"
+#include "ecdsakey.h"
+
+#define QCRYPTO_ECDSA_PUBKEY_FMT_UNCOMPRESSED 0x04
+
+static int extract_mpi(void *ctx, const uint8_t *value,
+                       size_t vlen, Error **errp)
+{
+    QCryptoAkCipherMPI *mpi = (QCryptoAkCipherMPI *)ctx;
+    if (vlen == 0) {
+        error_setg(errp, "Empty mpi field");
+        return -1;
+    }
+    mpi->data = g_memdup2(value, vlen);
+    mpi->len = vlen;
+    return 0;
+}
+
+static int extract_version(void *ctx, const uint8_t *value,
+                           size_t vlen, Error **errp)
+{
+    uint8_t *version = (uint8_t *)ctx;
+    if (vlen != 1 || *value > 1) {
+        error_setg(errp, "Invalid rsakey version");
+        return -1;
+    }
+    *version = *value;
+    return 0;
+}
+
+static int extract_cons_content(void *ctx, const uint8_t *value,
+                                size_t vlen, Error **errp)
+{
+    const uint8_t **content = (const uint8_t **)ctx;
+    if (vlen == 0) {
+        error_setg(errp, "Empty sequence");
+        return -1;
+    }
+    *content = value;
+    return 0;
+}
+
+static int __qcrypto_akcipher_builtin_ecdsa_pubkey_parse(
+    QCryptoAkCipherECDSAKey *ecdsa,
+    const uint8_t *key, size_t keylen, Error **errp);
+
+static int extract_pubkey(void *ctx, const uint8_t *value,
+                          size_t vlen, Error **errp)
+{
+    QCryptoAkCipherECDSAKey *ecdsa = (QCryptoAkCipherECDSAKey *)ctx;
+    if (vlen < 4) {
+        error_setg(errp, "Public key part too short");
+        return -1;
+    }
+    /* Skip meta bit of BIT STRING */
+    value++;
+    vlen--;
+    return __qcrypto_akcipher_builtin_ecdsa_pubkey_parse(
+        ecdsa, value, vlen, errp);
+}
+
+/**
+ *
+ *        ECDSASignature ::= SEQUENCE {
+ *             r           INTEGER
+ *             s           INTEGER
+ *         }
+ */
+QCryptoAkCipherECDSASig *qcrypto_akcipher_ecdsasig_parse(
+    const uint8_t *signature, size_t len, Error **errp)
+{
+    QCryptoAkCipherECDSASig *sig = g_new0(QCryptoAkCipherECDSASig, 1);
+    const uint8_t *seq;
+    size_t seq_length;
+    int decode_ret;
+
+    decode_ret = qcrypto_der_decode_seq(&signature, &len,
+                                        extract_cons_content, &seq, errp);
+
+    if (decode_ret < 0 || len != 0) {
+        goto error;
+    }
+    seq_length = decode_ret;
+
+    if (qcrypto_der_decode_int(&seq, &seq_length, extract_mpi,
+                               &sig->r, errp) < 0 ||
+        qcrypto_der_decode_int(&seq, &seq_length, extract_mpi,
+                               &sig->s, errp) < 0) {
+        goto error;
+    }
+    if (seq_length != 0) {
+        goto error;
+    }
+
+    return sig;
+
+error:
+    if (errp && !*errp) {
+        error_setg(errp, "Invalid RSA public key");
+    }
+    qcrypto_akcipher_ecdsasig_free(sig);
+    return NULL;
+}
+
+/**
+ *   ECDSAPublicKey: compress-format | x coordinate | y coordinate
+ */
+static int __qcrypto_akcipher_builtin_ecdsa_pubkey_parse(
+    QCryptoAkCipherECDSAKey *ecdsa,
+    const uint8_t *key, size_t keylen, Error **errp)
+{
+    if (keylen < 3) {
+        error_setg(errp, "keylen is too short: %zu", keylen);
+        return -1;
+    }
+    if (key[0] != QCRYPTO_ECDSA_PUBKEY_FMT_UNCOMPRESSED) {
+        error_setg(errp, "Only uncompressed ECDSA public key is supported");
+        return -1;
+    }
+
+    /* Skip format byte */
+    key++;
+    keylen--;
+    if (keylen % 2 != 0) {
+        error_setg(errp, "ECDSA public key's length must be odd");
+        return -1;
+    }
+
+    ecdsa->pub_x.data = g_memdup2(key, keylen / 2);
+    ecdsa->pub_x.len = keylen / 2;
+    ecdsa->pub_y.data = g_memdup2(key + keylen / 2, keylen / 2);
+    ecdsa->pub_y.len = keylen / 2;
+
+    return 0;
+}
+
+static QCryptoAkCipherECDSAKey *qcrypto_akcipher_builtin_ecdsa_pubkey_parse(
+    const uint8_t *key, size_t keylen, Error **errp)
+{
+    QCryptoAkCipherECDSAKey *ecdsa = g_new0(QCryptoAkCipherECDSAKey, 1);
+    if (__qcrypto_akcipher_builtin_ecdsa_pubkey_parse(
+        ecdsa, key, keylen, errp) != 0) {
+        goto error;
+    }
+    return ecdsa;
+
+error:
+    qcrypto_akcipher_ecdsakey_free(ecdsa);
+    return NULL;
+}
+
+/**
+ *     ECDSAPrivateKey ::= SEQUENCE {
+ *          version         INTEGER
+ *             k            OCTET STRING
+ *          parameters [0]    OID         OPTIONAL
+ *          publickey  [1]  BIT STRING    OPTIONAL
+ *     }
+ */
+static QCryptoAkCipherECDSAKey *qcrypto_akcipher_builtin_ecdsa_privkey_parse(
+    const uint8_t *key, size_t keylen, Error **errp)
+{
+    QCryptoAkCipherECDSAKey *ecdsa = g_new0(QCryptoAkCipherECDSAKey, 1);
+    uint8_t version;
+    const uint8_t *seq, *pubkey;
+    int decode_ret;
+    size_t seq_length, pubkey_length;
+
+    decode_ret = qcrypto_der_decode_seq(&key, &keylen, extract_cons_content,
+                                        &seq, errp);
+    if (decode_ret < 0 || keylen != 0) {
+        goto error;
+    }
+    seq_length = decode_ret;
+
+    if (qcrypto_der_decode_int(&seq, &seq_length, extract_version,
+                               &version, errp) < 0 ||
+        qcrypto_der_decode_octet_str(&seq, &seq_length, extract_mpi,
+                                     &ecdsa->priv, errp) < 0) {
+        goto error;
+    }
+
+    /* Here we just ignore curve id */
+    qcrypto_der_decode_ctx_tag(&seq, &seq_length, 0, NULL, NULL, NULL);
+
+    decode_ret = qcrypto_der_decode_ctx_tag(&seq, &seq_length, 1,
+                                            extract_cons_content,
+                                            &pubkey, NULL);
+    if (decode_ret > 0) {
+        pubkey_length = decode_ret;
+        if (qcrypto_der_decode_bit_str(&pubkey, &pubkey_length,
+                                       extract_pubkey, ecdsa, errp) < 0 ||
+            pubkey_length != 0) {
+            goto error;
+        }
+    }
+
+    if (seq_length != 0) {
+        goto error;
+    }
+
+    return ecdsa;
+
+error:
+    if (errp && !*errp) {
+        error_setg(errp, "Failed to parse ecdsa private key");
+    }
+    qcrypto_akcipher_ecdsakey_free(ecdsa);
+    return NULL;
+}
+
+QCryptoAkCipherECDSAKey *qcrypto_akcipher_ecdsakey_parse(
+    QCryptoAkCipherKeyType type,
+    const uint8_t *key, size_t keylen, Error **errp)
+{
+    switch (type) {
+    case QCRYPTO_AKCIPHER_KEY_TYPE_PRIVATE:
+        return qcrypto_akcipher_builtin_ecdsa_privkey_parse(key, keylen, errp);
+
+    case QCRYPTO_AKCIPHER_KEY_TYPE_PUBLIC:
+        return qcrypto_akcipher_builtin_ecdsa_pubkey_parse(key, keylen, errp);
+
+    default:
+        error_setg(errp, "Unknown key type: %d", type);
+        return NULL;
+    }
+}
diff --git a/crypto/ecdsakey.c b/crypto/ecdsakey.c
new file mode 100644
index 0000000000..466dcffbc7
--- /dev/null
+++ b/crypto/ecdsakey.c
@@ -0,0 +1,118 @@
+/*
+ * QEMU Crypto ECDSA key parser
+ *
+ * Copyright (c) 2022 Bytedance
+ * Author: lei he <helei.sig11@bytedance.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "ecdsakey.h"
+#include "der.h"
+
+void qcrypto_akcipher_ecdsasig_free(QCryptoAkCipherECDSASig *sig)
+{
+    if (!sig) {
+        return;
+    }
+    g_free(sig->r.data);
+    g_free(sig->s.data);
+    g_free(sig);
+}
+
+void qcrypto_akcipher_ecdsasig_x9_62_encode(QCryptoAkCipherECDSASig *sig,
+    uint8_t *dst, size_t *dst_len)
+{
+    size_t r_len, s_len;
+    uint8_t *r_dst, *s_dst;
+    g_autofree uint8_t *buff = NULL;
+
+    qcrypto_der_encode_int(NULL, sig->r.len, NULL, &r_len);
+    qcrypto_der_encode_int(NULL, sig->s.len, NULL, &s_len);
+
+    buff = g_new0(uint8_t, r_len + s_len);
+    r_dst = buff;
+    qcrypto_der_encode_int(sig->r.data, sig->r.len, r_dst, &r_len);
+    s_dst = buff + r_len;
+    qcrypto_der_encode_int(sig->s.data, sig->s.len, s_dst, &s_len);
+
+    qcrypto_der_encode_seq(buff, r_len + s_len, dst, dst_len);
+}
+
+QCryptoAkCipherECDSASig *qcrypto_akcipher_ecdsasig_alloc(
+    QCryptoCurveID curve_id, Error **errp)
+{
+    int keylen;
+    QCryptoAkCipherECDSASig *sig;
+
+    switch (curve_id) {
+    case QCRYPTO_CURVE_ID_NIST_P192:
+        keylen = 192 / 8;
+        break;
+
+    case QCRYPTO_CURVE_ID_NIST_P256:
+        keylen = 256 / 8;
+        break;
+
+    case QCRYPTO_CURVE_ID_NIST_P384:
+        keylen = 384 / 8;
+        break;
+
+    default:
+        error_setg(errp, "Unknown curve id: %d", curve_id);
+        return NULL;
+    }
+
+    /*
+     * Note: when encoding positive bignum in tow'complement, we have to add
+     * a leading zero if the most significant byte is greater than or
+     * equal to 0x80.
+     */
+    sig = g_new0(QCryptoAkCipherECDSASig, 1);
+    sig->r.data = g_new0(uint8_t, keylen + 1);
+    sig->r.len = keylen + 1;
+    sig->s.data = g_new0(uint8_t, keylen + 1);
+    sig->s.len = keylen + 1;
+    return sig;
+}
+
+size_t qcrypto_akcipher_ecdsasig_x9_62_size(size_t keylen)
+{
+    size_t integer_len;
+    size_t seq_len;
+
+    /*
+     * Note: when encoding positive bignum in tow'complement, we have to add
+     * a leading zero if the most significant byte is greater than or
+     * equal to 0x80.
+     */
+    qcrypto_der_encode_int(NULL, keylen + 1, NULL, &integer_len);
+    qcrypto_der_encode_seq(NULL, integer_len * 2, NULL, &seq_len);
+    return seq_len;
+}
+
+void qcrypto_akcipher_ecdsakey_free(QCryptoAkCipherECDSAKey *ecdsa)
+{
+    if (!ecdsa) {
+        return;
+    }
+    g_free(ecdsa->priv.data);
+    g_free(ecdsa->pub_x.data);
+    g_free(ecdsa->pub_y.data);
+    g_free(ecdsa);
+}
+
+#include "ecdsakey-builtin.c.inc"
diff --git a/crypto/ecdsakey.h b/crypto/ecdsakey.h
new file mode 100644
index 0000000000..a0532a0e75
--- /dev/null
+++ b/crypto/ecdsakey.h
@@ -0,0 +1,66 @@
+/*
+ * QEMU Crypto ECDSA signature parser
+ *
+ * Copyright (c) 2022 Bytedance
+ * Author: lei he <helei.sig11@bytedance.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef QCRYPTO_ECDSASIG_H
+#define QCRYPTO_ECDSASIG_H
+
+#include "qemu/host-utils.h"
+#include "crypto/akcipher.h"
+#include "crypto/rsakey.h"
+
+typedef struct QCryptoAkCipherECDSAKey QCryptoAkCipherECDSAKey;
+typedef struct QCryptoAkCipherECDSASig QCryptoAkCipherECDSASig;
+
+struct QCryptoAkCipherECDSASig {
+    QCryptoAkCipherMPI r;
+    QCryptoAkCipherMPI s;
+};
+
+struct QCryptoAkCipherECDSAKey {
+    QCryptoAkCipherMPI priv;
+    QCryptoAkCipherMPI pub_x;
+    QCryptoAkCipherMPI pub_y;
+};
+
+QCryptoAkCipherECDSASig *qcrypto_akcipher_ecdsasig_parse(
+    const uint8_t *sig, size_t len, Error **errp);
+
+QCryptoAkCipherECDSASig *qcrypto_akcipher_ecdsasig_alloc(
+    QCryptoCurveID curve_id, Error **errp);
+
+void qcrypto_akcipher_ecdsasig_free(QCryptoAkCipherECDSASig *sig);
+
+void qcrypto_akcipher_ecdsasig_x9_62_encode(
+    QCryptoAkCipherECDSASig *sig, uint8_t *dst, size_t *dst_len);
+
+size_t qcrypto_akcipher_ecdsasig_x9_62_size(size_t keylen);
+
+QCryptoAkCipherECDSAKey *qcrypto_akcipher_ecdsakey_parse(
+    QCryptoAkCipherKeyType type,
+    const uint8_t *key, size_t keylen, Error **errp);
+
+void qcrypto_akcipher_ecdsakey_free(QCryptoAkCipherECDSAKey *key);
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoAkCipherECDSASig,
+                              qcrypto_akcipher_ecdsasig_free);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QCryptoAkCipherECDSAKey,
+                              qcrypto_akcipher_ecdsakey_free);
+#endif
diff --git a/crypto/meson.build b/crypto/meson.build
index 5f03a30d34..36e2e08938 100644
--- a/crypto/meson.build
+++ b/crypto/meson.build
@@ -7,6 +7,7 @@ crypto_ss.add(files(
   'block.c',
   'cipher.c',
   'der.c',
+  'ecdsakey.c',
   'hash.c',
   'hmac.c',
   'ivgen-essiv.c',
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 5/7] crypto: Implement ECDSA algorithm by hogweed
  2022-06-13  8:45 [PATCH 0/7] crypto: Introduce ECDSA algorithm Lei He
                   ` (3 preceding siblings ...)
  2022-06-13  8:45 ` [PATCH 4/7] crypto: Add ECDSA key parser Lei He
@ 2022-06-13  8:45 ` Lei He
  2022-06-17 11:35   ` Daniel P. Berrangé
  2022-06-13  8:45 ` [PATCH 6/7] crypto: Implement ECDSA algorithm by gcrypt Lei He
  2022-06-13  8:45 ` [PATCH 7/7] crypto: Add test suite for ECDSA algorithm Lei He
  6 siblings, 1 reply; 21+ messages in thread
From: Lei He @ 2022-06-13  8:45 UTC (permalink / raw)
  To: mst, arei.gonglei, berrange, qemu-devel; +Cc: helei.sig11, pizhenwei, jasowang

Implement ECDSA algorithm by hogweed and nettle.

Signed-off-by: lei he <helei.sig11@bytedance.com>
---
 crypto/akcipher-nettle.c.inc | 268 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 268 insertions(+)

diff --git a/crypto/akcipher-nettle.c.inc b/crypto/akcipher-nettle.c.inc
index 02699e6e6d..94d6e0b629 100644
--- a/crypto/akcipher-nettle.c.inc
+++ b/crypto/akcipher-nettle.c.inc
@@ -20,6 +20,8 @@
  */
 
 #include <nettle/rsa.h>
+#include <nettle/ecc-curve.h>
+#include <nettle/ecdsa.h>
 
 #include "qemu/osdep.h"
 #include "qemu/host-utils.h"
@@ -28,6 +30,7 @@
 #include "qapi/error.h"
 #include "sysemu/cryptodev.h"
 #include "rsakey.h"
+#include "ecdsakey.h"
 
 typedef struct QCryptoNettleRSA {
     QCryptoAkCipher akcipher;
@@ -37,6 +40,33 @@ typedef struct QCryptoNettleRSA {
     QCryptoHashAlgorithm hash_alg;
 } QCryptoNettleRSA;
 
+typedef struct QCryptoNettleECDSA {
+    QCryptoAkCipher akcipher;
+    QCryptoCurveID curve_id;
+    const struct ecc_curve *curve;
+    struct ecc_point pubkey;
+    struct ecc_scalar privkey;
+} QCryptoNettleECDSA;
+
+static int qcrypto_nettle_invalid_encrypt(QCryptoAkCipher *akcipher,
+                                          const void *data, size_t data_len,
+                                          void *enc, size_t enc_len,
+                                          Error **errp)
+{
+    error_setg(errp, "Invalid operation");
+    return -1;
+}
+
+
+static int qcrypto_nettle_invalid_decrypt(QCryptoAkCipher *akcipher,
+                                          const void *enc, size_t enc_len,
+                                          void *data, size_t data_len,
+                                          Error **errp)
+{
+    error_setg(errp, "Invalid operation");
+    return -1;
+}
+
 static void qcrypto_nettle_rsa_free(QCryptoAkCipher *akcipher)
 {
     QCryptoNettleRSA *rsa = (QCryptoNettleRSA *)akcipher;
@@ -55,6 +85,12 @@ static QCryptoAkCipher *qcrypto_nettle_rsa_new(
     const uint8_t *key,  size_t keylen,
     Error **errp);
 
+static QCryptoAkCipher *qcrypto_nettle_ecdsa_new(
+    const QCryptoAkCipherOptionsECDSA *opts,
+    QCryptoAkCipherKeyType type,
+    const uint8_t *key, size_t keylen,
+    Error **errp);
+
 QCryptoAkCipher *qcrypto_akcipher_new(const QCryptoAkCipherOptions *opts,
                                       QCryptoAkCipherKeyType type,
                                       const uint8_t *key, size_t keylen,
@@ -64,6 +100,10 @@ QCryptoAkCipher *qcrypto_akcipher_new(const QCryptoAkCipherOptions *opts,
     case QCRYPTO_AKCIPHER_ALG_RSA:
         return qcrypto_nettle_rsa_new(&opts->u.rsa, type, key, keylen, errp);
 
+    case QCRYPTO_AKCIPHER_ALG_ECDSA:
+        return qcrypto_nettle_ecdsa_new(&opts->u.ecdsa, type,
+                                        key, keylen, errp);
+
     default:
         error_setg(errp, "Unsupported algorithm: %u", opts->alg);
         return NULL;
@@ -421,6 +461,223 @@ error:
     return NULL;
 }
 
+static int qcrypto_nettle_parse_curve_id(
+    QCryptoNettleECDSA *ecdsa,
+    const QCryptoAkCipherOptionsECDSA *opts, Error **errp)
+{
+    /* ECDSA algorithm can't used for encryption */
+    ecdsa->akcipher.max_plaintext_len = 0;
+    ecdsa->akcipher.max_ciphertext_len = 0;
+
+    switch (opts->curve_id) {
+    case QCRYPTO_CURVE_ID_NIST_P192:
+        ecdsa->akcipher.max_signature_len =
+            qcrypto_akcipher_ecdsasig_x9_62_size(192 / 8);
+        ecdsa->akcipher.max_dgst_len = 192 / 8;
+        ecdsa->curve = nettle_get_secp_192r1();
+        break;
+
+    case QCRYPTO_CURVE_ID_NIST_P256:
+        ecdsa->akcipher.max_signature_len =
+            qcrypto_akcipher_ecdsasig_x9_62_size(256 / 8);
+        ecdsa->akcipher.max_dgst_len = 256 / 8;
+        ecdsa->curve = nettle_get_secp_256r1();
+        break;
+
+    case QCRYPTO_CURVE_ID_NIST_P384:
+        ecdsa->akcipher.max_signature_len =
+            qcrypto_akcipher_ecdsasig_x9_62_size(384 / 8);
+        ecdsa->akcipher.max_dgst_len = 256 / 8;
+        ecdsa->curve = nettle_get_secp_384r1();
+        break;
+
+    default:
+        error_setg(errp, "Unknown curve id: %d", opts->curve_id);
+        return -1;
+    }
+
+    return 0;
+}
+
+static void qcrypto_nettle_ecdsa_free(QCryptoAkCipher *akcipher)
+{
+    QCryptoNettleECDSA *ecdsa = (QCryptoNettleECDSA *)akcipher;
+    if (!ecdsa) {
+        return;
+    }
+    ecc_point_clear(&ecdsa->pubkey);
+    ecc_scalar_clear(&ecdsa->privkey);
+    g_free(ecdsa);
+}
+
+static int qcrypt_nettle_parse_ecdsa_private_key(
+    QCryptoNettleECDSA *ecdsa,
+    const uint8_t *key,
+    size_t keylen,
+    Error **errp)
+{
+    g_autoptr(QCryptoAkCipherECDSAKey) ecdsa_key =
+        qcrypto_akcipher_ecdsakey_parse(QCRYPTO_AKCIPHER_KEY_TYPE_PRIVATE,
+                                        key, keylen, errp);
+    mpz_t scalar;
+    int ret = -1;
+
+    if (!ecdsa_key) {
+        return ret;
+    }
+    nettle_mpz_init_set_str_256_u(
+        scalar, ecdsa_key->priv.len, ecdsa_key->priv.data);
+    if (ecc_scalar_set(&ecdsa->privkey, scalar) != 1) {
+        goto cleanup;
+    }
+    ret = 0;
+
+cleanup:
+    mpz_clear(scalar);
+    return ret;
+}
+
+static int qcrypt_nettle_parse_ecdsa_public_key(
+    QCryptoNettleECDSA *ecdsa,
+    const uint8_t *key,
+    size_t keylen,
+    Error **errp)
+{
+    g_autoptr(QCryptoAkCipherECDSAKey) ecdsa_key =
+        qcrypto_akcipher_ecdsakey_parse(QCRYPTO_AKCIPHER_KEY_TYPE_PUBLIC,
+                                        key, keylen, errp);
+    mpz_t x, y;
+    int ret = -1;
+
+    if (!ecdsa_key) {
+        return ret;
+    }
+    nettle_mpz_init_set_str_256_u(
+        x, ecdsa_key->pub_x.len, ecdsa_key->pub_x.data);
+    nettle_mpz_init_set_str_256_u(
+        y, ecdsa_key->pub_y.len, ecdsa_key->pub_y.data);
+    if (ecc_point_set(&ecdsa->pubkey, x, y) != 1) {
+        goto cleanup;
+    }
+    ret = 0;
+
+cleanup:
+    mpz_clear(x);
+    mpz_clear(y);
+    return ret;
+}
+
+static int qcrypto_nettle_ecdsa_sign(QCryptoAkCipher *akcipher,
+                                     const void *data, size_t data_len,
+                                     void *sig, size_t sig_len, Error **errp)
+{
+    QCryptoNettleECDSA *ecdsa = (QCryptoNettleECDSA *)akcipher;
+    int ret = -1;
+    size_t actual_len;
+    struct dsa_signature nettle_sig;
+    g_autoptr(QCryptoAkCipherECDSASig) qcrypto_sig = NULL;
+
+    if (sig_len < akcipher->max_signature_len) {
+        error_setg(errp, "Signature buffer should be not less than: %d",
+                   akcipher->max_signature_len);
+        return ret;
+    }
+    dsa_signature_init(&nettle_sig);
+    qcrypto_sig = qcrypto_akcipher_ecdsasig_alloc(ecdsa->curve_id, errp);
+    ecdsa_sign(&ecdsa->privkey, NULL, wrap_nettle_random_func,
+               data_len, data, &nettle_sig);
+    nettle_mpz_get_str_256(
+        qcrypto_sig->r.len, qcrypto_sig->r.data, nettle_sig.r);
+    nettle_mpz_get_str_256(
+        qcrypto_sig->s.len, qcrypto_sig->s.data, nettle_sig.s);
+    qcrypto_akcipher_ecdsasig_x9_62_encode(qcrypto_sig, sig, &actual_len);
+    ret = actual_len;
+
+    dsa_signature_clear(&nettle_sig);
+    return ret;
+}
+
+static int qcrypto_nettle_ecdsa_verify(QCryptoAkCipher *akcipher,
+                                       const void *sig, size_t sig_len,
+                                       const void *data, size_t data_len,
+                                       Error **errp)
+{
+    QCryptoNettleECDSA *ecdsa = (QCryptoNettleECDSA *)akcipher;
+    int ret = -1;
+    struct dsa_signature nettle_sig;
+    g_autoptr(QCryptoAkCipherECDSASig) qcrypto_sig = NULL;
+
+    qcrypto_sig = qcrypto_akcipher_ecdsasig_parse(sig, sig_len, errp);
+    if (!qcrypto_sig) {
+        return ret;
+    }
+    dsa_signature_init(&nettle_sig);
+    nettle_mpz_init_set_str_256_u(
+        nettle_sig.r, qcrypto_sig->r.len, qcrypto_sig->r.data);
+    nettle_mpz_init_set_str_256_u(
+        nettle_sig.s, qcrypto_sig->s.len, qcrypto_sig->s.data);
+    if (ecdsa_verify(&ecdsa->pubkey, data_len, data, &nettle_sig) == 1) {
+        ret = 0;
+    } else {
+        error_setg(errp, "Failed to verify signature");
+    }
+
+    dsa_signature_clear(&nettle_sig);
+    return ret;
+}
+
+
+
+static QCryptoAkCipherDriver nettle_ecdsa = {
+    .encrypt = qcrypto_nettle_invalid_encrypt,
+    .decrypt = qcrypto_nettle_invalid_decrypt,
+    .sign = qcrypto_nettle_ecdsa_sign,
+    .verify = qcrypto_nettle_ecdsa_verify,
+    .free = qcrypto_nettle_ecdsa_free,
+};
+
+static QCryptoAkCipher *qcrypto_nettle_ecdsa_new(
+    const QCryptoAkCipherOptionsECDSA *opts,
+    QCryptoAkCipherKeyType type,
+    const uint8_t *key, size_t keylen,
+    Error **errp)
+{
+    QCryptoNettleECDSA *ecdsa = g_new0(QCryptoNettleECDSA, 1);
+    if (qcrypto_nettle_parse_curve_id(ecdsa, opts, errp) != 0) {
+        goto error;
+    }
+
+    ecdsa->akcipher.driver = &nettle_ecdsa;
+    ecdsa->curve_id = opts->curve_id;
+    ecc_scalar_init(&ecdsa->privkey, ecdsa->curve);
+    ecc_point_init(&ecdsa->pubkey, ecdsa->curve);
+
+    switch (type) {
+    case QCRYPTO_AKCIPHER_KEY_TYPE_PRIVATE:
+        if (qcrypt_nettle_parse_ecdsa_private_key(
+            ecdsa, key, keylen, errp) != 0) {
+            goto error;
+        }
+        break;
+
+    case QCRYPTO_AKCIPHER_KEY_TYPE_PUBLIC:
+        if (qcrypt_nettle_parse_ecdsa_public_key(
+            ecdsa, key, keylen, errp) != 0) {
+            goto error;
+        }
+        break;
+
+    default:
+        error_setg(errp, "Unknown akcipher key type %d", type);
+        goto error;
+    }
+
+    return (QCryptoAkCipher *)ecdsa;
+
+error:
+    qcrypto_nettle_ecdsa_free((QCryptoAkCipher *)ecdsa);
+    return NULL;
+}
 
 bool qcrypto_akcipher_supports(QCryptoAkCipherOptions *opts)
 {
@@ -445,6 +702,17 @@ bool qcrypto_akcipher_supports(QCryptoAkCipherOptions *opts)
         }
         break;
 
+    case QCRYPTO_AKCIPHER_ALG_ECDSA:
+        switch (opts->u.ecdsa.curve_id) {
+        case QCRYPTO_CURVE_ID_NIST_P192:
+        case QCRYPTO_CURVE_ID_NIST_P256:
+        case QCRYPTO_CURVE_ID_NIST_P384:
+            return true;
+
+        default:
+            return false;
+        }
+
     default:
         return false;
     }
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 6/7] crypto: Implement ECDSA algorithm by gcrypt
  2022-06-13  8:45 [PATCH 0/7] crypto: Introduce ECDSA algorithm Lei He
                   ` (4 preceding siblings ...)
  2022-06-13  8:45 ` [PATCH 5/7] crypto: Implement ECDSA algorithm by hogweed Lei He
@ 2022-06-13  8:45 ` Lei He
  2022-06-17 13:41   ` Daniel P. Berrangé
  2022-06-13  8:45 ` [PATCH 7/7] crypto: Add test suite for ECDSA algorithm Lei He
  6 siblings, 1 reply; 21+ messages in thread
From: Lei He @ 2022-06-13  8:45 UTC (permalink / raw)
  To: mst, arei.gonglei, berrange, qemu-devel; +Cc: helei.sig11, pizhenwei, jasowang

Implement ECDSA algorithm by gcrypt

Signed-off-by: lei he <helei.sig11@bytedance.com>
---
 crypto/akcipher-gcrypt.c.inc | 400 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 400 insertions(+)

diff --git a/crypto/akcipher-gcrypt.c.inc b/crypto/akcipher-gcrypt.c.inc
index abb1fb272e..24377bbec6 100644
--- a/crypto/akcipher-gcrypt.c.inc
+++ b/crypto/akcipher-gcrypt.c.inc
@@ -28,6 +28,7 @@
 #include "qapi/error.h"
 #include "sysemu/cryptodev.h"
 #include "rsakey.h"
+#include "ecdsakey.h"
 
 typedef struct QCryptoGcryptRSA {
     QCryptoAkCipher akcipher;
@@ -36,6 +37,13 @@ typedef struct QCryptoGcryptRSA {
     QCryptoHashAlgorithm hash_alg;
 } QCryptoGcryptRSA;
 
+typedef struct QCryptoGcryptECDSA {
+    QCryptoAkCipher akcipher;
+    gcry_sexp_t key;
+    QCryptoCurveID curve_id;
+    const char *curve_name;
+} QCryptoGcryptECDSA;
+
 static void qcrypto_gcrypt_rsa_free(QCryptoAkCipher *akcipher)
 {
     QCryptoGcryptRSA *rsa = (QCryptoGcryptRSA *)akcipher;
@@ -53,6 +61,12 @@ static QCryptoGcryptRSA *qcrypto_gcrypt_rsa_new(
     const uint8_t *key,  size_t keylen,
     Error **errp);
 
+static QCryptoGcryptECDSA *qcrypto_gcrypt_ecdsa_new(
+    const QCryptoAkCipherOptionsECDSA *opts,
+    QCryptoAkCipherKeyType type,
+    const uint8_t *key, size_t keylen,
+    Error **errp);
+
 QCryptoAkCipher *qcrypto_akcipher_new(const QCryptoAkCipherOptions *opts,
                                       QCryptoAkCipherKeyType type,
                                       const uint8_t *key, size_t keylen,
@@ -63,6 +77,10 @@ QCryptoAkCipher *qcrypto_akcipher_new(const QCryptoAkCipherOptions *opts,
         return (QCryptoAkCipher *)qcrypto_gcrypt_rsa_new(
             &opts->u.rsa, type, key, keylen, errp);
 
+    case QCRYPTO_AKCIPHER_ALG_ECDSA:
+        return (QCryptoAkCipher *)qcrypto_gcrypt_ecdsa_new(
+            &opts->u.ecdsa, type, key, keylen, errp);
+
     default:
         error_setg(errp, "Unsupported algorithm: %u", opts->alg);
         return NULL;
@@ -564,6 +582,377 @@ error:
     return NULL;
 }
 
+static int qcrypto_gcrypt_parse_curve_id(QCryptoGcryptECDSA *ecdsa,
+    const QCryptoAkCipherOptionsECDSA *opts, Error **errp)
+{
+    /* ECDSA algorithm can't used for encryption */
+    ecdsa->akcipher.max_plaintext_len = 0;
+    ecdsa->akcipher.max_ciphertext_len = 0;
+
+    switch (opts->curve_id) {
+    case QCRYPTO_CURVE_ID_NIST_P192:
+        ecdsa->curve_name = "nistp192";
+        ecdsa->akcipher.max_signature_len =
+            qcrypto_akcipher_ecdsasig_x9_62_size(192 / 8);
+        ecdsa->akcipher.max_dgst_len = 192 / 8;
+        break;
+
+    case QCRYPTO_CURVE_ID_NIST_P256:
+        ecdsa->curve_name = "nistp256";
+        ecdsa->akcipher.max_signature_len =
+            qcrypto_akcipher_ecdsasig_x9_62_size(256 / 8);
+        ecdsa->akcipher.max_dgst_len = 256 / 8;
+        break;
+
+    case QCRYPTO_CURVE_ID_NIST_P384:
+        ecdsa->curve_name = "nistp384";
+        ecdsa->akcipher.max_signature_len =
+            qcrypto_akcipher_ecdsasig_x9_62_size(384 / 8);
+        ecdsa->akcipher.max_dgst_len = 256 / 8;
+        break;
+
+    default:
+        error_setg(errp, "Unknown curve id: %d", opts->curve_id);
+        return -1;
+    }
+
+    return 0;
+}
+
+static int qcrypto_gcrypt_parse_ecdsa_private_key(
+    QCryptoGcryptECDSA *ecdsa, const char *curve_name,
+    const uint8_t *key, size_t keylen,
+    Error **errp)
+{
+    g_autoptr(QCryptoAkCipherECDSAKey) ecdsa_key =
+        qcrypto_akcipher_ecdsakey_parse(QCRYPTO_AKCIPHER_KEY_TYPE_PRIVATE,
+                                        key, keylen, errp);
+    gcry_mpi_t d = NULL;
+    gcry_error_t err;
+    int ret = -1;
+
+    if (!ecdsa_key) {
+        return ret;
+    }
+
+    err = gcry_mpi_scan(&d, GCRYMPI_FMT_USG, ecdsa_key->priv.data,
+                        ecdsa_key->priv.len, NULL);
+    if (gcry_err_code(err) != 0) {
+        error_setg(errp, "Failed to parse ECDSA parivate key: %s/%s",
+                   gcry_strsource(err), gcry_strerror(err));
+        return ret;
+    }
+
+    err = gcry_sexp_build(&ecdsa->key, NULL,
+        "(private-key (ecc (curve %s) (d %m)))", curve_name, d);
+    if (gcry_err_code(err) != 0) {
+        error_setg(errp, "Failed to build ECDSA parivate key: %s/%s",
+                   gcry_strsource(err), gcry_strerror(err));
+        goto cleanup;
+    }
+
+    ret = 0;
+
+cleanup:
+    gcry_mpi_release(d);
+    return ret;
+}
+
+static int qcrypto_gcrypt_parse_ecdsa_public_key(
+    QCryptoGcryptECDSA *ecdsa, const char *curve_name,
+    const uint8_t *key, size_t keylen,
+    Error **errp)
+{
+    gcry_mpi_t q = NULL;
+    gcry_error_t err;
+    int ret = -1;
+
+    err = gcry_mpi_scan(&q, GCRYMPI_FMT_USG, key, keylen, NULL);
+    if (gcry_err_code(err) != 0) {
+        error_setg(errp, "Failed to scan public point: %s/%s",
+                   gcry_strsource(err), gcry_strerror(err));
+        return -1;
+    }
+
+    err = gcry_sexp_build(&ecdsa->key, NULL,
+        "(public-key (ecc (curve %s) (q %m)))", curve_name, q);
+    if (gcry_err_code(err) != 0) {
+        error_setg(errp, "Failed to build ECDSA public key: %s/%s",
+                   gcry_strsource(err), gcry_strerror(err));
+        goto cleanup;
+    }
+    ret = 0;
+
+cleanup:
+    gcry_mpi_release(q);
+    return ret;
+}
+
+static void qcrypto_gcrypt_ecdsa_free(QCryptoAkCipher *akcipher)
+{
+    QCryptoGcryptECDSA *ecdsa = (QCryptoGcryptECDSA *)akcipher;
+    if (!ecdsa) {
+        return;
+    }
+    gcry_sexp_release(ecdsa->key);
+    g_free(ecdsa);
+}
+
+static int qcrypto_gcrypt_invalid_encrypt(QCryptoAkCipher *akcipher,
+                                          const void *in, size_t in_len,
+                                          void *out, size_t out_len,
+                                          Error **errp)
+{
+    error_setg(errp, "Operation is invalid");
+    return -1;
+}
+
+static int qcrypto_gcrypt_invalid_decrypt(QCryptoAkCipher *akcipher,
+                                          const void *in, size_t in_len,
+                                          void *out, size_t out_len,
+                                          Error **errp)
+{
+    error_setg(errp, "Operation is invalid");
+    return -1;
+}
+
+static int qcrypto_gcrypt_ecdsa_sign(QCryptoAkCipher *akcipher,
+                                     const void *in, size_t in_len,
+                                     void *out, size_t out_len, Error **errp)
+{
+    QCryptoGcryptECDSA *ecdsa = (QCryptoGcryptECDSA *)akcipher;
+    int ret = -1;
+    gcry_mpi_t data = NULL, r_mpi = NULL, s_mpi = NULL;
+    gcry_sexp_t dgst_sexp = NULL, sig_sexp = NULL;
+    gcry_sexp_t r_sexp_item = NULL, s_sexp_item = NULL;
+    size_t actual_len;
+    gcry_error_t err;
+    g_autoptr(QCryptoAkCipherECDSASig) sig = NULL;
+
+    if (out_len < akcipher->max_signature_len) {
+        error_setg(errp, "Signature buffer should be not less than: %d",
+                   akcipher->max_signature_len);
+        return -1;
+    }
+    /*
+     * For ecdsa, digest length less than key length is recommended but not
+     * required, truncation occurs when digest is too long, see FIPS 186-4:
+     * https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-4.pdf.
+     * Here we don't do the check, gcrypt will handle it.
+     */
+    err = gcry_mpi_scan(&data, GCRYMPI_FMT_USG, in, in_len, NULL);
+    if (gcry_err_code(err) != 0) {
+        error_setg(errp, "Failed to build data: %s/%s",
+                   gcry_strsource(err), gcry_strerror(err));
+        goto cleanup;
+    }
+
+    err = gcry_sexp_build(&dgst_sexp, NULL,
+                          "(data (flags raw) (value %m))", data);
+    if (gcry_err_code(err) != 0) {
+        error_setg(errp, "Failed to build dgst signature: %s/%s",
+                   gcry_strsource(err), gcry_strerror(err));
+        goto cleanup;
+    }
+
+    err = gcry_pk_sign(&sig_sexp, dgst_sexp, ecdsa->key);
+    if (gcry_err_code(err) != 0) {
+        error_setg(errp, "Failed to make signature: %s/%s",
+                   gcry_strsource(err), gcry_strerror(err));
+        goto cleanup;
+    }
+
+    sig = qcrypto_akcipher_ecdsasig_alloc(ecdsa->curve_id, errp);
+    if (!sig) {
+        goto cleanup;
+    }
+
+    /* S-expression of signature: (sig-val (ecdsa (r r-mpi) (s s-mpi))) */
+    r_sexp_item = gcry_sexp_find_token(sig_sexp, "r", 0);
+    if (!r_sexp_item || gcry_sexp_length(r_sexp_item) != 2) {
+        error_setg(errp, "Invalid signature result");
+        goto cleanup;
+    }
+    r_mpi = gcry_sexp_nth_mpi(r_sexp_item, 1, GCRYMPI_FMT_USG);
+    if (!r_mpi) {
+        error_setg(errp, "Invalid signature result");
+    }
+    err = gcry_mpi_print(GCRYMPI_FMT_STD, sig->r.data, sig->r.len,
+                         &actual_len, r_mpi);
+    if (gcry_err_code(err) != 0) {
+        error_setg(errp, "Failed to print MPI: %s/%s",
+                   gcry_strsource(err), gcry_strerror(err));
+        goto cleanup;
+    }
+    if (unlikely(actual_len > sig->r.len)) {
+        error_setg(errp, "Internal error: signature buffer is too small");
+        goto cleanup;
+    }
+    sig->r.len = actual_len;
+
+    s_sexp_item = gcry_sexp_find_token(sig_sexp, "s", 0);
+    if (!s_sexp_item || gcry_sexp_length(s_sexp_item) != 2) {
+        error_setg(errp, "Invalid signature result");
+        goto cleanup;
+    }
+    s_mpi = gcry_sexp_nth_mpi(s_sexp_item, 1, GCRYMPI_FMT_USG);
+    if (!s_mpi) {
+        error_setg(errp, "Invalid signature result");
+    }
+    err = gcry_mpi_print(GCRYMPI_FMT_STD, sig->s.data, sig->s.len,
+                         &actual_len, s_mpi);
+    if (gcry_err_code(err) != 0) {
+        error_setg(errp, "Failed to print MPI: %s/%s",
+                   gcry_strsource(err), gcry_strerror(err));
+        goto cleanup;
+    }
+    if (unlikely(actual_len > sig->s.len)) {
+        error_setg(errp, "Internal error: signature buffer is too small");
+        goto cleanup;
+    }
+    sig->s.len = actual_len;
+
+    qcrypto_akcipher_ecdsasig_x9_62_encode(sig, out, &out_len);
+    ret = out_len;
+
+cleanup:
+    gcry_mpi_release(data);
+    gcry_mpi_release(r_mpi);
+    gcry_mpi_release(s_mpi);
+    gcry_sexp_release(dgst_sexp);
+    gcry_sexp_release(sig_sexp);
+    gcry_sexp_release(r_sexp_item);
+
+    return ret;
+}
+
+static int qcrypto_gcrypt_ecdsa_verify(QCryptoAkCipher *akcipher,
+                                       const void *in, size_t in_len,
+                                       const void *in2, size_t in2_len,
+                                       Error **errp)
+{
+    QCryptoGcryptECDSA *ecdsa = (QCryptoGcryptECDSA *)akcipher;
+    int ret = -1;
+    QCryptoAkCipherECDSASig *sig;
+    gcry_mpi_t sig_s = NULL, sig_r = NULL, dgst_mpi = NULL;
+    gcry_sexp_t sig_sexp = NULL, dgst_sexp = NULL;
+    gcry_error_t err;
+
+    /*
+     * We only check the signature length, dgst length will be handled
+     * by gcrypt, see qcrypto_gcrypt_ecdsa_sign.
+     */
+    if (in_len > akcipher->max_signature_len) {
+        error_setg(errp, "Signature length is greater than %d",
+                   akcipher->max_signature_len);
+        return ret;
+    }
+
+    sig = qcrypto_akcipher_ecdsasig_parse(in, in_len, errp);
+    if (!sig) {
+        return ret;
+    }
+
+    err = gcry_mpi_scan(&sig_r, GCRYMPI_FMT_STD, sig->r.data, sig->r.len, NULL);
+    if (gcry_err_code(err) != 0) {
+        error_setg(errp, "Failed to parse ECDSA signature: %s/%s",
+                   gcry_strsource(err), gcry_strerror(err));
+        goto cleanup;
+    }
+    err = gcry_mpi_scan(&sig_s, GCRYMPI_FMT_STD, sig->s.data, sig->s.len, NULL);
+    if (gcry_err_code(err) != 0) {
+        error_setg(errp, "Failed to parse ECDSA signature: %s/%s",
+                   gcry_strsource(err), gcry_strerror(err));
+        goto cleanup;
+    }
+    err = gcry_sexp_build(&sig_sexp, NULL,
+                          "(sig-val (ecdsa (r %m) (s %m)))", sig_r, sig_s);
+    if (gcry_err_code(err) != 0) {
+        error_setg(errp, "Failed to build signature: %s/%s",
+                   gcry_strsource(err), gcry_strerror(err));
+        goto cleanup;
+    }
+
+    err = gcry_mpi_scan(&dgst_mpi, GCRYMPI_FMT_USG, in2, in2_len, NULL);
+    if (gcry_err_code(err) != 0) {
+        error_setg(errp, "Failed to parse scan mpi: %s/%s",
+                   gcry_strsource(err), gcry_strerror(err));
+        goto cleanup;
+    }
+    err = gcry_sexp_build(&dgst_sexp, NULL,
+                          "(data (flags raw) (value %m))", dgst_mpi);
+    if (gcry_err_code(err) != 0) {
+        error_setg(errp, "Failed to build dgst: %s/%s",
+                   gcry_strsource(err), gcry_strerror(err));
+        goto cleanup;
+    }
+
+    err = gcry_pk_verify(sig_sexp, dgst_sexp, ecdsa->key);
+    if (gcry_err_code(err) != 0) {
+        error_setg(errp, "Failed to verify signature: %s/%s",
+                   gcry_strsource(err), gcry_strerror(err));
+        goto cleanup;
+    }
+    ret = 0;
+
+cleanup:
+    gcry_mpi_release(sig_s);
+    gcry_mpi_release(sig_r);
+    gcry_mpi_release(dgst_mpi);
+    gcry_sexp_release(dgst_sexp);
+    gcry_sexp_release(sig_sexp);
+    qcrypto_akcipher_ecdsasig_free(sig);
+
+    return ret;
+}
+
+static QCryptoAkCipherDriver gcrypt_ecdsa = {
+    .encrypt = qcrypto_gcrypt_invalid_encrypt,
+    .decrypt = qcrypto_gcrypt_invalid_decrypt,
+    .sign = qcrypto_gcrypt_ecdsa_sign,
+    .verify = qcrypto_gcrypt_ecdsa_verify,
+    .free = qcrypto_gcrypt_ecdsa_free,
+};
+
+static QCryptoGcryptECDSA *qcrypto_gcrypt_ecdsa_new(
+    const QCryptoAkCipherOptionsECDSA *opts,
+    QCryptoAkCipherKeyType type,
+    const uint8_t *key, size_t keylen,
+    Error **errp)
+{
+    QCryptoGcryptECDSA *ecdsa = g_new0(QCryptoGcryptECDSA, 1);
+    if (qcrypto_gcrypt_parse_curve_id(ecdsa, opts, errp) != 0) {
+        goto error;
+    }
+    ecdsa->curve_id = opts->curve_id;
+    ecdsa->akcipher.driver = &gcrypt_ecdsa;
+
+    switch (type) {
+    case QCRYPTO_AKCIPHER_KEY_TYPE_PRIVATE:
+        if (qcrypto_gcrypt_parse_ecdsa_private_key(
+            ecdsa, ecdsa->curve_name, key, keylen, errp) != 0) {
+            goto error;
+        }
+        break;
+
+    case QCRYPTO_AKCIPHER_KEY_TYPE_PUBLIC:
+        if (qcrypto_gcrypt_parse_ecdsa_public_key(
+            ecdsa, ecdsa->curve_name, key, keylen, errp) != 0) {
+            goto error;
+        }
+        break;
+
+    default:
+        error_setg(errp, "Unknown akcipher key type %d", type);
+        goto error;
+    }
+
+    return ecdsa;
+
+error:
+    qcrypto_gcrypt_ecdsa_free((QCryptoAkCipher *)ecdsa);
+    return NULL;
+}
 
 bool qcrypto_akcipher_supports(QCryptoAkCipherOptions *opts)
 {
@@ -589,6 +978,17 @@ bool qcrypto_akcipher_supports(QCryptoAkCipherOptions *opts)
             return false;
         }
 
+    case QCRYPTO_AKCIPHER_ALG_ECDSA:
+        switch (opts->u.ecdsa.curve_id) {
+        case QCRYPTO_CURVE_ID_NIST_P192:
+        case QCRYPTO_CURVE_ID_NIST_P256:
+        case QCRYPTO_CURVE_ID_NIST_P384:
+            return true;
+
+        default:
+            return false;
+        }
+
     default:
         return true;
     }
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 7/7] crypto: Add test suite for ECDSA algorithm
  2022-06-13  8:45 [PATCH 0/7] crypto: Introduce ECDSA algorithm Lei He
                   ` (5 preceding siblings ...)
  2022-06-13  8:45 ` [PATCH 6/7] crypto: Implement ECDSA algorithm by gcrypt Lei He
@ 2022-06-13  8:45 ` Lei He
  2022-06-17 13:42   ` Daniel P. Berrangé
  6 siblings, 1 reply; 21+ messages in thread
From: Lei He @ 2022-06-13  8:45 UTC (permalink / raw)
  To: mst, arei.gonglei, berrange, qemu-devel; +Cc: helei.sig11, pizhenwei, jasowang

1. add test suite for ecdsa algorithm.
2. use qcrypto_akcihper_max_xxx_len to help create buffers in
test code.

Signed-off-by: lei he <helei.sig11@bytedance.com>
---
 tests/unit/test-crypto-akcipher.c | 227 +++++++++++++++++++++++++++++++++++---
 1 file changed, 212 insertions(+), 15 deletions(-)

diff --git a/tests/unit/test-crypto-akcipher.c b/tests/unit/test-crypto-akcipher.c
index 4f1f4214dd..7b0f5ac4d4 100644
--- a/tests/unit/test-crypto-akcipher.c
+++ b/tests/unit/test-crypto-akcipher.c
@@ -314,6 +314,100 @@ static const uint8_t rsa2048_public_key[] = {
     0xed, 0x02, 0x03, 0x01, 0x00, 0x01
 };
 
+static const uint8_t ecdsa_p192_public_key[] = {
+    0x04, 0xc4, 0x16, 0xb3, 0xff, 0xac, 0xd5, 0x87,
+    0x98, 0xf7, 0xd9, 0x45, 0xfe, 0xd3, 0x5c, 0x17,
+    0x9d, 0xb2, 0x36, 0x22, 0xcc, 0x07, 0xb3, 0x6d,
+    0x3c, 0x4e, 0x04, 0x5f, 0xeb, 0xb6, 0x52, 0x58,
+    0xfb, 0x36, 0x10, 0x52, 0xb7, 0x01, 0x62, 0x0e,
+    0x94, 0x51, 0x1d, 0xe2, 0xef, 0x10, 0x82, 0x88,
+    0x78,
+};
+
+static const uint8_t ecdsa_p192_private_key[] = {
+    0x30, 0x53, 0x02, 0x01, 0x01, 0x04, 0x18, 0xcb,
+    0xc8, 0x86, 0x0e, 0x66, 0x3c, 0xf7, 0x5a, 0x44,
+    0x13, 0xb8, 0xef, 0xea, 0x1d, 0x7b, 0xa6, 0x1c,
+    0xda, 0xf4, 0x1b, 0xc7, 0x67, 0x6b, 0x35, 0xa1,
+    0x34, 0x03, 0x32, 0x00, 0x04, 0xc4, 0x16, 0xb3,
+    0xff, 0xac, 0xd5, 0x87, 0x98, 0xf7, 0xd9, 0x45,
+    0xfe, 0xd3, 0x5c, 0x17, 0x9d, 0xb2, 0x36, 0x22,
+    0xcc, 0x07, 0xb3, 0x6d, 0x3c, 0x4e, 0x04, 0x5f,
+    0xeb, 0xb6, 0x52, 0x58, 0xfb, 0x36, 0x10, 0x52,
+    0xb7, 0x01, 0x62, 0x0e, 0x94, 0x51, 0x1d, 0xe2,
+    0xef, 0x10, 0x82, 0x88, 0x78,
+};
+
+static const uint8_t ecdsa_p256_private_key[] = {
+    0x30, 0x77, 0x02, 0x01, 0x01, 0x04, 0x20, 0xf6,
+    0x92, 0xdd, 0x29, 0x1c, 0x6e, 0xef, 0xb6, 0xb2,
+    0x73, 0x9f, 0x40, 0x1b, 0xb3, 0x2a, 0x28, 0xd2,
+    0x37, 0xd6, 0x4a, 0x5b, 0xe4, 0x40, 0x4c, 0x6a,
+    0x95, 0x99, 0xfa, 0xf7, 0x92, 0x49, 0xbe, 0xa0,
+    0x0a, 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d,
+    0x03, 0x01, 0x07, 0xa1, 0x44, 0x03, 0x42, 0x00,
+    0x04, 0xed, 0x42, 0x9c, 0x67, 0x79, 0xbe, 0x46,
+    0x83, 0x88, 0x3e, 0x8c, 0xc1, 0x33, 0xf3, 0xc3,
+    0xf6, 0x2c, 0xf3, 0x13, 0x6a, 0x00, 0xc2, 0xc9,
+    0x3e, 0x87, 0x7f, 0x86, 0x39, 0xe6, 0xae, 0xe3,
+    0xb9, 0xba, 0x2f, 0x58, 0x63, 0x32, 0x62, 0x62,
+    0x54, 0x07, 0x27, 0xf9, 0x5a, 0x3a, 0xc7, 0x3a,
+    0x6b, 0x5b, 0xbc, 0x0d, 0x33, 0xba, 0xbb, 0xd4,
+    0xa3, 0xff, 0x4f, 0x9e, 0xdd, 0xf5, 0x59, 0xc0,
+    0xf6,
+};
+
+static const uint8_t ecdsa_p256_public_key[] = {
+    0x04, 0xed, 0x42, 0x9c, 0x67, 0x79, 0xbe, 0x46,
+    0x83, 0x88, 0x3e, 0x8c, 0xc1, 0x33, 0xf3, 0xc3,
+    0xf6, 0x2c, 0xf3, 0x13, 0x6a, 0x00, 0xc2, 0xc9,
+    0x3e, 0x87, 0x7f, 0x86, 0x39, 0xe6, 0xae, 0xe3,
+    0xb9, 0xba, 0x2f, 0x58, 0x63, 0x32, 0x62, 0x62,
+    0x54, 0x07, 0x27, 0xf9, 0x5a, 0x3a, 0xc7, 0x3a,
+    0x6b, 0x5b, 0xbc, 0x0d, 0x33, 0xba, 0xbb, 0xd4,
+    0xa3, 0xff, 0x4f, 0x9e, 0xdd, 0xf5, 0x59, 0xc0,
+    0xf6,
+};
+
+static const uint8_t ecdsa_p384_public_key[] = {
+    0x04, 0xab, 0xd5, 0xf8, 0x87, 0x1d, 0x23, 0x9b,
+    0x26, 0xb9, 0x57, 0x7e, 0x97, 0x78, 0x10, 0xcd,
+    0x13, 0xe3, 0x98, 0x25, 0xa8, 0xd6, 0xab, 0x66,
+    0x35, 0x26, 0x68, 0x8a, 0x0e, 0x49, 0xd9, 0x4a,
+    0x91, 0x7d, 0x6c, 0x94, 0x06, 0x06, 0x99, 0xf1,
+    0x8d, 0x2a, 0x25, 0x8d, 0xf9, 0xbf, 0x40, 0xfa,
+    0xb7, 0xcb, 0xe1, 0x14, 0x22, 0x0a, 0xa7, 0xfb,
+    0x0a, 0xb4, 0x02, 0x05, 0x8b, 0x98, 0xaa, 0x78,
+    0xcd, 0x53, 0x00, 0x1e, 0xd1, 0x79, 0x6a, 0x5f,
+    0x09, 0x01, 0x88, 0xb4, 0xbc, 0x32, 0x62, 0x83,
+    0x92, 0x84, 0x2d, 0xc6, 0xf8, 0xda, 0xc4, 0x7f,
+    0x10, 0xa3, 0x18, 0x1d, 0xae, 0x0d, 0xa4, 0x41,
+    0x9f,
+};
+
+static const uint8_t ecdsa_p384_private_key[] = {
+    0x30, 0x81, 0x9b, 0x02, 0x01, 0x01, 0x04, 0x30,
+    0xb6, 0x04, 0xef, 0xb1, 0x2c, 0x98, 0xdf, 0xcf,
+    0xd4, 0x16, 0x31, 0xd4, 0x69, 0x0c, 0x27, 0x81,
+    0x4a, 0xac, 0x1a, 0x83, 0x3c, 0xe4, 0xef, 0x65,
+    0xe1, 0x7a, 0x6a, 0xc6, 0xd6, 0xf7, 0xea, 0x79,
+    0xbe, 0xf1, 0x00, 0x3c, 0xdf, 0x6e, 0x9d, 0x10,
+    0x22, 0x61, 0x1b, 0x11, 0xcf, 0x49, 0x6e, 0x62,
+    0xa1, 0x64, 0x03, 0x62, 0x00, 0x04, 0xab, 0xd5,
+    0xf8, 0x87, 0x1d, 0x23, 0x9b, 0x26, 0xb9, 0x57,
+    0x7e, 0x97, 0x78, 0x10, 0xcd, 0x13, 0xe3, 0x98,
+    0x25, 0xa8, 0xd6, 0xab, 0x66, 0x35, 0x26, 0x68,
+    0x8a, 0x0e, 0x49, 0xd9, 0x4a, 0x91, 0x7d, 0x6c,
+    0x94, 0x06, 0x06, 0x99, 0xf1, 0x8d, 0x2a, 0x25,
+    0x8d, 0xf9, 0xbf, 0x40, 0xfa, 0xb7, 0xcb, 0xe1,
+    0x14, 0x22, 0x0a, 0xa7, 0xfb, 0x0a, 0xb4, 0x02,
+    0x05, 0x8b, 0x98, 0xaa, 0x78, 0xcd, 0x53, 0x00,
+    0x1e, 0xd1, 0x79, 0x6a, 0x5f, 0x09, 0x01, 0x88,
+    0xb4, 0xbc, 0x32, 0x62, 0x83, 0x92, 0x84, 0x2d,
+    0xc6, 0xf8, 0xda, 0xc4, 0x7f, 0x10, 0xa3, 0x18,
+    0x1d, 0xae, 0x0d, 0xa4, 0x41, 0x9f,
+};
+
 static const uint8_t test_sha1_dgst[] = {
     0x3c, 0x05, 0x19, 0x34, 0x29, 0x19, 0xc7, 0xe0,
     0x87, 0xb6, 0x24, 0xf9, 0x58, 0xac, 0xa4, 0xd4,
@@ -374,6 +468,45 @@ static const uint8_t exp_signature_rsa1024_pkcs1[] = {
     0xab, 0x0d, 0xc6, 0x59, 0x1d, 0xc7, 0x33, 0x7b,
 };
 
+static const uint8_t exp_signature_ecdsa_p192[] = {
+    0x30, 0x35, 0x02, 0x19, 0x00, 0xba, 0xf7, 0xc0,
+    0xc1, 0x7e, 0xf5, 0x69, 0xd5, 0xb7, 0x5d, 0x06,
+    0xcb, 0x92, 0x28, 0x57, 0x52, 0x96, 0x9a, 0xdc,
+    0xc9, 0xf9, 0xd5, 0x2c, 0x51, 0x02, 0x18, 0x26,
+    0x21, 0x5d, 0x16, 0xba, 0xff, 0x19, 0x74, 0x56,
+    0x8e, 0xdf, 0x51, 0x2b, 0x2c, 0xce, 0xc2, 0x7b,
+    0x5b, 0x03, 0x10, 0x56, 0x57, 0x63, 0x47,
+};
+
+static const uint8_t exp_signature_ecdsa_p256[] = {
+    0x30, 0x45, 0x02, 0x21, 0x00, 0xac, 0x09, 0xf3,
+    0x32, 0xb6, 0xf6, 0x7e, 0x12, 0x4f, 0x68, 0xdb,
+    0x10, 0x14, 0x61, 0xf6, 0x29, 0xbd, 0xdd, 0x72,
+    0x9f, 0x81, 0xf8, 0x83, 0x8a, 0xf3, 0x29, 0x87,
+    0x7b, 0xbb, 0xcf, 0xea, 0x64, 0x02, 0x20, 0x14,
+    0xfc, 0x2e, 0x2f, 0x3e, 0x06, 0xb1, 0xd0, 0xbb,
+    0x91, 0x44, 0xd5, 0x53, 0xb4, 0x72, 0xa1, 0x83,
+    0xc7, 0x3a, 0xa8, 0xfc, 0x43, 0x1b, 0x2e, 0xbb,
+    0xb0, 0xe9, 0xef, 0x0b, 0x03, 0x32, 0x74,
+};
+
+static const uint8_t exp_signature_ecdsa_p384[] = {
+    0x30, 0x64, 0x02, 0x30, 0x3c, 0x79, 0x7f, 0x5a,
+    0x91, 0x08, 0x79, 0xde, 0x6e, 0x03, 0x19, 0x39,
+    0xcb, 0x94, 0x9c, 0xc6, 0x09, 0x12, 0xfa, 0xbd,
+    0xa8, 0x35, 0x5e, 0x3f, 0x74, 0x05, 0x12, 0xd1,
+    0x8e, 0xd9, 0x3c, 0x79, 0x9d, 0x7c, 0x1a, 0xae,
+    0x96, 0x05, 0x0e, 0x35, 0x21, 0x73, 0xd8, 0xfe,
+    0x1b, 0x43, 0x06, 0xb9, 0x02, 0x30, 0x11, 0xdf,
+    0xa7, 0xba, 0x70, 0x84, 0x4b, 0x74, 0xab, 0x1e,
+    0x9e, 0x6a, 0xc1, 0x46, 0xe3, 0x98, 0x0a, 0x25,
+    0x82, 0xf5, 0xff, 0xb5, 0x6f, 0x04, 0xda, 0xc3,
+    0xfd, 0x3e, 0xea, 0x96, 0x03, 0x0c, 0x22, 0xf2,
+    0xda, 0x86, 0xeb, 0x91, 0x2f, 0x36, 0x13, 0xba,
+    0x37, 0xcd, 0xe7, 0x91, 0x85, 0xf3,
+};
+
+
 static const uint8_t test_plaintext[] = {
     0x00, 0x44, 0xbc, 0x6f, 0x77, 0xfb, 0xe2, 0xa4,
     0x98, 0x9e, 0xf5, 0x33, 0xa0, 0xbd, 0x81, 0xb9,
@@ -870,7 +1003,60 @@ static QCryptoAkCipherTestData akcipher_test_data[] = {
         .signature = exp_signature_rsa2048_pkcs1,
         .slen = sizeof(exp_signature_rsa2048_pkcs1),
     },
+    {
+        .path = "/crypto/akcipher/ecdsa-nist-p192",
+        .opt = {
+            .alg = QCRYPTO_AKCIPHER_ALG_ECDSA,
+            .u.ecdsa = {
+                .curve_id = QCRYPTO_CURVE_ID_NIST_P192,
+            },
+        },
+        .pub_key = ecdsa_p192_public_key,
+        .pub_key_len = sizeof(ecdsa_p192_public_key),
+        .priv_key = ecdsa_p192_private_key,
+        .priv_key_len = sizeof(ecdsa_p192_private_key),
 
+        .dgst = test_sha1_dgst,
+        .dlen = sizeof(test_sha1_dgst),
+        .signature = exp_signature_ecdsa_p192,
+        .slen = sizeof(exp_signature_ecdsa_p192),
+    },
+    {
+        .path = "/crypto/akcipher/ecdsa-nist-p256",
+        .opt = {
+            .alg = QCRYPTO_AKCIPHER_ALG_ECDSA,
+            .u.ecdsa = {
+                .curve_id = QCRYPTO_CURVE_ID_NIST_P256,
+            },
+        },
+        .pub_key = ecdsa_p256_public_key,
+        .pub_key_len = sizeof(ecdsa_p256_public_key),
+        .priv_key = ecdsa_p256_private_key,
+        .priv_key_len = sizeof(ecdsa_p256_private_key),
+
+        .dgst = test_sha1_dgst,
+        .dlen = sizeof(test_sha1_dgst),
+        .signature = exp_signature_ecdsa_p256,
+        .slen = sizeof(exp_signature_ecdsa_p256),
+    },
+    {
+        .path = "/crypto/akcipher/ecdsa-nist-p384",
+        .opt = {
+            .alg = QCRYPTO_AKCIPHER_ALG_ECDSA,
+            .u.ecdsa = {
+                .curve_id = QCRYPTO_CURVE_ID_NIST_P384,
+            },
+        },
+        .pub_key = ecdsa_p384_public_key,
+        .pub_key_len = sizeof(ecdsa_p384_public_key),
+        .priv_key = ecdsa_p384_private_key,
+        .priv_key_len = sizeof(ecdsa_p384_private_key),
+
+        .dgst = test_sha1_dgst,
+        .dlen = sizeof(test_sha1_dgst),
+        .signature = exp_signature_ecdsa_p384,
+        .slen = sizeof(exp_signature_ecdsa_p384),
+    },
 };
 
 static void test_akcipher(const void *opaque)
@@ -879,6 +1065,8 @@ static void test_akcipher(const void *opaque)
     g_autofree uint8_t *plaintext = NULL;
     g_autofree uint8_t *ciphertext = NULL;
     g_autofree uint8_t *signature = NULL;
+    int signature_len, ciphertext_len, plaintext_len;
+    int max_plen, max_slen, max_clen;
     QCryptoAkCipher *pub_key, *priv_key;
 
     if (!qcrypto_akcipher_supports((QCryptoAkCipherOptions *)&data->opt)) {
@@ -894,32 +1082,41 @@ static void test_akcipher(const void *opaque)
                                     data->priv_key, data->priv_key_len,
                                     &error_abort);
     g_assert(priv_key != NULL);
+    max_plen = qcrypto_akcipher_max_plaintext_len(pub_key);
+    max_clen = qcrypto_akcipher_max_plaintext_len(pub_key);
+    max_slen = qcrypto_akcipher_max_signature_len(priv_key);
 
     if (data->plaintext != NULL) {
 
-        ciphertext = g_new0(uint8_t, data->clen);
-        g_assert(qcrypto_akcipher_encrypt(pub_key, data->plaintext, data->plen,
-                                          ciphertext, data->clen,
-                                          &error_abort) > 0);
+        ciphertext = g_new0(uint8_t, max_clen);
+        ciphertext_len = qcrypto_akcipher_encrypt(pub_key,
+                                                  data->plaintext, data->plen,
+                                                  ciphertext, max_clen,
+                                                  &error_abort);
+        g_assert(ciphertext_len > 0);
 
         /**
          * In the asymmetric encryption algorithms, the ciphertext generated
          * each time may be different, here only compare the decrypted
          * plaintext
          */
-        plaintext = g_new0(uint8_t, data->clen);
-        g_assert(qcrypto_akcipher_decrypt(priv_key, ciphertext,
-                                          data->clen, plaintext,
-                                          data->plen,
-                                          &error_abort) == data->plen);
+        plaintext = g_new0(uint8_t, max_plen);
+        plaintext_len = qcrypto_akcipher_decrypt(priv_key,
+                                                 ciphertext, ciphertext_len,
+                                                 plaintext, max_plen,
+                                                 &error_abort);
+        g_assert(plaintext_len == data->plen);
         g_assert(!memcmp(plaintext, data->plaintext, data->plen));
     }
 
     if (data->signature != NULL) {
-        signature = g_new(uint8_t, data->slen);
-        g_assert(qcrypto_akcipher_sign(priv_key, data->dgst, data->dlen,
-                                       signature, data->slen,
-                                       &error_abort) > 0);
+        signature = g_new(uint8_t, max_slen);
+        signature_len = qcrypto_akcipher_sign(priv_key,
+                                              data->dgst, data->dlen,
+                                              signature, max_slen,
+                                              &error_abort);
+        g_assert(signature_len > 0);
+
         /**
          * The signature generated each time may be different, here only check
          * the verification.
@@ -927,12 +1124,12 @@ static void test_akcipher(const void *opaque)
         g_assert(qcrypto_akcipher_verify(pub_key, data->signature, data->slen,
                                          data->dgst, data->dlen,
                                          &error_abort) == 0);
-        g_assert(qcrypto_akcipher_verify(pub_key, signature, data->slen,
+        g_assert(qcrypto_akcipher_verify(pub_key, signature, signature_len,
                                          data->dgst, data->dlen,
                                          &error_abort) == 0);
         ++signature[0];
         /* Here error should be ignored */
-        g_assert(qcrypto_akcipher_verify(pub_key, signature, data->slen,
+        g_assert(qcrypto_akcipher_verify(pub_key, signature, signature_len,
                                          data->dgst, data->dlen, NULL) != 0);
     }
 
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/7] crypto: remove "qemu/osdep.h" in rsakey.h
  2022-06-13  8:45 ` [PATCH 3/7] crypto: remove "qemu/osdep.h" in rsakey.h Lei He
@ 2022-06-13 13:55   ` Philippe Mathieu-Daudé via
  2022-06-17 11:20   ` Daniel P. Berrangé
  1 sibling, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-06-13 13:55 UTC (permalink / raw)
  To: Lei He, mst, arei.gonglei, berrange, qemu-devel; +Cc: pizhenwei, jasowang

On 13/6/22 10:45, Lei He wrote:
> Move 'include "qemu/osdep.h"' from rsakey.h to rsakey.c.
> 
> Signed-off-by: lei he <helei.sig11@bytedance.com>
> ---
>   crypto/rsakey.c | 1 +
>   crypto/rsakey.h | 1 -
>   2 files changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/7] crypto: Add ECDSA key parser
  2022-06-13  8:45 ` [PATCH 4/7] crypto: Add ECDSA key parser Lei He
@ 2022-06-13 14:19   ` Philippe Mathieu-Daudé via
  2022-06-14  1:43     ` [Phishing Risk] [External] " 何磊
  2022-06-17 11:34   ` Daniel P. Berrangé
  1 sibling, 1 reply; 21+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-06-13 14:19 UTC (permalink / raw)
  To: Lei He, mst, arei.gonglei, berrange, qemu-devel; +Cc: pizhenwei, jasowang

On 13/6/22 10:45, Lei He wrote:
> Add ECDSA key parser and ECDSA signautre parser.
> 
> Signed-off-by: lei he <helei.sig11@bytedance.com>
> ---
>   crypto/ecdsakey-builtin.c.inc | 248 ++++++++++++++++++++++++++++++++++++++++++
>   crypto/ecdsakey.c             | 118 ++++++++++++++++++++
>   crypto/ecdsakey.h             |  66 +++++++++++
>   crypto/meson.build            |   1 +
>   4 files changed, 433 insertions(+)
>   create mode 100644 crypto/ecdsakey-builtin.c.inc
>   create mode 100644 crypto/ecdsakey.c
>   create mode 100644 crypto/ecdsakey.h
> 
> diff --git a/crypto/ecdsakey-builtin.c.inc b/crypto/ecdsakey-builtin.c.inc
> new file mode 100644
> index 0000000000..5da317ec44
> --- /dev/null
> +++ b/crypto/ecdsakey-builtin.c.inc
> @@ -0,0 +1,248 @@
> +/*
> + * QEMU Crypto akcipher algorithms
> + *
> + * Copyright (c) 2022 Bytedance
> + * Author: lei he <helei.sig11@bytedance.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "der.h"
> +#include "ecdsakey.h"
> +
> +#define QCRYPTO_ECDSA_PUBKEY_FMT_UNCOMPRESSED 0x04
> +
> +static int extract_mpi(void *ctx, const uint8_t *value,
> +                       size_t vlen, Error **errp)
> +{
> +    QCryptoAkCipherMPI *mpi = (QCryptoAkCipherMPI *)ctx;
> +    if (vlen == 0) {
> +        error_setg(errp, "Empty mpi field");
> +        return -1;

Functions taking Error* param usually return a boolean.

> +    }
> +    mpi->data = g_memdup2(value, vlen);
> +    mpi->len = vlen;
> +    return 0;
> +}
> +
> +static int extract_version(void *ctx, const uint8_t *value,
> +                           size_t vlen, Error **errp)
> +{
> +    uint8_t *version = (uint8_t *)ctx;
> +    if (vlen != 1 || *value > 1) {
> +        error_setg(errp, "Invalid rsakey version");
> +        return -1;
> +    }
> +    *version = *value;
> +    return 0;
> +}
> +
> +static int extract_cons_content(void *ctx, const uint8_t *value,
> +                                size_t vlen, Error **errp)
> +{
> +    const uint8_t **content = (const uint8_t **)ctx;
> +    if (vlen == 0) {
> +        error_setg(errp, "Empty sequence");
> +        return -1;
> +    }
> +    *content = value;

You need to check (vlen >= sizeof(uint8_t *)) to avoid overrun.

> +    return 0;
> +}
> +
> +static int __qcrypto_akcipher_builtin_ecdsa_pubkey_parse(
> +    QCryptoAkCipherECDSAKey *ecdsa,
> +    const uint8_t *key, size_t keylen, Error **errp);

Why use the reserved __prefix?


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Phishing Risk] [External] Re: [PATCH 4/7] crypto: Add ECDSA key parser
  2022-06-13 14:19   ` Philippe Mathieu-Daudé via
@ 2022-06-14  1:43     ` 何磊
  2022-06-14  6:48       ` Philippe Mathieu-Daudé via
  2022-06-16 16:44       ` Michael S. Tsirkin
  0 siblings, 2 replies; 21+ messages in thread
From: 何磊 @ 2022-06-14  1:43 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: 何磊, S. Tsirkin, Michael, Gonglei (Arei),
	"Daniel P . Berrangé",
	qemu-devel, pizhenwei, jasowang

Hi Philippe, lots of thanks for your review!

> On Jun 13, 2022, at 10:19 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> 
> On 13/6/22 10:45, Lei He wrote:
>> Add ECDSA key parser and ECDSA signautre parser.
>> Signed-off-by: lei he <helei.sig11@bytedance.com>
>> ---
>>  crypto/ecdsakey-builtin.c.inc | 248 ++++++++++++++++++++++++++++++++++++++++++
>>  crypto/ecdsakey.c             | 118 ++++++++++++++++++++
>>  crypto/ecdsakey.h             |  66 +++++++++++
>>  crypto/meson.build            |   1 +
>>  4 files changed, 433 insertions(+)
>>  create mode 100644 crypto/ecdsakey-builtin.c.inc
>>  create mode 100644 crypto/ecdsakey.c
>>  create mode 100644 crypto/ecdsakey.h
>> diff --git a/crypto/ecdsakey-builtin.c.inc b/crypto/ecdsakey-builtin.c.inc
>> new file mode 100644
>> index 0000000000..5da317ec44
>> --- /dev/null
>> +++ b/crypto/ecdsakey-builtin.c.inc
>> @@ -0,0 +1,248 @@
>> +/*
>> + * QEMU Crypto akcipher algorithms
>> + *
>> + * Copyright (c) 2022 Bytedance
>> + * Author: lei he <helei.sig11@bytedance.com>
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>> + *
>> + */
>> +
>> +#include "der.h"
>> +#include "ecdsakey.h"
>> +
>> +#define QCRYPTO_ECDSA_PUBKEY_FMT_UNCOMPRESSED 0x04
>> +
>> +static int extract_mpi(void *ctx, const uint8_t *value,
>> +                       size_t vlen, Error **errp)
>> +{
>> +    QCryptoAkCipherMPI *mpi = (QCryptoAkCipherMPI *)ctx;
>> +    if (vlen == 0) {
>> +        error_setg(errp, "Empty mpi field");
>> +        return -1;
> 
> Functions taking Error* param usually return a boolean.

It's a good idea to make such functions that only return 0 or -1 return bool directly, but this change 
will require modification of rsakey related code. If you strongly request it, I will modify it in another patch.

> 
>> +    }
>> +    mpi->data = g_memdup2(value, vlen);
>> +    mpi->len = vlen;
>> +    return 0;
>> +}
>> +
>> +static int extract_version(void *ctx, const uint8_t *value,
>> +                           size_t vlen, Error **errp)
>> +{
>> +    uint8_t *version = (uint8_t *)ctx;
>> +    if (vlen != 1 || *value > 1) {
>> +        error_setg(errp, "Invalid rsakey version");
>> +        return -1;
>> +    }
>> +    *version = *value;
>> +    return 0;
>> +}
>> +
>> +static int extract_cons_content(void *ctx, const uint8_t *value,
>> +                                size_t vlen, Error **errp)
>> +{
>> +    const uint8_t **content = (const uint8_t **)ctx;
>> +    if (vlen == 0) {
>> +        error_setg(errp, "Empty sequence");
>> +        return -1;
>> +    }
>> +    *content = value;
> 
> You need to check (vlen >= sizeof(uint8_t *)) to avoid overrun.

The decoder will parse the meta data of ASN1 types and pass the real data part to the callback function. 
The above statement only saves the starting address of the ‘data part' and does not actually access the 
data, so there is no need to check the size of vlen. 

> 
>> +    return 0;
>> +}
>> +
>> +static int __qcrypto_akcipher_builtin_ecdsa_pubkey_parse(
>> +    QCryptoAkCipherECDSAKey *ecdsa,
>> +    const uint8_t *key, size_t keylen, Error **errp);
> 
> Why use the reserved __prefix?

I will fix it later.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Phishing Risk] [External] Re: [PATCH 4/7] crypto: Add ECDSA key parser
  2022-06-14  1:43     ` [Phishing Risk] [External] " 何磊
@ 2022-06-14  6:48       ` Philippe Mathieu-Daudé via
  2022-06-16 16:44       ` Michael S. Tsirkin
  1 sibling, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-06-14  6:48 UTC (permalink / raw)
  To: 何磊
  Cc: S. Tsirkin, Michael, Gonglei (Arei), Daniel P . Berrangé,
	qemu-devel, pizhenwei, jasowang

On 14/6/22 03:43, 何磊 wrote:
> Hi Philippe, lots of thanks for your review!
> 
>> On Jun 13, 2022, at 10:19 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> On 13/6/22 10:45, Lei He wrote:
>>> Add ECDSA key parser and ECDSA signautre parser.
>>> Signed-off-by: lei he <helei.sig11@bytedance.com>
>>> ---
>>>   crypto/ecdsakey-builtin.c.inc | 248 ++++++++++++++++++++++++++++++++++++++++++
>>>   crypto/ecdsakey.c             | 118 ++++++++++++++++++++
>>>   crypto/ecdsakey.h             |  66 +++++++++++
>>>   crypto/meson.build            |   1 +
>>>   4 files changed, 433 insertions(+)
>>>   create mode 100644 crypto/ecdsakey-builtin.c.inc
>>>   create mode 100644 crypto/ecdsakey.c
>>>   create mode 100644 crypto/ecdsakey.h
>>> diff --git a/crypto/ecdsakey-builtin.c.inc b/crypto/ecdsakey-builtin.c.inc
>>> new file mode 100644
>>> index 0000000000..5da317ec44
>>> --- /dev/null
>>> +++ b/crypto/ecdsakey-builtin.c.inc
>>> @@ -0,0 +1,248 @@
>>> +/*
>>> + * QEMU Crypto akcipher algorithms
>>> + *
>>> + * Copyright (c) 2022 Bytedance
>>> + * Author: lei he <helei.sig11@bytedance.com>
>>> + *
>>> + * This library is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Lesser General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2.1 of the License, or (at your option) any later version.
>>> + *
>>> + * This library is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + * Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public
>>> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>>> + *
>>> + */
>>> +
>>> +#include "der.h"
>>> +#include "ecdsakey.h"
>>> +
>>> +#define QCRYPTO_ECDSA_PUBKEY_FMT_UNCOMPRESSED 0x04
>>> +
>>> +static int extract_mpi(void *ctx, const uint8_t *value,
>>> +                       size_t vlen, Error **errp)
>>> +{
>>> +    QCryptoAkCipherMPI *mpi = (QCryptoAkCipherMPI *)ctx;
>>> +    if (vlen == 0) {
>>> +        error_setg(errp, "Empty mpi field");
>>> +        return -1;
>>
>> Functions taking Error* param usually return a boolean.
> 
> It's a good idea to make such functions that only return 0 or -1 return bool directly, but this change
> will require modification of rsakey related code. If you strongly request it, I will modify it in another patch.

QCryptoDERDecodeCb seems to only return a boolean, so should follow the
style recommended in 
https://gitlab.com/qemu-project/qemu/-/commit/e3fe3988d7. Can be done 
later as a follow-up cleanup of course.

>>> +    }
>>> +    mpi->data = g_memdup2(value, vlen);
>>> +    mpi->len = vlen;
>>> +    return 0;
>>> +}
>>> +
>>> +static int extract_version(void *ctx, const uint8_t *value,
>>> +                           size_t vlen, Error **errp)
>>> +{
>>> +    uint8_t *version = (uint8_t *)ctx;
>>> +    if (vlen != 1 || *value > 1) {
>>> +        error_setg(errp, "Invalid rsakey version");
>>> +        return -1;
>>> +    }
>>> +    *version = *value;
>>> +    return 0;
>>> +}
>>> +
>>> +static int extract_cons_content(void *ctx, const uint8_t *value,
>>> +                                size_t vlen, Error **errp)
>>> +{
>>> +    const uint8_t **content = (const uint8_t **)ctx;
>>> +    if (vlen == 0) {
>>> +        error_setg(errp, "Empty sequence");
>>> +        return -1;
>>> +    }
>>> +    *content = value;
>>
>> You need to check (vlen >= sizeof(uint8_t *)) to avoid overrun.
> 
> The decoder will parse the meta data of ASN1 types and pass the real data part to the callback function.
> The above statement only saves the starting address of the ‘data part' and does not actually access the
> data, so there is no need to check the size of vlen.

Oops, indeed you are right :)

>>
>>> +    return 0;
>>> +}
>>> +
>>> +static int __qcrypto_akcipher_builtin_ecdsa_pubkey_parse(
>>> +    QCryptoAkCipherECDSAKey *ecdsa,
>>> +    const uint8_t *key, size_t keylen, Error **errp);
>>
>> Why use the reserved __prefix?
> 
> I will fix it later.
> 



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/7] crypto: Introduce ECDSA algorithm API
  2022-06-13  8:45 ` [PATCH 1/7] crypto: Introduce ECDSA algorithm API Lei He
@ 2022-06-14  6:51   ` Philippe Mathieu-Daudé via
  2022-06-17 11:14   ` Daniel P. Berrangé
  1 sibling, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-06-14  6:51 UTC (permalink / raw)
  To: Lei He, mst, arei.gonglei, berrange, qemu-devel; +Cc: pizhenwei, jasowang

On 13/6/22 10:45, Lei He wrote:
> Introduce ECDSA related structures to qapi-crypto.
> 
> Signed-off-by: lei he <helei.sig11@bytedance.com>
> ---
>   qapi/crypto.json | 28 ++++++++++++++++++++++++++--
>   1 file changed, 26 insertions(+), 2 deletions(-)

> +##
> +# @QCryptoAkCipherOptionsECDSA:
> +#
> +# Specific parameters for ECDSA algorithm.
> +#
> +# @curve-id: QCryptoCurveId
> +#
> +# Since: 7.1
> +##
> +{ 'struct': 'QCryptoAkCipherOptionsECDSA',
> +  'data': { 'curve-id':'QCryptoCurveID' }}

Missing an extra space after 'curve-id':, otherwise
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Phishing Risk] [External] Re: [PATCH 4/7] crypto: Add ECDSA key parser
  2022-06-14  1:43     ` [Phishing Risk] [External] " 何磊
  2022-06-14  6:48       ` Philippe Mathieu-Daudé via
@ 2022-06-16 16:44       ` Michael S. Tsirkin
  1 sibling, 0 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2022-06-16 16:44 UTC (permalink / raw)
  To: 何磊
  Cc: Philippe Mathieu-Daudé, Gonglei (Arei),
	"Daniel P . Berrangé",
	qemu-devel, pizhenwei, jasowang

On Tue, Jun 14, 2022 at 09:43:59AM +0800, 何磊 wrote:
> Hi Philippe, lots of thanks for your review!
> 
> > On Jun 13, 2022, at 10:19 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > 
> > On 13/6/22 10:45, Lei He wrote:
> >> Add ECDSA key parser and ECDSA signautre parser.
> >> Signed-off-by: lei he <helei.sig11@bytedance.com>
> >> ---
> >>  crypto/ecdsakey-builtin.c.inc | 248 ++++++++++++++++++++++++++++++++++++++++++
> >>  crypto/ecdsakey.c             | 118 ++++++++++++++++++++
> >>  crypto/ecdsakey.h             |  66 +++++++++++
> >>  crypto/meson.build            |   1 +
> >>  4 files changed, 433 insertions(+)
> >>  create mode 100644 crypto/ecdsakey-builtin.c.inc
> >>  create mode 100644 crypto/ecdsakey.c
> >>  create mode 100644 crypto/ecdsakey.h
> >> diff --git a/crypto/ecdsakey-builtin.c.inc b/crypto/ecdsakey-builtin.c.inc
> >> new file mode 100644
> >> index 0000000000..5da317ec44
> >> --- /dev/null
> >> +++ b/crypto/ecdsakey-builtin.c.inc
> >> @@ -0,0 +1,248 @@
> >> +/*
> >> + * QEMU Crypto akcipher algorithms
> >> + *
> >> + * Copyright (c) 2022 Bytedance
> >> + * Author: lei he <helei.sig11@bytedance.com>
> >> + *
> >> + * This library is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU Lesser General Public
> >> + * License as published by the Free Software Foundation; either
> >> + * version 2.1 of the License, or (at your option) any later version.
> >> + *
> >> + * This library is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >> + * Lesser General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU Lesser General Public
> >> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> >> + *
> >> + */
> >> +
> >> +#include "der.h"
> >> +#include "ecdsakey.h"
> >> +
> >> +#define QCRYPTO_ECDSA_PUBKEY_FMT_UNCOMPRESSED 0x04
> >> +
> >> +static int extract_mpi(void *ctx, const uint8_t *value,
> >> +                       size_t vlen, Error **errp)
> >> +{
> >> +    QCryptoAkCipherMPI *mpi = (QCryptoAkCipherMPI *)ctx;
> >> +    if (vlen == 0) {
> >> +        error_setg(errp, "Empty mpi field");
> >> +        return -1;
> > 
> > Functions taking Error* param usually return a boolean.
> 
> It's a good idea to make such functions that only return 0 or -1 return bool directly, but this change 
> will require modification of rsakey related code. If you strongly request it, I will modify it in another patch.
> 
> > 
> >> +    }
> >> +    mpi->data = g_memdup2(value, vlen);
> >> +    mpi->len = vlen;
> >> +    return 0;
> >> +}
> >> +
> >> +static int extract_version(void *ctx, const uint8_t *value,
> >> +                           size_t vlen, Error **errp)
> >> +{
> >> +    uint8_t *version = (uint8_t *)ctx;
> >> +    if (vlen != 1 || *value > 1) {
> >> +        error_setg(errp, "Invalid rsakey version");
> >> +        return -1;
> >> +    }
> >> +    *version = *value;
> >> +    return 0;
> >> +}
> >> +
> >> +static int extract_cons_content(void *ctx, const uint8_t *value,
> >> +                                size_t vlen, Error **errp)
> >> +{
> >> +    const uint8_t **content = (const uint8_t **)ctx;
> >> +    if (vlen == 0) {
> >> +        error_setg(errp, "Empty sequence");
> >> +        return -1;
> >> +    }
> >> +    *content = value;
> > 
> > You need to check (vlen >= sizeof(uint8_t *)) to avoid overrun.
> 
> The decoder will parse the meta data of ASN1 types and pass the real data part to the callback function. 
> The above statement only saves the starting address of the ‘data part' and does not actually access the 
> data, so there is no need to check the size of vlen. 
> 
> > 
> >> +    return 0;
> >> +}
> >> +
> >> +static int __qcrypto_akcipher_builtin_ecdsa_pubkey_parse(
> >> +    QCryptoAkCipherECDSAKey *ecdsa,
> >> +    const uint8_t *key, size_t keylen, Error **errp);
> > 
> > Why use the reserved __prefix?
> 
> I will fix it later.

expect a new version then



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/7] crypto: Introduce ECDSA algorithm API
  2022-06-13  8:45 ` [PATCH 1/7] crypto: Introduce ECDSA algorithm API Lei He
  2022-06-14  6:51   ` Philippe Mathieu-Daudé via
@ 2022-06-17 11:14   ` Daniel P. Berrangé
  1 sibling, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2022-06-17 11:14 UTC (permalink / raw)
  To: Lei He; +Cc: mst, arei.gonglei, qemu-devel, pizhenwei, jasowang

On Mon, Jun 13, 2022 at 04:45:25PM +0800, Lei He wrote:
> Introduce ECDSA related structures to qapi-crypto.
> 
> Signed-off-by: lei he <helei.sig11@bytedance.com>
> ---
>  qapi/crypto.json | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/7] crypto: Support more ASN.1 types
  2022-06-13  8:45 ` [PATCH 2/7] crypto: Support more ASN.1 types Lei He
@ 2022-06-17 11:20   ` Daniel P. Berrangé
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2022-06-17 11:20 UTC (permalink / raw)
  To: Lei He; +Cc: mst, arei.gonglei, qemu-devel, pizhenwei, jasowang

On Mon, Jun 13, 2022 at 04:45:26PM +0800, Lei He wrote:
> 1. support decoding of 'bit string','octet string',
> 'object id', and 'context specific tag' for DER decoder.
> 2. support encoding of int and sequence for DER decoder.
> 3. add test suites for the above changes.
> 
> Signed-off-by: lei he <helei.sig11@bytedance.com>
> ---
>  crypto/der.c                 | 174 +++++++++++++++++++++++++++++++++++++------
>  crypto/der.h                 | 128 ++++++++++++++++++++++++++++++-
>  tests/unit/test-crypto-der.c | 126 +++++++++++++++++++++++++------
>  3 files changed, 382 insertions(+), 46 deletions(-)
> 
> diff --git a/crypto/der.c b/crypto/der.c
> index f877390bbb..edf2c6c313 100644
> --- a/crypto/der.c
> +++ b/crypto/der.c
> @@ -27,15 +27,68 @@ enum QCryptoDERTypeTag {
>      QCRYPTO_DER_TYPE_TAG_INT = 0x2,
>      QCRYPTO_DER_TYPE_TAG_BIT_STR = 0x3,
>      QCRYPTO_DER_TYPE_TAG_OCT_STR = 0x4,
> -    QCRYPTO_DER_TYPE_TAG_OCT_NULL = 0x5,
> -    QCRYPTO_DER_TYPE_TAG_OCT_OID = 0x6,
> +    QCRYPTO_DER_TYPE_TAG_NULL = 0x5,
> +    QCRYPTO_DER_TYPE_TAG_OID = 0x6,
>      QCRYPTO_DER_TYPE_TAG_SEQ = 0x10,
>      QCRYPTO_DER_TYPE_TAG_SET = 0x11,
>  };
>  
> -#define QCRYPTO_DER_CONSTRUCTED_MASK 0x20
> +enum QCryptoDERTagClass {
> +    QCRYPTO_DER_TAG_CLASS_UNIV = 0x0,
> +    QCRYPTO_DER_TAG_CLASS_APPL = 0x1,
> +    QCRYPTO_DER_TAG_CLASS_CONT = 0x2,
> +    QCRYPTO_DER_TAG_CLASS_PRIV = 0x3,
> +};
> +
> +enum QCryptoDERTagEnc {
> +    QCRYPTO_DER_TAG_ENC_PRIM = 0x0,
> +    QCRYPTO_DER_TAG_ENC_CONS = 0x1,
> +};
> +
> +#define QCRYPTO_DER_TAG_ENC_MASK 0x20
> +#define QCRYPTO_DER_TAG_ENC_SHIFT 5
> +
> +#define QCRYPTO_DER_TAG_CLASS_MASK 0xc0
> +#define QCRYPTO_DER_TAG_CLASS_SHIFT 6
> +
> +#define QCRYPTO_DER_TAG_VAL_MASK 0x1f
>  #define QCRYPTO_DER_SHORT_LEN_MASK 0x80
>  
> +#define QCRYPTO_DER_TAG(class, enc, val)        \
> +    (((class) << QCRYPTO_DER_TAG_CLASS_SHIFT) | \
> +     ((enc) << QCRYPTO_DER_TAG_ENC_SHIFT) | val)
> +
> +static void qcrypto_der_encode_data(const uint8_t *src, size_t src_len,
> +                                    uint8_t *dst, size_t *dst_len)
> +{
> +    size_t max_length = 0xFF;
> +    uint8_t length_bytes = 0, header_byte;
> +
> +    if (src_len < QCRYPTO_DER_SHORT_LEN_MASK) {
> +        header_byte = src_len;
> +        *dst_len = src_len + 1;
> +    } else {
> +        for (length_bytes = 1; max_length < src_len;) {
> +            length_bytes++;
> +            max_length = (max_length << 8) + max_length;
> +        }

Can't length_bytes++ be in the for clause like:

         for (length_bytes = 1; max_length < src_len;length_bytes++) {
             max_length = (max_length << 8) + max_length;
         }

?


Aside from that minor nitpick

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/7] crypto: remove "qemu/osdep.h" in rsakey.h
  2022-06-13  8:45 ` [PATCH 3/7] crypto: remove "qemu/osdep.h" in rsakey.h Lei He
  2022-06-13 13:55   ` Philippe Mathieu-Daudé via
@ 2022-06-17 11:20   ` Daniel P. Berrangé
  1 sibling, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2022-06-17 11:20 UTC (permalink / raw)
  To: Lei He; +Cc: mst, arei.gonglei, qemu-devel, pizhenwei, jasowang

On Mon, Jun 13, 2022 at 04:45:27PM +0800, Lei He wrote:
> Move 'include "qemu/osdep.h"' from rsakey.h to rsakey.c.
> 
> Signed-off-by: lei he <helei.sig11@bytedance.com>
> ---
>  crypto/rsakey.c | 1 +
>  crypto/rsakey.h | 1 -
>  2 files changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/7] crypto: Add ECDSA key parser
  2022-06-13  8:45 ` [PATCH 4/7] crypto: Add ECDSA key parser Lei He
  2022-06-13 14:19   ` Philippe Mathieu-Daudé via
@ 2022-06-17 11:34   ` Daniel P. Berrangé
  1 sibling, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2022-06-17 11:34 UTC (permalink / raw)
  To: Lei He; +Cc: mst, arei.gonglei, qemu-devel, pizhenwei, jasowang

On Mon, Jun 13, 2022 at 04:45:28PM +0800, Lei He wrote:
> Add ECDSA key parser and ECDSA signautre parser.

                         typo:  'signature'

> 
> Signed-off-by: lei he <helei.sig11@bytedance.com>
> ---
>  crypto/ecdsakey-builtin.c.inc | 248 ++++++++++++++++++++++++++++++++++++++++++
>  crypto/ecdsakey.c             | 118 ++++++++++++++++++++
>  crypto/ecdsakey.h             |  66 +++++++++++
>  crypto/meson.build            |   1 +
>  4 files changed, 433 insertions(+)
>  create mode 100644 crypto/ecdsakey-builtin.c.inc
>  create mode 100644 crypto/ecdsakey.c
>  create mode 100644 crypto/ecdsakey.h
> 
> diff --git a/crypto/ecdsakey-builtin.c.inc b/crypto/ecdsakey-builtin.c.inc
> new file mode 100644
> index 0000000000..5da317ec44
> --- /dev/null
> +++ b/crypto/ecdsakey-builtin.c.inc
> @@ -0,0 +1,248 @@
> +/*
> + * QEMU Crypto akcipher algorithms
> + *
> + * Copyright (c) 2022 Bytedance
> + * Author: lei he <helei.sig11@bytedance.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "der.h"
> +#include "ecdsakey.h"
> +
> +#define QCRYPTO_ECDSA_PUBKEY_FMT_UNCOMPRESSED 0x04
> +
> +static int extract_mpi(void *ctx, const uint8_t *value,
> +                       size_t vlen, Error **errp)
> +{
> +    QCryptoAkCipherMPI *mpi = (QCryptoAkCipherMPI *)ctx;
> +    if (vlen == 0) {
> +        error_setg(errp, "Empty mpi field");
> +        return -1;
> +    }
> +    mpi->data = g_memdup2(value, vlen);
> +    mpi->len = vlen;
> +    return 0;
> +}
> +
> +static int extract_version(void *ctx, const uint8_t *value,
> +                           size_t vlen, Error **errp)
> +{
> +    uint8_t *version = (uint8_t *)ctx;
> +    if (vlen != 1 || *value > 1) {
> +        error_setg(errp, "Invalid rsakey version");
> +        return -1;
> +    }
> +    *version = *value;
> +    return 0;
> +}
> +
> +static int extract_cons_content(void *ctx, const uint8_t *value,
> +                                size_t vlen, Error **errp)
> +{
> +    const uint8_t **content = (const uint8_t **)ctx;
> +    if (vlen == 0) {
> +        error_setg(errp, "Empty sequence");
> +        return -1;
> +    }
> +    *content = value;
> +    return 0;
> +}
> +
> +static int __qcrypto_akcipher_builtin_ecdsa_pubkey_parse(
> +    QCryptoAkCipherECDSAKey *ecdsa,
> +    const uint8_t *key, size_t keylen, Error **errp);

It is not good practice to use '_' on the start of method
names in apps, as names with a leading '_' are reserved.

> +
> +static int extract_pubkey(void *ctx, const uint8_t *value,
> +                          size_t vlen, Error **errp)
> +{
> +    QCryptoAkCipherECDSAKey *ecdsa = (QCryptoAkCipherECDSAKey *)ctx;
> +    if (vlen < 4) {
> +        error_setg(errp, "Public key part too short");
> +        return -1;
> +    }
> +    /* Skip meta bit of BIT STRING */
> +    value++;
> +    vlen--;
> +    return __qcrypto_akcipher_builtin_ecdsa_pubkey_parse(
> +        ecdsa, value, vlen, errp);
> +}
> +
> +/**
> + *
> + *        ECDSASignature ::= SEQUENCE {
> + *             r           INTEGER
> + *             s           INTEGER
> + *         }
> + */
> +QCryptoAkCipherECDSASig *qcrypto_akcipher_ecdsasig_parse(
> +    const uint8_t *signature, size_t len, Error **errp)
> +{
> +    QCryptoAkCipherECDSASig *sig = g_new0(QCryptoAkCipherECDSASig, 1);

Use  g_autoptr(QCryptoAkCipherECDSASig) sig  here

> +    const uint8_t *seq;
> +    size_t seq_length;
> +    int decode_ret;
> +
> +    decode_ret = qcrypto_der_decode_seq(&signature, &len,
> +                                        extract_cons_content, &seq, errp);
> +
> +    if (decode_ret < 0 || len != 0) {
> +        goto error;
> +    }

If 'decode_ret < 0' then errp should be set by qcrypto_der_decode_seq
which is fine.  For len != 0, we need to report an error ourselves.
I see you pushed it to the error label so later codepath can share it.
I think it is better to do it here though, because it makes it clear
to the reader which codepaths are triggering this generic error
messages. So

     if (decode_ret < 0)
         goto error;
     }
     if (len != 0) {
         error_setg(errp, "Invalid RSA public key");
     }


> +    seq_length = decode_ret;
> +
> +    if (qcrypto_der_decode_int(&seq, &seq_length, extract_mpi,
> +                               &sig->r, errp) < 0 ||
> +        qcrypto_der_decode_int(&seq, &seq_length, extract_mpi,
> +                               &sig->s, errp) < 0) {
> +        goto error;
> +    }
> +    if (seq_length != 0) {

Add

        error_setg(errp, "Invalid RSA public key");

> +        goto error;
> +    }
> +
> +    return sig;

return g_steal_pointer(&sig)

> +
> +error:
> +    if (errp && !*errp) {
> +        error_setg(errp, "Invalid RSA public key");
> +    }

and remove this

> +    qcrypto_akcipher_ecdsasig_free(sig);
> +    return NULL;
> +}

This error block won't need to exist at all. Everywhere can
just 'return NULL' instead of 'goto error'



> +static QCryptoAkCipherECDSAKey *qcrypto_akcipher_builtin_ecdsa_privkey_parse(
> +    const uint8_t *key, size_t keylen, Error **errp)
> +{
> +    QCryptoAkCipherECDSAKey *ecdsa = g_new0(QCryptoAkCipherECDSAKey, 1);

g_autoptr(QCryptoAkCipherECDSAKey)

and change all the 'goto error' to 'return NULL'

> +    uint8_t version;
> +    const uint8_t *seq, *pubkey;
> +    int decode_ret;
> +    size_t seq_length, pubkey_length;
> +
> +    decode_ret = qcrypto_der_decode_seq(&key, &keylen, extract_cons_content,
> +                                        &seq, errp);
> +    if (decode_ret < 0 || keylen != 0) {
> +        goto error;
> +    }
> +    seq_length = decode_ret;
> +
> +    if (qcrypto_der_decode_int(&seq, &seq_length, extract_version,
> +                               &version, errp) < 0 ||
> +        qcrypto_der_decode_octet_str(&seq, &seq_length, extract_mpi,
> +                                     &ecdsa->priv, errp) < 0) {
> +        goto error;
> +    }
> +
> +    /* Here we just ignore curve id */
> +    qcrypto_der_decode_ctx_tag(&seq, &seq_length, 0, NULL, NULL, NULL);
> +
> +    decode_ret = qcrypto_der_decode_ctx_tag(&seq, &seq_length, 1,
> +                                            extract_cons_content,
> +                                            &pubkey, NULL);
> +    if (decode_ret > 0) {
> +        pubkey_length = decode_ret;
> +        if (qcrypto_der_decode_bit_str(&pubkey, &pubkey_length,
> +                                       extract_pubkey, ecdsa, errp) < 0 ||
> +            pubkey_length != 0) {
> +            goto error;
> +        }
> +    }
> +
> +    if (seq_length != 0) {
> +        goto error;
> +    }
> +
> +    return ecdsa;

return g_steal_pointer(&ecdsa)

> +
> +error:
> +    if (errp && !*errp) {
> +        error_setg(errp, "Failed to parse ecdsa private key");
> +    }

Same note as earlier, about having this error_setg earlier
at the exact places where the relevant error condition first
occurs

> +    qcrypto_akcipher_ecdsakey_free(ecdsa);
> +    return NULL;
> +}



With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/7] crypto: Implement ECDSA algorithm by hogweed
  2022-06-13  8:45 ` [PATCH 5/7] crypto: Implement ECDSA algorithm by hogweed Lei He
@ 2022-06-17 11:35   ` Daniel P. Berrangé
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2022-06-17 11:35 UTC (permalink / raw)
  To: Lei He; +Cc: mst, arei.gonglei, qemu-devel, pizhenwei, jasowang

On Mon, Jun 13, 2022 at 04:45:29PM +0800, Lei He wrote:
> Implement ECDSA algorithm by hogweed and nettle.
> 
> Signed-off-by: lei he <helei.sig11@bytedance.com>
> ---
>  crypto/akcipher-nettle.c.inc | 268 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 268 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/7] crypto: Implement ECDSA algorithm by gcrypt
  2022-06-13  8:45 ` [PATCH 6/7] crypto: Implement ECDSA algorithm by gcrypt Lei He
@ 2022-06-17 13:41   ` Daniel P. Berrangé
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2022-06-17 13:41 UTC (permalink / raw)
  To: Lei He; +Cc: mst, arei.gonglei, qemu-devel, pizhenwei, jasowang

On Mon, Jun 13, 2022 at 04:45:30PM +0800, Lei He wrote:
> Implement ECDSA algorithm by gcrypt
> 
> Signed-off-by: lei he <helei.sig11@bytedance.com>
> ---
>  crypto/akcipher-gcrypt.c.inc | 400 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 400 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 7/7] crypto: Add test suite for ECDSA algorithm
  2022-06-13  8:45 ` [PATCH 7/7] crypto: Add test suite for ECDSA algorithm Lei He
@ 2022-06-17 13:42   ` Daniel P. Berrangé
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel P. Berrangé @ 2022-06-17 13:42 UTC (permalink / raw)
  To: Lei He; +Cc: mst, arei.gonglei, qemu-devel, pizhenwei, jasowang

On Mon, Jun 13, 2022 at 04:45:31PM +0800, Lei He wrote:
> 1. add test suite for ecdsa algorithm.
> 2. use qcrypto_akcihper_max_xxx_len to help create buffers in
> test code.
> 
> Signed-off-by: lei he <helei.sig11@bytedance.com>
> ---
>  tests/unit/test-crypto-akcipher.c | 227 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 212 insertions(+), 15 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2022-06-17 13:48 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-13  8:45 [PATCH 0/7] crypto: Introduce ECDSA algorithm Lei He
2022-06-13  8:45 ` [PATCH 1/7] crypto: Introduce ECDSA algorithm API Lei He
2022-06-14  6:51   ` Philippe Mathieu-Daudé via
2022-06-17 11:14   ` Daniel P. Berrangé
2022-06-13  8:45 ` [PATCH 2/7] crypto: Support more ASN.1 types Lei He
2022-06-17 11:20   ` Daniel P. Berrangé
2022-06-13  8:45 ` [PATCH 3/7] crypto: remove "qemu/osdep.h" in rsakey.h Lei He
2022-06-13 13:55   ` Philippe Mathieu-Daudé via
2022-06-17 11:20   ` Daniel P. Berrangé
2022-06-13  8:45 ` [PATCH 4/7] crypto: Add ECDSA key parser Lei He
2022-06-13 14:19   ` Philippe Mathieu-Daudé via
2022-06-14  1:43     ` [Phishing Risk] [External] " 何磊
2022-06-14  6:48       ` Philippe Mathieu-Daudé via
2022-06-16 16:44       ` Michael S. Tsirkin
2022-06-17 11:34   ` Daniel P. Berrangé
2022-06-13  8:45 ` [PATCH 5/7] crypto: Implement ECDSA algorithm by hogweed Lei He
2022-06-17 11:35   ` Daniel P. Berrangé
2022-06-13  8:45 ` [PATCH 6/7] crypto: Implement ECDSA algorithm by gcrypt Lei He
2022-06-17 13:41   ` Daniel P. Berrangé
2022-06-13  8:45 ` [PATCH 7/7] crypto: Add test suite for ECDSA algorithm Lei He
2022-06-17 13:42   ` Daniel P. Berrangé

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.