Use of none botan private keys for signing certificates.

[larssilven]

I got my own library for pkcs#11 with classes for different kind of keys.
When signing certificates I have created my own PKCS11_RSA_Signature_Operation that implements your PK_Ops::Signature.
This works fine but I had to make some of the Botan internal API public. The changes that I made is in this pull request.
The reason that I can not use your p11 private key class is that:

• My p11 lib got other features not implemented in botan that is essential to me.
• I am not using a p11 module for all p11 instances. P11 instances may also reside on other hosts. A proxy TCP protocol is used for remote instances.
  A better alternative than making part of Botan API public could be to define a callback interface with the key signing that I could implement. Then I could call botan directly with my private key class.

I would appreciate any help to solve my issue.

BR Lars

[securitykernel]

Thanks for raising this issue. Do I understand correctly that you have encapsulated PKCS#11 access to your HSMs in your own library, but want to sign certificates using Botan's X.509 module with keys in your HSM? If so, then instead of trying to derive from PKCS11_RSA_PrivateKey and the like, you could create key types derived from Private_Key and within those forward calls to your PKCS#11 library, which is what I have done with TPMs in the past, for example. Deriving from the PKCS#11 key types is not supported, in fact they are marked final, because PKCS#11 as you know is a standardized interface where the modularity lies not in the interface, but in the vendor-specific PKCS#11 module's implementation, which is only invoked via the PKCS#11 interface.

[larssilven]

Maybe it is easier to understand if you look at the source code (working) for the signing. Below is my implementation of Botan::Private_Key. I used botan ./src/lib/prov/pkcs11/p11_rsa.cpp as template. I have tried to use as much from Botan as possible but m_key is not from Botan. With m_key all p11 functions of a key could be called for a p11 session.
Then I could create a Botan CA like this:
const X509_CA ca(issuerX509, issuerPrivateKey, sHash, sPadding, rng);
Where issuerPrivateKey key is the sep::x509::PKCS11_RSA_PrivateKey that is implemented in the code below. Whit this CA certificate signing works as expected (also with EMSA4 padding).

// clang-format off
#include <botan/p11_mechanism.h>
#include <botan/pk_ops.h>
#include "./pkcs11_rsa.h"
#include <botan/pss_params.h>
// clang-format on

using namespace Botan;
using namespace Botan::PKCS11;
using namespace sep::p11;

