Some fixes for EdDSA signatures + test coverage

src/encoder.c

@@ -970,7 +970,7 @@ static int p11prov_ec_edwards_encoder_encode_text(
     P11PROV_OBJ *key = (P11PROV_OBJ *)inkey;
     CK_KEY_TYPE type;
     CK_ULONG keysize;
-    const char *type_name = "ED25519";
+    const char *type_name = ED25519;
     char *uri = NULL;
     BIO *out;
     int ret;
@@ -990,8 +990,8 @@ static int p11prov_ec_edwards_encoder_encode_text(
     }
 
     keysize = p11prov_obj_get_key_bit_size(key);
-    if (keysize == 448) {
-        type_name = "ED448";
+    if (keysize == ED448_BIT_SIZE) {
+        type_name = ED448;
     }
     if (selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) {
         CK_OBJECT_CLASS class = p11prov_obj_get_class(key);

src/objects.c

@@ -2477,6 +2477,31 @@ static int match_public_keys(P11PROV_OBJ *key1, P11PROV_OBJ *key2)
     return ret;
 }
 
+static int p11prov_obj_get_ed_nid(CK_ATTRIBUTE *ecp)
+{
+    const unsigned char *val = ecp->pValue;
+    ASN1_OBJECT *obj = d2i_ASN1_OBJECT(NULL, &val, ecp->ulValueLen);
+    if (obj) {
+        int nid = OBJ_obj2nid(obj);
+        ASN1_OBJECT_free(obj);
+        if (nid != NID_undef) {
+            return nid;
+        }
+    }
+
+    /* it might be the parameters are encoded printable string
+     * for EdDSA which OpenSSL does not understand */
+    if (ecp->ulValueLen == ED25519_EC_PARAMS_LEN
+        && memcmp(ecp->pValue, ed25519_ec_params, ED25519_EC_PARAMS_LEN) == 0) {
+        return NID_ED25519;
+    } else if (ecp->ulValueLen == ED448_EC_PARAMS_LEN
+               && memcmp(ecp->pValue, ed448_ec_params, ED448_EC_PARAMS_LEN)
+                      == 0) {
+        return NID_ED448;
+    }
+    return NID_undef;
+}
+
 int p11prov_obj_key_cmp(P11PROV_OBJ *key1, P11PROV_OBJ *key2, CK_KEY_TYPE type,
                         int cmp_type)
 {
@@ -2535,7 +2560,6 @@ int p11prov_obj_key_cmp(P11PROV_OBJ *key1, P11PROV_OBJ *key2, CK_KEY_TYPE type,
         break;
 
     case CKK_EC:
-    case CKK_EC_EDWARDS:
         ret = cmp_attr(key1, key2, CKA_EC_PARAMS);
         if (ret != RET_OSSL_OK) {
             /* If EC_PARAMS do not match it may be due to encoding.
@@ -2604,6 +2628,50 @@ int p11prov_obj_key_cmp(P11PROV_OBJ *key1, P11PROV_OBJ *key2, CK_KEY_TYPE type,
             cmp_type = OBJ_CMP_KEY_PUBLIC;
         }
         break;
+    case CKK_EC_EDWARDS:
+        /* The EdDSA params can be encoded as printable string, which is
+         * not recognized by OpenSSL and does not have respective EC_GROUP */
+        ret = cmp_attr(key1, key2, CKA_EC_PARAMS);
+        if (ret != RET_OSSL_OK) {
+            /* If EC_PARAMS do not match it may be due to encoding. */
+            CK_ATTRIBUTE *ec_p;
+            int nid1;
+            int nid2;
+
+            ec_p = p11prov_obj_get_attr(key1, CKA_EC_PARAMS);
+            if (!ec_p) {
+                return RET_OSSL_ERR;
+            }
+            nid1 = p11prov_obj_get_ed_nid(ec_p);
+            if (nid1 == NID_undef) {
+                return RET_OSSL_ERR;
+            }
+
+            ec_p = p11prov_obj_get_attr(key2, CKA_EC_PARAMS);
+            if (!ec_p) {
+                return RET_OSSL_ERR;
+            }
+            nid2 = p11prov_obj_get_ed_nid(ec_p);
+            if (nid2 == NID_undef) {
+                return RET_OSSL_ERR;
+            }
+            if (nid1 != nid2) {
+                return RET_OSSL_ERR;
+            }
+        }
+        if (cmp_type & OBJ_CMP_KEY_PRIVATE) {
+            /* unfortunately we can't really read private attributes
+             * and there is no comparison function int he PKCS11 API.
+             * Generally you do not have 2 identical keys stored in to two
+             * separate objects so the initial shortcircuit that matches if
+             * slotid/handle are identical will often cover this. When that
+             * fails we have no option but to fail for now. */
+            P11PROV_debug("We can't really match private keys");
+            /* OTOH if group and pub point match either this is a broken key
+             * or the private key must also match */
+            cmp_type = OBJ_CMP_KEY_PUBLIC;
+        }
+        break;
 
     default:
         return RET_OSSL_ERR;

src/signature.c

@@ -2289,62 +2289,33 @@ static int p11prov_eddsa_get_ctx_params(void *ctx, OSSL_PARAM *params)
 static int p11prov_eddsa_set_ctx_params(void *ctx, const OSSL_PARAM params[])
 {
     P11PROV_SIG_CTX *sigctx = (P11PROV_SIG_CTX *)ctx;
+    const char *instance = "Ed25519";
     const OSSL_PARAM *p;
+    CK_ULONG size;
+    bool matched = false;
     int ret;
 
     P11PROV_debug("eddsa set ctx params (ctx=%p, params=%p)", sigctx, params);
 
-    if (params == NULL) {
-        return RET_OSSL_OK;
+    size = p11prov_obj_get_key_bit_size(sigctx->key);
+    if (size != ED25519_BIT_SIZE && size != ED448_BIT_SIZE) {
+        P11PROV_raise(sigctx->provctx, CKR_KEY_TYPE_INCONSISTENT,
+                      "Invalid EdDSA key size %lu", size);
+        return RET_OSSL_ERR;
+    }
+
+    /* PKCS #11 parameters are mandatory for Ed448 key type anyway */
+    if (size == ED448_BIT_SIZE) {
+        instance = "Ed448";
     }
 
     p = OSSL_PARAM_locate_const(params, OSSL_SIGNATURE_PARAM_INSTANCE);
     if (p) {
-        const char *instance = NULL;
-        bool matched = false;
-        CK_ULONG size;
-
         ret = OSSL_PARAM_get_utf8_string_ptr(p, &instance);
         if (ret != RET_OSSL_OK) {
             return ret;
         }
         P11PROV_debug("Set OSSL_SIGNATURE_PARAM_INSTANCE to %s", instance);
-
-        size = p11prov_obj_get_key_bit_size(sigctx->key);
-        if (size != ED25519_BIT_SIZE && size != ED448_BIT_SIZE) {
-            P11PROV_raise(sigctx->provctx, CKR_KEY_TYPE_INCONSISTENT,
-                          "Invalid EdDSA key size %lu", size);
-            return RET_OSSL_ERR;
-        }
-        if (size == ED25519_BIT_SIZE) {
-            if (OPENSSL_strcasecmp(instance, "Ed25519") == 0) {
-                matched = true;
-                sigctx->use_eddsa_params = CK_FALSE;
-            } else if (OPENSSL_strcasecmp(instance, "Ed25519ph") == 0) {
-                matched = true;
-                sigctx->use_eddsa_params = CK_TRUE;
-                sigctx->eddsa_params.phFlag = CK_TRUE;
-            } else if (OPENSSL_strcasecmp(instance, "Ed25519ctx") == 0) {
-                matched = true;
-                sigctx->use_eddsa_params = CK_TRUE;
-                sigctx->eddsa_params.phFlag = CK_FALSE;
-            }
-        } else if (size == ED448_BIT_SIZE) {
-            if (OPENSSL_strcasecmp(instance, "Ed448") == 0) {
-                matched = true;
-                sigctx->use_eddsa_params = CK_TRUE;
-                sigctx->eddsa_params.phFlag = CK_FALSE;
-            } else if (OPENSSL_strcasecmp(instance, "Ed448ph") == 0) {
-                matched = true;
-                sigctx->use_eddsa_params = CK_TRUE;
-                sigctx->eddsa_params.phFlag = CK_TRUE;
-            }
-        }
-        if (!matched) {
-            P11PROV_raise(sigctx->provctx, CKR_ARGUMENTS_BAD,
-                          "Invalid instance");
-            return RET_OSSL_ERR;
-        }
     }
 
     p = OSSL_PARAM_locate_const(params, OSSL_SIGNATURE_PARAM_CONTEXT_STRING);
@@ -2361,6 +2332,36 @@ static int p11prov_eddsa_set_ctx_params(void *ctx, const OSSL_PARAM params[])
         sigctx->eddsa_params.ulContextDataLen = datalen;
     }
 
+    if (size == ED25519_BIT_SIZE) {
+        if (OPENSSL_strcasecmp(instance, "Ed25519") == 0) {
+            matched = true;
+            sigctx->use_eddsa_params = CK_FALSE;
+        } else if (OPENSSL_strcasecmp(instance, "Ed25519ph") == 0) {
+            matched = true;
+            sigctx->use_eddsa_params = CK_TRUE;
+            sigctx->eddsa_params.phFlag = CK_TRUE;
+        } else if (OPENSSL_strcasecmp(instance, "Ed25519ctx") == 0) {
+            matched = true;
+            sigctx->use_eddsa_params = CK_TRUE;
+            sigctx->eddsa_params.phFlag = CK_FALSE;
+        }
+    } else if (size == ED448_BIT_SIZE) {
+        if (OPENSSL_strcasecmp(instance, "Ed448") == 0) {
+            matched = true;
+            sigctx->use_eddsa_params = CK_TRUE;
+            sigctx->eddsa_params.phFlag = CK_FALSE;
+        } else if (OPENSSL_strcasecmp(instance, "Ed448ph") == 0) {
+            matched = true;
+            sigctx->use_eddsa_params = CK_TRUE;
+            sigctx->eddsa_params.phFlag = CK_TRUE;
+        }
+    }
+    if (!matched) {
+        P11PROV_raise(sigctx->provctx, CKR_ARGUMENTS_BAD, "Invalid instance %s",
+                      instance);
+        return RET_OSSL_ERR;
+    }
+
     return RET_OSSL_OK;
 }
 

tests/setup.sh

@@ -232,34 +232,38 @@ if [ "${TOKENTYPE}" != "softokn" ]; then
     echo "${EDPUBURI}"
     echo "${EDPRIURI}"
     echo "${EDCRTURI}"
-fi
 
-# FIXME The pkcs11-tool before OpenSC 0.26 does not support Ed448 so they can
-# not be generated here
-#
-# generate ED448
-#KEYID='0009'
-#URIKEYID="%00%09"
-#ED2CRTN="ed2Cert"
-#
-# pkcs11-tool "${P11DEFARGS[@]}" --keypairgen --key-type="EC:edwards448" \
-# 	--label="${ED2CRTN}" --id="$KEYID"
-# ca_sign $ED2CRTN "My ED448 Cert" $KEYID
-#
-# ED2BASEURIWITHPINVALUE="pkcs11:id=${URIKEYID};pin-value=${PINVALUE}"
-# ED2BASEURIWITHPINSOURCE="pkcs11:id=${URIKEYID};pin-source=file:${PINFILE}"
-# ED2BASEURI="pkcs11:id=${URIKEYID}"
-# ED2PUBURI="pkcs11:type=public;id=${URIKEYID}"
-# ED2PRIURI="pkcs11:type=private;id=${URIKEYID}"
-# ED2CRTURI="pkcs11:type=cert;object=${ED2CRTN}"
-#
-# title LINE "ED448 PKCS11 URIS"
-# echo "${EDBASEURIWITHPINVALUE}"
-# echo "${EDBASEURIWITHPINSOURCE}"
-# echo "${EDBASEURI}"
-# echo "${EDPUBURI}"
-# echo "${EDPRIURI}"
-# echo "${EDCRTURI}"
+    # this requires OpenSC 0.26.0, which is not available in Ubuntu and CentOS 9
+    if [[ -f /etc/debian_version ]] && grep Ubuntu /etc/lsb-release; then
+        echo "Ed448 not supported in Ubuntu's OpenSC version"
+    elif [[ -f /etc/redhat-release ]] && grep "release 9" /etc/redhat-release; then
+        echo "Ed448 not supported in EL9's OpenSC version"
+    else
+        # generate ED448
+        KEYID='0009'
+        URIKEYID="%00%09"
+        ED2CRTN="ed2Cert"
+
+        pkcs11-tool "${P11DEFARGS[@]}" --keypairgen --key-type="EC:Ed448" \
+            --label="${ED2CRTN}" --id="$KEYID"
+        ca_sign $ED2CRTN "My ED448 Cert" $KEYID
+
+        ED2BASEURIWITHPINVALUE="pkcs11:id=${URIKEYID};pin-value=${PINVALUE}"
+        ED2BASEURIWITHPINSOURCE="pkcs11:id=${URIKEYID};pin-source=file:${PINFILE}"
+        ED2BASEURI="pkcs11:id=${URIKEYID}"
+        ED2PUBURI="pkcs11:type=public;id=${URIKEYID}"
+        ED2PRIURI="pkcs11:type=private;id=${URIKEYID}"
+        ED2CRTURI="pkcs11:type=cert;object=${ED2CRTN}"
+
+        title LINE "ED448 PKCS11 URIS"
+        echo "${ED2BASEURIWITHPINVALUE}"
+        echo "${ED2BASEURIWITHPINSOURCE}"
+        echo "${ED2BASEURI}"
+        echo "${ED2PUBURI}"
+        echo "${ED2PRIURI}"
+        echo "${ED2CRTURI}"
+    fi
+fi
 
 title PARA "generate RSA key pair, self-signed certificate, remove public key"
 KEYID='0005'
@@ -454,6 +458,18 @@ export EDCRTURI="${EDCRTURI}"
 DBGSCRIPT
 fi
 
+if [ -n "${ED2BASEURI}" ]; then
+    cat >> "${TMPPDIR}/testvars" <<DBGSCRIPT
+
+export ED2BASEURIWITHPINVALUE="${ED2BASEURIWITHPINVALUE}"
+export ED2BASEURIWITHPINSOURCE="${ED2BASEURIWITHPINSOURCE}"
+export ED2BASEURI="${ED2BASEURI}"
+export ED2PUBURI="${ED2PUBURI}"
+export ED2PRIURI="${ED2PRIURI}"
+export ED2CRTURI="${ED2CRTURI}"
+DBGSCRIPT
+fi
+
 if [ -n "${ECXBASEURI}" ]; then
     cat >> "${TMPPDIR}/testvars" <<DBGSCRIPT
 

tests/tedwards

@@ -103,9 +103,9 @@ if [[ -n $ED2BASEURI ]]; then
 
     title PARA "Test EVP_PKEY_eq on public ED448 key via import"
     $CHECKER "${TESTBLDDIR}/tcmpkeys" "$ED2PUBURI" "${TMPPDIR}"/ed2out.pub
-    title PARA "Match private ED key against public key"
+    title PARA "Match private ED448 key against public key"
     $CHECKER "${TESTBLDDIR}/tcmpkeys" "$ED2PRIURI" "${TMPPDIR}"/ed2out.pub
-    title PARA "Match private ED key against public key (commutativity)"
+    title PARA "Match private ED448 key against public key (commutativity)"
     $CHECKER "${TESTBLDDIR}/tcmpkeys" "${TMPPDIR}"/ed2out.pub "$ED2PRIURI"
 fi
 
@@ -121,4 +121,34 @@ if [ $FAIL -ne 0 ]; then
     exit 1
 fi
 
+ORIG_OPENSSL_CONF=${OPENSSL_CONF}
+sed "s/^pkcs11-module-token-pin.*$/##nopin/" "${OPENSSL_CONF}" > "${OPENSSL_CONF}.nopin"
+OPENSSL_CONF=${OPENSSL_CONF}.nopin
+
+title PARA "Test interactive Login on key without ALWAYS AUTHENTICATE"
+# shellcheck disable=SC2153 # It is correctly defined in the testvars
+output=$(expect -c "spawn -noecho $CHECKER ${TESTBLDDIR}/tsession \"$EDBASEURI\";
+                expect \"Enter PIN for PKCS#11 Token (Slot *:\" {
+                    send \"${PINVALUE}\r\"; exp_continue; }
+                expect \"ALL A-OK\";")
+FAIL=0
+echo "$output" | grep "Enter PIN for PKCS#11 Token (Slot .*):" > /dev/null 2>&1 || FAIL=1
+prompts=$(echo "$output" | grep -c "Enter PIN for PKCS#11 Token (Slot .*):" 2>&1)
+# 1 login to read key only
+if [ "$prompts" -ne "1" ]; then
+    echo "Failed receive expected amount of prompts (got $prompts, expected 1)"
+    FAIL=2
+fi
+if [ $FAIL -eq 1 ]; then
+    echo "Failed to obtain expected prompt"
+fi
+if [ $FAIL -ne 0 ]; then
+    echo
+    echo "Original command output:"
+    echo "$output"
+    echo
+    exit 1
+fi
+OPENSSL_CONF=${ORIG_OPENSSL_CONF}
+
 exit 0