Repository: randombit/botan
Pull request #3702 (2023-09-15 10:46:27)

Add some minimal API sanity checks for PK_Signer/PK_Verifier
Add some minimal API sanity checks for the `PK_Signer`/`PK_Verifier` that are not covered by other general public key tests.

The main motivation behind this is to detect use after free problems in case the `PK_Signer`/`PK_Verifier` wrongly only hold a reference to the keys.
Bellow is a table of affected classes. Together with a status if it was detected in the CI by the sanitizers/valgrind, and if it is fixed in this PR.

Possible use-after-free:

|                 class                 | sanitizers |    valgrind    | fixed |
| ------------------------------------- | ---------- | -------------- | ----- |
| `Dilithium_Signature_Operation`       | ✔          | ✔              | ✔     |
| `ECDSA_Signature_Operation`           | ✔          | ✔              | ✔     |
| `ECGDSA_Signature_Operation`          | ✔          | ✔              | ✔     |
| `ECKCDSA_Signature_Operation`         | ✔          | ✔              | ✔     |
| `GOST_3410_Signature_Operation`       | ✔          | ✔              | ✔     |
| `PKCS11_ECDSA_Signature_Operation`    | ❌          | ❌ (no PKCS#11) | ✔     |
| `PKCS11_RSA_Signature_Operation`      | ❌          | ❌ (no PKCS#11) | ✔     |
| `SM2_Signature_Operation`             | ✔          | ✔              | ✔     |
| `TPM_Signing_Operation`               | ❌          | ❌ (no TPM)     | ✔     |
| `XMSS_PrivateKey_Internal`            | ✔          | ❌ (skipped)    | ✔     |
| `Ed25519_Pure_Verify_Operation`       | ✔          | ❌ (skipped)    | ✔     |
| `Ed25519_Hashed_Verify_Operation`     | ✔          | ❌ (skipped)    | ✔     |
| `PKCS11_ECDSA_Verification_Operation` | ❌          | ❌ (no PKCS#11) | ✔     |
| `PKCS11_RSA_Verification_Operation`   | ❌          | ❌ (no PKCS#11) | ✔     |
| `XMSS_Verification_Operation`         | ✔          | ❌ (skipped)    | ✔     |

Possible null pointer dereferencing:

|            class             | sanitizers | valgrind | fixed |
| ---------------------------- | ---------- | -------- | ----- |
| `SM2_Signature_Operation`    | ✔          | ✔        | ✔     |
| `SM2_Verification_Operation` | ✔          | ✔        | ✔     |

Also added a note to the documentation that for PKCS#11 the session needs to remain alive as long as any object using it is still alive.

lieser: Open point for discussion: I'm unsure if it is a good idea to add the new tests to the `PK_Signature_Generation_Test` test, or if it would not be better to add a new general test for it.

Advantage for `PK_Signature_Generation_Test`:

- The test are automatically added form most public key algorithms.

Advantage for a separate test:

- Can be reused for algorithms that do not have a `PK_Signature_Generation_Test` test.
- Would probably be enough to run the new test once per algorithm instead of for each KAT test.
randombit: I would suggest a new test for this.

My thinking right now would be to use `Text_Based_Test` that has a format along the lines of

```
Algorithm = RSA
AlgoParams = 2048
SigParams = PSSR(SHA-256)

Algorithm = ECDSA
AlgoParams = secp256k1
SigParams = SHA-256
...
```

which then does the basic test that you have written here but using `create_private_key(Algorithm, AlgoParams)` then creates a `PK_Signer`/`PK_Verifier` using `SigParams`.

We have to remember to update the test each time a new algorithm is added but that isn't that commonly done, and it avoids running the same test with each KAT which seems somewhat excessive.

A similar test could/should be written for the encryptor and key agreement `pubkey.h` types
randombit: The ideal fix here would be, as with RSA and DH already, for the keys to be stored internally as a `shared_ptr` and then the operations objects can just capture the key information that way. I started on changing this for the EC objects prior to 3.0 and never got it over the line; maybe I'll take this up again next week.
lieser: > My thinking right now would be to use Text_Based_Test that has a format along the lines of [...]

With a separate test I was thinking about creating a base class that all algorithms then have to derive from, similar too `PK_Signature_Generation_Test`.
But I like your suggestion, and will try to find time this week to rewrite it as a single `Text_Based_Test`.

Writing something similar for the encryptor and key agreement is probably a good idea. At least `PKCS11_RSA_Encryption_Operation` seems to have the same problem. But I will probably leave it as a follow up.

> The ideal fix here would be, as with RSA and DH already, for the keys to be stored internally as a shared_ptr and then the operations objects can just capture the key information that way.

Yeah I saw that some algorithms go this way. And if the key data is big enough that we don't want to copy it around it is probably the way to go long term.
For this PR I will however keep the fixes as simple as possible if that is OK.
randombit: > Yeah I saw that some algorithms go this way. And if the key data is big enough that we don't want to copy it around it is probably the way to go long term.
For this PR I will however keep the fixes as simple as possible if that is OK.

For now just capturing by value as you're doing is absolutely fine, thanks for looking into this.
randombit: Sorry stray mouse click there
lieser: Reworked the test to be a single `Text_Based_Test` test. Most classes are now fixed, only the PKCS11 and TPM related ones are still missing. Have to see if/how we want to test that with sanitizers in the CI too.

A note about PKCS11:
There all objects hold a reference to a `Session`. This is also a potential for memory problems, but probably not something we want to change for now. That seems to be to integrated into the PKCS11 code.
But at least at a glance I also did not see a big warning in the manual that the user has to ensure that a `Session` exists as long a any of the associated object still exists.
coveralls: 
[![Coverage Status](https://coveralls.io/builds/62777515/badge)](https://coveralls.io/builds/62777515)

coverage: 91.763% (+0.01%) from 91.749% when pulling **8d8f447ffaa9d8caa12ae67467588f80ff50c6f3 on pl/fix_key_lifetime** into **1f26e75d781745a7dbead57f10e0d31077a79709 on master**.

randombit: @lieser Is this ready for review? Even if it does not completely address all the issues, what you have already seems an improvement so it would be good to merge this for 3.2 if we can.

Patch:
diff --git a/doc/api_ref/pkcs11.rst b/doc/api_ref/pkcs11.rst
index 8d176ade3..4bfde4d00 100644
--- a/doc/api_ref/pkcs11.rst
+++ b/doc/api_ref/pkcs11.rst
@@ -242,6 +242,12 @@ The :cpp:class:`Session` class represents a PKCS#11 session and is defined in ``
 
 A session is a logical connection between an application and a token.
 
+The session is passed to most other PKCS#11 operations, and must remain alive
+as long as any other PKCS#11 object which the session was passed to is still
+alive, otherwise errors or even an application crash are possible.
+In the future,
+the API may change to using ``shared_ptr`` to remove this problem.
+
 .. cpp:class:: Session
 
    There are two constructors to create a new session and one constructor to
diff --git a/src/lib/prov/pkcs11/p11_ecdsa.cpp b/src/lib/prov/pkcs11/p11_ecdsa.cpp
index faa441deb..3fc4589c0 100644
--- a/src/lib/prov/pkcs11/p11_ecdsa.cpp
+++ b/src/lib/prov/pkcs11/p11_ecdsa.cpp
@@ -53,7 +53,7 @@ namespace {
 
 class PKCS11_ECDSA_Signature_Operation final : public PK_Ops::Signature {
    public:
-      PKCS11_ECDSA_Signature_Operation(const PKCS11_EC_PrivateKey& key, std::string_view hash) :
+      PKCS11_ECDSA_Signature_Operation(const PKCS11_ECDSA_PrivateKey& key, std::string_view hash) :
             PK_Ops::Signature(),
             m_key(key),
             m_order(key.domain().get_order()),
@@ -99,8 +99,8 @@ class PKCS11_ECDSA_Signature_Operation final : public PK_Ops::Signature {
       std::string hash_function() const override { return m_hash; }
 
    private:
-      const PKCS11_EC_PrivateKey& m_key;
-      const BigInt& m_order;
+      const PKCS11_ECDSA_PrivateKey m_key;
+      const BigInt m_order;
       MechanismWrapper m_mechanism;
       const std::string m_hash;
       secure_vector<uint8_t> m_first_message;
@@ -115,7 +115,7 @@ AlgorithmIdentifier PKCS11_ECDSA_Signature_Operation::algorithm_identifier() con
 
 class PKCS11_ECDSA_Verification_Operation final : public PK_Ops::Verification {
    public:
-      PKCS11_ECDSA_Verification_Operation(const PKCS11_EC_PublicKey& key, std::string_view hash) :
+      PKCS11_ECDSA_Verification_Operation(const PKCS11_ECDSA_PublicKey& key, std::string_view hash) :
             PK_Ops::Verification(),
             m_key(key),
             m_order(key.domain().get_order()),
@@ -165,8 +165,8 @@ class PKCS11_ECDSA_Verification_Operation final : public PK_Ops::Verification {
       std::string hash_function() const override { return m_hash; }
 
    private:
-      const PKCS11_EC_PublicKey& m_key;
-      const BigInt& m_order;
+      const PKCS11_ECDSA_PublicKey m_key;
+      const BigInt m_order;
       MechanismWrapper m_mechanism;
       const std::string m_hash;
       secure_vector<uint8_t> m_first_message;
diff --git a/src/lib/prov/pkcs11/p11_rsa.cpp b/src/lib/prov/pkcs11/p11_rsa.cpp
index ba6b79c1f..9773e8e1b 100644
--- a/src/lib/prov/pkcs11/p11_rsa.cpp
+++ b/src/lib/prov/pkcs11/p11_rsa.cpp
@@ -249,7 +249,7 @@ class PKCS11_RSA_Signature_Operation final : public PK_Ops::Signature {
       AlgorithmIdentifier algorithm_identifier() const override;
 
    private:
-      const PKCS11_RSA_PrivateKey& m_key;
+      PKCS11_RSA_PrivateKey m_key;
       bool m_initialized = false;
       secure_vector<uint8_t> m_first_message;
       MechanismWrapper m_mechanism;
@@ -370,7 +370,7 @@ class PKCS11_RSA_Verification_Operation final : public PK_Ops::Verification {
       std::string hash_function() const override;
 
    private:
-      const PKCS11_RSA_PublicKey& m_key;
+      const PKCS11_RSA_PublicKey m_key;
       bool m_initialized = false;
       secure_vector<uint8_t> m_first_message;
       MechanismWrapper m_mechanism;
diff --git a/src/lib/prov/tpm/tpm.cpp b/src/lib/prov/tpm/tpm.cpp
index 23438f4af..b8d9d537e 100644
--- a/src/lib/prov/tpm/tpm.cpp
+++ b/src/lib/prov/tpm/tpm.cpp
@@ -360,7 +360,7 @@ class TPM_Signing_Operation final : public PK_Ops::Signature {
       }
 
    private:
-      const TPM_PrivateKey& m_key;
+      const TPM_PrivateKey m_key;
       std::unique_ptr<HashFunction> m_hash;
       std::vector<uint8_t> m_hash_id;
 };
diff --git a/src/lib/pubkey/dilithium/dilithium_common/dilithium.cpp b/src/lib/pubkey/dilithium/dilithium_common/dilithium.cpp
index 21f6c45a4..f2668970a 100644
--- a/src/lib/pubkey/dilithium/dilithium_common/dilithium.cpp
+++ b/src/lib/pubkey/dilithium/dilithium_common/dilithium.cpp
@@ -406,7 +406,7 @@ class Dilithium_Signature_Operation final : public PK_Ops::Signature {
          return sig;
       }
 
-      const Dilithium_PrivateKey& m_priv_key;
+      const Dilithium_PrivateKey m_priv_key;
       const Dilithium::PolynomialMatrix m_matrix;
       SHAKE_256 m_shake;
       bool m_randomized;
diff --git a/src/lib/pubkey/ecdsa/ecdsa.cpp b/src/lib/pubkey/ecdsa/ecdsa.cpp
index d8cc9d874..be1f10d23 100644
--- a/src/lib/pubkey/ecdsa/ecdsa.cpp
+++ b/src/lib/pubkey/ecdsa/ecdsa.cpp
@@ -134,7 +134,7 @@ class ECDSA_Signature_Operation final : public PK_Ops::Signature_with_Hash {
 
    private:
       const EC_Group m_group;
-      const BigInt& m_x;
+      const BigInt m_x;
 
 #if defined(BOTAN_HAS_RFC6979_GENERATOR)
       std::unique_ptr<RFC6979_Nonce_Generator> m_rfc6979;
diff --git a/src/lib/pubkey/ecgdsa/ecgdsa.cpp b/src/lib/pubkey/ecgdsa/ecgdsa.cpp
index fe959e4e5..73aca08f3 100644
--- a/src/lib/pubkey/ecgdsa/ecgdsa.cpp
+++ b/src/lib/pubkey/ecgdsa/ecgdsa.cpp
@@ -49,7 +49,7 @@ class ECGDSA_Signature_Operation final : public PK_Ops::Signature_with_Hash {
 
    private:
       const EC_Group m_group;
-      const BigInt& m_x;
+      const BigInt m_x;
       std::vector<BigInt> m_ws;
 };
 
diff --git a/src/lib/pubkey/eckcdsa/eckcdsa.cpp b/src/lib/pubkey/eckcdsa/eckcdsa.cpp
index 7decbe701..d1443d4c0 100644
--- a/src/lib/pubkey/eckcdsa/eckcdsa.cpp
+++ b/src/lib/pubkey/eckcdsa/eckcdsa.cpp
@@ -154,7 +154,7 @@ class ECKCDSA_Signature_Operation final : public PK_Ops::Signature {
       secure_vector<uint8_t> raw_sign(const uint8_t msg[], size_t msg_len, RandomNumberGenerator& rng);
 
       const EC_Group m_group;
-      const BigInt& m_x;
+      const BigInt m_x;
       std::unique_ptr<HashFunction> m_hash;
       std::vector<uint8_t> m_prefix;
       std::vector<BigInt> m_ws;
diff --git a/src/lib/pubkey/ed25519/ed25519_key.cpp b/src/lib/pubkey/ed25519/ed25519_key.cpp
index 87d261b7b..1d63b7b69 100644
--- a/src/lib/pubkey/ed25519/ed25519_key.cpp
+++ b/src/lib/pubkey/ed25519/ed25519_key.cpp
@@ -134,7 +134,7 @@ namespace {
 */
 class Ed25519_Pure_Verify_Operation final : public PK_Ops::Verification {
    public:
-      explicit Ed25519_Pure_Verify_Operation(const Ed25519_PublicKey& key) : m_key(key) {}
+      explicit Ed25519_Pure_Verify_Operation(const Ed25519_PublicKey& key) : m_key(key.get_public_key()) {}
 
       void update(const uint8_t msg[], size_t msg_len) override { m_msg.insert(m_msg.end(), msg, msg + msg_len); }
 
@@ -143,9 +143,8 @@ class Ed25519_Pure_Verify_Operation final : public PK_Ops::Verification {
             return false;
          }
 
-         const std::vector<uint8_t>& pub_key = m_key.get_public_key();
-         BOTAN_ASSERT_EQUAL(pub_key.size(), 32, "Expected size");
-         const bool ok = ed25519_verify(m_msg.data(), m_msg.size(), sig, pub_key.data(), nullptr, 0);
+         BOTAN_ASSERT_EQUAL(m_key.size(), 32, "Expected size");
+         const bool ok = ed25519_verify(m_msg.data(), m_msg.size(), sig, m_key.data(), nullptr, 0);
          m_msg.clear();
          return ok;
       }
@@ -154,7 +153,7 @@ class Ed25519_Pure_Verify_Operation final : public PK_Ops::Verification {
 
    private:
       std::vector<uint8_t> m_msg;
-      const Ed25519_PublicKey& m_key;
+      std::vector<uint8_t> m_key;
 };
 
 /**
@@ -162,7 +161,8 @@ class Ed25519_Pure_Verify_Operation final : public PK_Ops::Verification {
 */
 class Ed25519_Hashed_Verify_Operation final : public PK_Ops::Verification {
    public:
-      Ed25519_Hashed_Verify_Operation(const Ed25519_PublicKey& key, std::string_view hash, bool rfc8032) : m_key(key) {
+      Ed25519_Hashed_Verify_Operation(const Ed25519_PublicKey& key, std::string_view hash, bool rfc8032) :
+            m_key(key.get_public_key()) {
          m_hash = HashFunction::create_or_throw(hash);
 
          if(rfc8032) {
@@ -181,17 +181,16 @@ class Ed25519_Hashed_Verify_Operation final : public PK_Ops::Verification {
          std::vector<uint8_t> msg_hash(m_hash->output_length());
          m_hash->final(msg_hash.data());
 
-         const std::vector<uint8_t>& pub_key = m_key.get_public_key();
-         BOTAN_ASSERT_EQUAL(pub_key.size(), 32, "Expected size");
+         BOTAN_ASSERT_EQUAL(m_key.size(), 32, "Expected size");
          return ed25519_verify(
-            msg_hash.data(), msg_hash.size(), sig, pub_key.data(), m_domain_sep.data(), m_domain_sep.size());
+            msg_hash.data(), msg_hash.size(), sig, m_key.data(), m_domain_sep.data(), m_domain_sep.size());
       }
 
       std::string hash_function() const override { return m_hash->name(); }
 
    private:
       std::unique_ptr<HashFunction> m_hash;
-      const Ed25519_PublicKey& m_key;
+      std::vector<uint8_t> m_key;
       std::vector<uint8_t> m_domain_sep;
 };
 
diff --git a/src/lib/pubkey/gost_3410/gost_3410.cpp b/src/lib/pubkey/gost_3410/gost_3410.cpp
index 7ee36e924..a17e5a1e9 100644
--- a/src/lib/pubkey/gost_3410/gost_3410.cpp
+++ b/src/lib/pubkey/gost_3410/gost_3410.cpp
@@ -137,7 +137,7 @@ class GOST_3410_Signature_Operation final : public PK_Ops::Signature_with_Hash {
 
    private:
       const EC_Group m_group;
-      const BigInt& m_x;
+      const BigInt m_x;
       std::vector<BigInt> m_ws;
 };
 
diff --git a/src/lib/pubkey/sm2/sm2.cpp b/src/lib/pubkey/sm2/sm2.cpp
index b88828de5..8d4c47b52 100644
--- a/src/lib/pubkey/sm2/sm2.cpp
+++ b/src/lib/pubkey/sm2/sm2.cpp
@@ -108,12 +108,12 @@ class SM2_Signature_Operation final : public PK_Ops::Signature {
 
       secure_vector<uint8_t> sign(RandomNumberGenerator& rng) override;
 
-      std::string hash_function() const override { return m_hash->name(); }
+      std::string hash_function() const override { return m_hash ? m_hash->name() : "Raw"; }
 
    private:
       const EC_Group m_group;
-      const BigInt& m_x;
-      const BigInt& m_da_inv;
+      const BigInt m_x;
+      const BigInt m_da_inv;
 
       std::vector<uint8_t> m_za;
       secure_vector<uint8_t> m_digest;
@@ -167,7 +167,7 @@ class SM2_Verification_Operation final : public PK_Ops::Verification {
 
       bool is_valid_signature(const uint8_t sig[], size_t sig_len) override;
 
-      std::string hash_function() const override { return m_hash->name(); }
+      std::string hash_function() const override { return m_hash ? m_hash->name() : "Raw"; }
 
    private:
       const EC_Group m_group;
diff --git a/src/lib/pubkey/xmss/xmss_privatekey.cpp b/src/lib/pubkey/xmss/xmss_privatekey.cpp
index 3c3ac5535..37b8e30cd 100644
--- a/src/lib/pubkey/xmss/xmss_privatekey.cpp
+++ b/src/lib/pubkey/xmss/xmss_privatekey.cpp
@@ -189,8 +189,8 @@ class XMSS_PrivateKey_Internal {
       }
 
    private:
-      const XMSS_Parameters& m_xmss_params;
-      const XMSS_WOTS_Parameters& m_wots_params;
+      XMSS_Parameters m_xmss_params;
+      XMSS_WOTS_Parameters m_wots_params;
       WOTS_Derivation_Method m_wots_derivation_method;
 
       XMSS_Hash m_hash;
diff --git a/src/lib/pubkey/xmss/xmss_verification_operation.h b/src/lib/pubkey/xmss/xmss_verification_operation.h
index bcb9f2c5f..4ff02510c 100644
--- a/src/lib/pubkey/xmss/xmss_verification_operation.h
+++ b/src/lib/pubkey/xmss/xmss_verification_operation.h
@@ -58,7 +58,7 @@ class XMSS_Verification_Operation final : public virtual PK_Ops::Verification {
        **/
       bool verify(const XMSS_Signature& sig, const secure_vector<uint8_t>& msg, const XMSS_PublicKey& pub_key);
 
-      const XMSS_PublicKey& m_pub_key;
+      const XMSS_PublicKey m_pub_key;
       XMSS_Hash m_hash;
       secure_vector<uint8_t> m_msg_buf;
 };


Test patch:
diff --git a/src/tests/data/pubkey/api_sign.vec b/src/tests/data/pubkey/api_sign.vec
new file mode 100644
index 000000000..175c73e6f
--- /dev/null
+++ b/src/tests/data/pubkey/api_sign.vec
@@ -0,0 +1,73 @@
+# [Dilithium]
+
+Algorithm = Dilithium
+AlgoParams = Dilithium-4x4-r3
+SigParams =
+
+# [DSA]
+
+Algorithm = DSA
+AlgoParams = dsa/jce/1024
+SigParams = SHA-256
+
+# [ECDSA]
+
+Algorithm = ECDSA
+AlgoParams = secp256k1
+SigParams = SHA-256
+
+# [ECGDSA]
+
+Algorithm = ECGDSA
+AlgoParams = secp256r1
+SigParams = SHA-256
+
+# [ECKCDSA]
+
+Algorithm = ECKCDSA
+AlgoParams = secp256r1
+SigParams = SHA-256
+
+# [Ed25519]
+
+Algorithm = Ed25519
+AlgoParams =
+SigParams = Pure
+
+Algorithm = Ed25519
+AlgoParams =
+SigParams = Ed25519ph
+
+# [GOST-34.10]
+
+Algorithm = GOST-34.10
+AlgoParams = gost_256A
+SigParams = Raw
+
+# [RSA]
+
+Algorithm = RSA
+AlgoParams = 2048
+SigParams = PSSR(SHA-256)
+
+# [SM2]
+
+Algorithm = SM2
+AlgoParams = secp256r1
+SigParams = ALICE123@YAHOO.COM,SM3
+
+Algorithm = SM2
+AlgoParams = secp256r1
+SigParams = ALICE123@YAHOO.COM,Raw
+
+# [SPHINCS+]
+
+Algorithm = SPHINCS+
+AlgoParams = SphincsPlus-shake-128s-r3.1
+SigParams =
+
+# [XMSS]
+
+Algorithm = XMSS
+AlgoParams = XMSS-SHA2_10_256
+SigParams = SHA2_10_256
diff --git a/src/tests/test_pubkey.cpp b/src/tests/test_pubkey.cpp
index 0b293af43..37850fe71 100644
--- a/src/tests/test_pubkey.cpp
+++ b/src/tests/test_pubkey.cpp
@@ -17,11 +17,14 @@
    #include <botan/pkcs8.h>
    #include <botan/pubkey.h>
    #include <botan/x509_key.h>
+   #include <botan/internal/fmt.h>
 
    #if defined(BOTAN_HAS_HMAC_DRBG)
       #include <botan/hmac_drbg.h>
    #endif
 
+   #include <array>
+
 namespace Botan_Tests {
 
 void check_invalid_signatures(Test::Result& result,
@@ -113,7 +116,7 @@ Test::Result PK_Signature_Generation_Test::run_one_test(const std::string& pad_h
    auto pubkey = Botan::X509::load_key(Botan::X509::BER_encode(*privkey));
 
    result.confirm("public key claims to support signatures",
-                  privkey->supports_operation(Botan::PublicKeyOperation::Signature));
+                  pubkey->supports_operation(Botan::PublicKeyOperation::Signature));
 
    std::vector<std::unique_ptr<Botan::PK_Verifier>> verifiers;
 
@@ -742,6 +745,72 @@ Test::Result PK_Key_Generation_Stability_Test::run_one_test(const std::string&,
    return result;
 }
 
+/**
+ * @brief Some general tests for minimal API sanity for signing/verification.
+ */
+class PK_API_Sign_Test : public Text_Based_Test {
+   public:
+      PK_API_Sign_Test() : Text_Based_Test("pubkey/api_sign.vec", "Algorithm,AlgoParams,SigParams", "Provider") {}
+
+   protected:
+      Test::Result run_one_test(const std::string&, const VarMap& vars) final {
+         const std::string algorithm = vars.get_req_str("Algorithm");
+         const std::string algo_params = vars.get_req_str("AlgoParams");
+         const std::string sig_params = vars.get_req_str("SigParams");
+         const std::string verify_params = vars.get_opt_str("VerifyParams", sig_params);
+         const std::string provider = vars.get_opt_str("Provider", "base");
+
+         std::ostringstream test_name;
+         test_name << "Sign/verify API tests " << algorithm;
+         if(!algo_params.empty()) {
+            test_name << '(' << algo_params << ')';
+         }
+         if(!sig_params.empty()) {
+            test_name << '/' << sig_params;
+         }
+         Test::Result result(test_name.str());
+
+         std::unique_ptr<Botan::Private_Key> privkey =
+            Botan::create_private_key(algorithm, Test::rng(), algo_params, provider);
+         if(!privkey) {
+            result.test_note(Botan::fmt(
+               "Skipping Sign/verify API tests for {}({}) with provider {}", algorithm, algo_params, provider));
+            return result;
+         }
+         std::unique_ptr<Botan::Public_Key> pubkey = Botan::X509::load_key(Botan::X509::BER_encode(*privkey));
+         result.confirm("Storing and loading public key works", pubkey != nullptr);
+
+         result.confirm("private key claims to support signatures",
+                        privkey->supports_operation(Botan::PublicKeyOperation::Signature));
+         result.confirm("public key claims to support signatures",
+                        pubkey->supports_operation(Botan::PublicKeyOperation::Signature));
+         result.test_gt("Public key length must be greater than 0", privkey->key_length(), 0);
+
+         std::unique_ptr<Botan::PK_Signer> signer = std::make_unique<Botan::PK_Signer>(
+            *privkey, Test::rng(), sig_params, Botan::Signature_Format::Standard, provider);
+         std::unique_ptr<Botan::PK_Verifier> verifier =
+            std::make_unique<Botan::PK_Verifier>(*pubkey, verify_params, Botan::Signature_Format::Standard, provider);
+         result.confirm("Creating PK_Signer works", signer != nullptr);
+         result.confirm("Creating PK_Signer works", verifier != nullptr);
+
+         result.test_is_nonempty("PK_Signer should report some hash", signer->hash_function());
+         result.test_is_nonempty("PK_Verifier should report some hash", verifier->hash_function());
+
+         pubkey.reset();
+         privkey.reset();
+         const std::array<uint8_t, 4> msg{0xde, 0xad, 0xbe, 0xef};
+         const auto sig = signer->sign_message(msg, Test::rng());
+         result.test_gt("Signer should still work if no one else hold a reference to the key", sig.size(), 0);
+         result.test_eq("Verifier should still work if no one else hold a reference to the key",
+                        verifier->verify_message(msg, sig),
+                        true);
+
+         return result;
+      }
+};
+
+BOTAN_REGISTER_TEST("pubkey", "pk_api_sign", PK_API_Sign_Test);
+
 }  // namespace Botan_Tests
 
 #endif