namespace sep {

namespace x509 {

template <typename CK_ULONG, typename CK_LONG>
PKCS11_RSA_PrivateKey<CK_ULONG, CK_LONG>::PKCS11_RSA_PrivateKey(const p11::Session<CK_ULONG, CK_LONG>* const _pSession,
                                                                const P11_DEF::CK_OBJECT_HANDLE _hKey,
                                                                const std::vector<unsigned char>* const pModulus,
                                                                const std::vector<unsigned char>* const pPublicExponent)
    : Botan::RSA_PublicKey(BigInt(&pModulus->front(), pModulus->size()),
                           BigInt(&pPublicExponent->front(), pPublicExponent->size()))
    , pSession(_pSession)
    , hKey(_hKey) {
}

template <typename CK_ULONG, typename CK_LONG> class PKCS11_RSA_Signature_Operation final : public PK_Ops::Signature {
   public:
    PKCS11_RSA_Signature_Operation(const PKCS11_RSA_PrivateKey<CK_ULONG, CK_LONG>& key, const std::string_view padding)
        : m_key(key), m_mechanism(MechanismWrapper::create_rsa_sign_mechanism(padding)) {
    }

    size_t signature_length() const override {
        return m_key.get_n().bytes();
    }

    void update(const uint8_t msg[], size_t msg_len) override {
        if(!m_initialized) {
            // first call to update: initialize and cache message because we can not determine yet whether a single- or
            // multiple-part operation will be performed
            m_key.pSession->signInit((const P11_DEF::CK_MECHANISM*)m_mechanism.data(), m_key.hKey);
            m_initialized = true;
            m_first_message = secure_vector<uint8_t>(msg, msg + msg_len);
            return;
        }
        if(!m_first_message.empty()) {
            // second call to update: start multiple-part operation
            m_key.pSession->signUpdate(&m_first_message.front(), m_first_message.size());
            m_first_message.clear();
        }
        m_key.pSession->signUpdate(msg, msg_len);
    }

    secure_vector<uint8_t> sign(RandomNumberGenerator&) override {
        m_initialized = false;
        if(!m_first_message.empty()) {
            // single call to update: perform single-part operation
            const std::unique_ptr<const std::vector<P11_DEF::CK_BYTE>> pSignature(
                m_key.pSession->sign(&m_first_message.front(), m_first_message.size()));
            m_first_message.clear();
            return secure_vector<uint8_t>(pSignature->begin(), pSignature->end());
        }
        // multiple calls to update (or none): finish multiple-part operation
        const std::unique_ptr<const std::vector<P11_DEF::CK_BYTE>> pSignature(m_key.pSession->signFinal());
        return secure_vector<uint8_t>(pSignature->begin(), pSignature->end());
    }

    std::string hash_function() const override {
        switch(m_mechanism.mechanism_type()) {
        case MechanismType::Sha1RsaPkcs:
        case MechanismType::Sha1RsaPkcsPss:
        case MechanismType::Sha1RsaX931:
            return "SHA-1";

        case MechanismType::Sha224RsaPkcs:
        case MechanismType::Sha224RsaPkcsPss:
            return "SHA-224";

        case MechanismType::Sha256RsaPkcs:
        case MechanismType::Sha256RsaPkcsPss:
            return "SHA-256";

        case MechanismType::Sha384RsaPkcs:
        case MechanismType::Sha384RsaPkcsPss:
            return "SHA-384";

        case MechanismType::Sha512RsaPkcs:
        case MechanismType::Sha512RsaPkcsPss:
            return "SHA-512";

        case MechanismType::RsaX509:
        case MechanismType::RsaX931:
        case MechanismType::RsaPkcs:
        case MechanismType::RsaPkcsPss:
            return "Raw";

        default:
            throw Internal_Error("Unable to determine associated hash function of PKCS11 RSA signature operation");
        }
    }

    AlgorithmIdentifier algorithm_identifier() const override {
        const std::string hash = this->hash_function();

        switch(m_mechanism.mechanism_type()) {
        case MechanismType::Sha1RsaPkcs:
        case MechanismType::Sha224RsaPkcs:
        case MechanismType::Sha256RsaPkcs:
        case MechanismType::Sha384RsaPkcs:
        case MechanismType::Sha512RsaPkcs: {
            const OID oid = OID::from_string("RSA/EMSA3(" + hash + ")");
            return AlgorithmIdentifier(oid, AlgorithmIdentifier::USE_NULL_PARAM);
        }

        case MechanismType::Sha1RsaPkcsPss:
            return pss_algorithm_identifier(20);
        case MechanismType::Sha224RsaPkcsPss:
            return pss_algorithm_identifier(28);
        case MechanismType::Sha256RsaPkcsPss:
            return pss_algorithm_identifier(32);
        case MechanismType::Sha384RsaPkcsPss:
            return pss_algorithm_identifier(48);
        case MechanismType::Sha512RsaPkcsPss:
            return pss_algorithm_identifier(64);

        default:
            throw Not_Implemented("No algorithm identifier defined for RSA with this PKCS11 mechanism");
        }
    }

   private:
    AlgorithmIdentifier pss_algorithm_identifier(const size_t salt_len) const {
        const OID oid(OID::from_string("RSA/EMSA4"));
        const PSS_Params params(hash_function(), salt_len);
        return AlgorithmIdentifier(oid, params.serialize());
    }

    const PKCS11_RSA_PrivateKey<CK_ULONG, CK_LONG>& m_key;
    bool m_initialized = false;
    secure_vector<uint8_t> m_first_message;
    MechanismWrapper m_mechanism;
};

template <typename CK_ULONG, typename CK_LONG>
std::unique_ptr<PK_Ops::Signature> PKCS11_RSA_PrivateKey<CK_ULONG, CK_LONG>::create_signature_op(
    RandomNumberGenerator& /*rng*/,
    const std::string_view params,
    const std::string_view /*provider*/
) const {
    return make_unique<PKCS11_RSA_Signature_Operation<CK_ULONG, CK_LONG>>(*this, params);
}

template <typename CK_ULONG, typename CK_LONG>
std::unique_ptr<Public_Key> PKCS11_RSA_PrivateKey<CK_ULONG, CK_LONG>::public_key() const {
    return std::make_unique<RSA_PublicKey>(
        this->get_n(),   // BigInt::decode(get_attribute_value(AttributeType::Modulus)),
        this->get_e());  // BigInt::decode(get_attribute_value(AttributeType::PublicExponent)));
}

template <typename CK_ULONG, typename CK_LONG>
Botan::secure_vector<uint8_t> PKCS11_RSA_PrivateKey<CK_ULONG, CK_LONG>::private_key_bits() const {
    return secure_vector<uint8_t>();
}

template class PKCS11_RSA_PrivateKey<LONG_DEF>;
template class PKCS11_RSA_PrivateKey<INT_DEF>;
template class PKCS11_RSA_Signature_Operation<LONG_DEF>;
template class PKCS11_RSA_Signature_Operation<INT_DEF>;
}  // namespace x509
}  // namespace sep

[securitykernel]

What I see is that your PKCS11_RSA_Signature_Operation is basically a copy of Botan's PKCS11_RSA_Signature_Operation, except that instead of invoking PKCS#11's C_Sign*() on the module (m_key.module()->C_Sign*()), you invoke custom functions m_key.pSession->sign*() on your key's session, which code you omitted, so it is hard for me to make a judgement. Whatever your pSession does, you seem to bypass Botan's low-level PKCS#11 API, which is not supported and for which I currently can not see a use case for.

I'd still suggest you derive from Botan::PrivateKey or even Botan::RSA_PrivateKey instead, and then from your RSA_Signature_Operation you can still call your m_key.pSession->sign*() and the like.

[larssilven]

Here are the 2 methods you were interested in (m_key.pSession->sign*). It is just p11 function calls. But ptr is not a ::CK_FUNCTION_LIST. ptr is a pointer to an interface class with 2 different implementation:

• Remote. The call is executed on a p11 instance on another host. A propitiatory protocol is used for this.
• Local. A local p11 module is used. Here ptr is just a wrapper on the ::CK_FUNCTION_LIST.

    void signInit(const P11_DEF::CK_MECHANISM* const pMechanism, const P11_DEF::CK_OBJECT_HANDLE hKey) const {
        TEST_P11(ptr->C_SignInit(handle, (P11_DEF::CK_MECHANISM*)pMechanism, hKey));
    }
    void signUpdate(const P11_DEF::CK_BYTE* const pPart, const CK_ULONG ulPartLen) const {
        TEST_P11(ptr->C_SignUpdate(handle, (P11_DEF::CK_BYTE*)pPart, ulPartLen));
    }

I am already implementing the Botan::PrivateKey with most of the code borrowed from the Botan class with same name (but different namespace). Here is the header for the implementation I posted before:

#ifndef PKCS11_RSA_H_
#define PKCS11_RSA_H_

#include <botan/rsa.h>
#include "../../include/Session.h"

namespace sep {
namespace x509 {

/// Represents a PKCS#11 RSA private key
template <typename CK_ULONG, typename CK_LONG>
class PKCS11_RSA_PrivateKey final : public Botan::Private_Key, public Botan::RSA_PublicKey {
   public:
    /// Creates a PKCS11_RSA_PrivateKey object from an existing PKCS#11 RSA private key
    PKCS11_RSA_PrivateKey(const sep::p11::Session<CK_ULONG, CK_LONG>* pSession,
                          P11_DEF::CK_OBJECT_HANDLE hKey,
                          const std::vector<unsigned char>* pModulus,
                          const std::vector<unsigned char>* pPublicExponent);

    std::unique_ptr<Botan::PK_Ops::Signature> create_signature_op(Botan::RandomNumberGenerator& rng,
                                                                  const std::string_view params,
                                                                  const std::string_view provider) const override;

    std::unique_ptr<Public_Key> public_key() const override;
    Botan::secure_vector<uint8_t> private_key_bits() const override;

    const sep::p11::Session<CK_ULONG, CK_LONG>* pSession;
    const P11_DEF::CK_OBJECT_HANDLE hKey;
};

}  // namespace x509
}  // namespace sep
#endif

The purpose of the pull request is to allow use of some parts of the Botan API that is now hidden to the world outside Botan, These classes needs to be public for my PKCS11_RSA_PrivateKey :

• PSS_Params
• MechanismWrapper
• Signature

[securitykernel]

Are you building on Windows with Visual Studio/MSVC? I am asking because deriving from Botan::Private_Key with upstream master´ works just fine on Linux, whereas Windows complains about undefined type Botan::PK_Ops::Signature`. I assume this is because the default visibility on Linux is public, but on Windows is private.

[larssilven]

I'm building on Linux with g++. As I said before everything work just fine with the patches in this pull request.
All needed is just to make parts of the API that is hidden public.

[securitykernel]

My point is that it should work without making PK_Ops::Signature (and other types) in pk_ops.h public, because these are forward-declared in pk_ops_fwd.h, so there is no need to include <botan/pk_ops.h>. This affects Windows, too, I must admit. I failed to #include <botan/pubkey.h> and I see this is include missing in your latest code, too. Can you try this fix?

May I ask which compile/linker errors you see with pk_ops.h being internal only when building/linking your application? This may not be required anymore, see above.

Exposing p11_mechanism.h and pss_params.h may be fine if you say you need it for your setup, but I'd like to ask @randombit for his opinion.

[larssilven]

#include <botan/pk_ops.h> is needed in the implementation:
template <typename CK_ULONG, typename CK_LONG> class PKCS11_RSA_Signature_Operation final : public PK_Ops::Signature { see #3867 (comment)
but not in the header file.
It should be valuable to have pk_ops.h if someone needs to do implementations for other operations.

[securitykernel]

This is actually a regression, which we'll handle via #3878. Thanks for pointing this out!

[randombit]

I guess I would not have anticipated this usage but it seems fine to expose both of these classes.

@larssilven one requested change regarding the version numbers. Otherwise lgtm

[larssilven]

Any more problem my latest push?

[larssilven]

I got this info now:

Merging is blocked
Merging can be performed automatically once the requested changes are addressed.

But what should I do to have the merging done done automatically?
I have rebased my branch against the upstream master branch.

[larssilven]

Maybe I should not have done any rebase?
Please have a look again. I think the version change to 3.6 has been done.

[reneme]

But what should I do to have the merging done automatically?

There's no automatic merging. This pull request is waiting on @randombit for merging.

Thanks for applying the changes requested earlier.

[reneme]

CI is complaining about your formatting. Essentially the order of includes. The failures in Windows don't seem to be related (some package during setup fails to download).

--- /home/runner/work/botan/botan/source/src/lib/prov/pkcs11/p11_ecdh.cpp (original)
+++ /home/runner/work/botan/botan/source/src/lib/prov/pkcs11/p11_ecdh.cpp
@@ -11,9 +11,9 @@
 #if defined(BOTAN_HAS_ECDH)
 
    #include <botan/der_enc.h>
+   #include <botan/p11_mechanism.h>
    #include <botan/pk_ops.h>
    #include <botan/rng.h>
-   #include <botan/p11_mechanism.h>
 
 namespace Botan::PKCS11 {
 
--- /home/runner/work/botan/botan/source/src/lib/prov/pkcs11/p11_ecdsa.cpp (original)
+++ /home/runner/work/botan/botan/source/src/lib/prov/pkcs11/p11_ecdsa.cpp
@@ -10,10 +10,10 @@
 
 #if defined(BOTAN_HAS_ECDSA)
 
+   #include <botan/p11_mechanism.h>
    #include <botan/pk_ops.h>
    #include <botan/rng.h>
    #include <botan/internal/keypair.h>
-   #include <botan/p11_mechanism.h>
 
 namespace Botan::PKCS11 {
 
--- /home/runner/work/botan/botan/source/src/lib/prov/pkcs11/p11_rsa.cpp (original)
+++ /home/runner/work/botan/botan/source/src/lib/prov/pkcs11/p11_rsa.cpp
@@ -12,10 +12,10 @@
 
 #if defined(BOTAN_HAS_RSA)
 
+   #include <botan/p11_mechanism.h>
    #include <botan/pubkey.h>
    #include <botan/rng.h>
    #include <botan/internal/blinding.h>
-   #include <botan/p11_mechanism.h>
    #include <botan/internal/pk_ops_impl.h>
 
 namespace Botan::PKCS11 {
--- /home/runner/work/botan/botan/source/src/lib/pubkey/pubkey.cpp (original)
+++ /home/runner/work/botan/botan/source/src/lib/pubkey/pubkey.cpp
@@ -11,8 +11,8 @@
 #include <botan/der_enc.h>
 #include <botan/mem_ops.h>
 #include <botan/pk_ops.h>
+#include <botan/pss_params.h>
 #include <botan/rng.h>
-#include <botan/pss_params.h>
 #include <botan/internal/ct_utils.h>
 #include <botan/internal/fmt.h>
 #include <botan/internal/parsing.h>
--- /home/runner/work/botan/botan/source/src/lib/pubkey/rsa/rsa.cpp (original)
+++ /home/runner/work/botan/botan/source/src/lib/pubkey/rsa/rsa.cpp
@@ -9,8 +9,8 @@
 
 #include <botan/ber_dec.h>
 #include <botan/der_enc.h>
+#include <botan/pss_params.h>
 #include <botan/reducer.h>
-#include <botan/pss_params.h>
 #include <botan/internal/blinding.h>
 #include <botan/internal/divide.h>
 #include <botan/internal/emsa.h>
--- /home/runner/work/botan/botan/source/src/lib/tls/tls_signature_scheme.cpp (original)
+++ /home/runner/work/botan/botan/source/src/lib/tls/tls_signature_scheme.cpp
@@ -11,9 +11,9 @@
 #include <botan/ec_group.h>
 #include <botan/hash.h>
 #include <botan/hex.h>
+#include <botan/pss_params.h>
 #include <botan/tls_exceptn.h>
 #include <botan/tls_version.h>
-#include <botan/pss_params.h>
 #include <botan/internal/stl_util.h>
 
 namespace Botan::TLS {

[coveralls]

coverage: 91.281% (+0.002%) from 91.279%
when pulling df7818c on larssilven:none-botan-p11-keys
into 0639e82 on randombit:master.

[larssilven]

@randombit changes are now applied.

[larssilven]

@randombit anything else I have to do to get your approval?
Should I rebase to 3.6.1 or master?
Would be great for me if this could be done until the 3.7