bcgit / bc-csharp

BouncyCastle.NET Cryptography Library (Mirror)
https://www.bouncycastle.org/csharp
MIT License
1.63k stars 547 forks source link

easier way to implement custom signer #192

Open DmitriNymi opened 5 years ago

DmitriNymi commented 5 years ago

This a request for making BouncyCastle more extendable. Specifically I had to implement a custom signer instead of ECDsaSigner because the private keys associated with the certificate in the certificate store are non-exportable, thus I cannot pass them to Asn1SignatureFactory (string algorithm, AsymmetricKeyParameter privateKey).

I had access to custom method that returns a signature of hash given location of the certificate in the store but integrating this custom signer requires copy&paste-ing of lots of code from Bouncy Castle instead of providing a simple extension point to inject my custom signer (derived from Org.BouncyCastle.Crypto.IDsa) for the given algorithm (ECDSA with SHA256).

  1. I had to create a custom class that is just a copy & paste Asn1SignatureFactory in order to overwrite one line of code in CreateCalculator() that creates ISigner: ISigner sig = PksSignerUtils.GetSigner(algorithm);

where PksSignerUtils is my custom implementation of BouncyCastle's SignerUtilities class again mostly copy&pasted except for these few lines of code in GetSigner(string algorithm):

if (Platform.EndsWith(mechanism, "withECDSA")) { string digestName = mechanism.Substring(0, mechanism.LastIndexOf("with")); IDigest digest = DigestUtilities.GetDigest(digestName); return new DsaDigestSigner(new ECDsaSigner(), digest); }

where I replaced new ECDsaSigner() with my custom signer class .

Since some helper classes are marked as "internal" and not "public" I had to copy&paste them too: SigCalculator, SigResult, SignerBucket (this was fixed in BouncyCastle 1.8.5 Org.BouncyCastle.Crypto.Operators but I was using BouncyCastle 1.8.3.1 ) Platform class - used in my custom override of SignerUtilities and X509Utilities X509Utilities class - used in my custom Asn1SignatureFactory

You can see all the classes at https://github.com/DmitriNymi/StoreCertificateSigner

I saw a similar item that was already closed but I think is still not resolved fully: https://github.com/bcgit/bc-csharp/issues/153

Notice that my custom signer uses a custom private key class, like so:

public class PksEcPrivateKey : AsymmetricKeyParameter
    {
        public string CertificateCommonName { get; }
        public string CertificateStoreName { get; }
        public string CertificateStoreLocation { get; }

        /// <summary>
        /// Defines the location of the certificate in the certificate store,
        /// so that the associated private key can be used to create the signature
        /// </summary>
        /// <param name="certificateCommonName">CN (common name) of the certificate to be found in certificate store</param>
        /// <param name="certificateStoreName">Example: "CA" or "MY"</param>
        /// <param name="certificateStoreLocation">Example: "LocalMachine", "User", "Service"</param>
        public PksEcPrivateKey(
            string certificateCommonName,
            string certificateStoreName,
            string certificateStoreLocation
        ) 
            : base(true)
        {
            CertificateCommonName    = certificateCommonName;
            CertificateStoreName     = certificateStoreName;
            CertificateStoreLocation = certificateStoreLocation;
        }
agpreynolds commented 5 years ago

Hi the issue you mentioned was mine, I also assisted someone else with a similar problem a few weeks back. See here for my updated implementation https://github.com/bcgit/bc-csharp/issues/187#issuecomment-486706341

As you mentioned the internal support classes were all made public in 1.8.5 in order to help resolve this issue. Is your problem that you can't upgrade to 1.8.5?

I can see your issue with SignerUtilities is that this is a sealed class, one approach could be to remove the sealed flag, make GetSigner virtual and override the method in a sub-class.

Personally though I would think about your use case, do you need to support the same number of algorithm's as the SignerUtilities class or are you in reality only targeting one? Maybe a simpler approach is to just instantiate a custom signer similar to how I did it in my example...

DmitriNymi commented 5 years ago

Concerning upgrade to 1.8.5:

I found out about some of the internal classes being made public in 1.8.5 only later in development (my fault), but when I moved to 1.8.5 my app started throwing exception during certificate creation when my class PksEcdsaSigner was used. After some debugging using the original code from 1.8.3 vs .1.8.5, I found out that if my PksEcdsaSigner class derives from Org.BouncyCastle.Crypto.Signers.ECDsaSigner then there was exception in 1.8.5 but not 1.8.3. That is because ECDsaSigner in 1.8.5 implements IDsaExt, while in 1.8.3 it implements IDsa. During encoding into ASN.1 step (after GenerateSignature) BC calls to ECDsaSigner.Order which has in ECDsaSigner ver 1.8.5 :

public virtual BigInteger Order { get { return key.Parameters.N; } }

since my key has no Parameters, it throws NullReferenceException here. This could be fixed by overriding Order getter, but only when my project uses BC 1.8.5, since BC 1.8.3 doesn't even have this getter.

My solution was just derive ECDsaSigner directly from IDsa, that way my code can be used with both versions of BC. Although that resolved my issue with ECDsaSigner, I am not sure there will be no issues with other classes in case I move to BC 1.8.5. So it requires testing all my projects with newer version. This work will be done soon, but currently we are not ready to move to 1.8.5.

Back to original question : You are absolutely right about: "I can see your issue with SignerUtilities is that this is a sealed class, one approach could be to remove the sealed flag, make GetSigner virtual and override the method in a sub-class." The problem is SignerUtilities is a static class, so you cannot derive from it (neither can you create a virtual method in it).
Another option can be allow passing a helper class into Asn1SignatureFactory constructor, that in turn will be used inside Asn1SignatureFactory.CreateCalculator() :


     public Asn1SignatureFactory(ISignerSelector selector, ...) { this.selector = selector;  ... }

     public IStreamCalculator CreateCalculator()
        {
            ISigner signer = SignerUtilities.InitSigner(selector, algorithm, true, privateKey, random);

            return new DefaultSignatureCalculator(signer);
        }

    // in  SignerUtilities
    public static ISigner InitSigner(ISignerSelector selector, string algorithm, bool forSigning, 
                                   AsymmetricKeyParameter privateKey, SecureRandom random)
        {
            ISigner signer = SignerUtilities.GetSigner(selector, algorithm);
            signer.Init(forSigning, ParameterUtilities.WithRandom(privateKey, random));
            return signer;
        }

    public static ISigner GetSigner(ISignerSelector selector, string algorithm)
    {
           if (algorithm == null) throw new ArgumentNullException("algorithm");
            algorithm = Platform.ToUpperInvariant(algorithm);
            string mechanism = (string) algorithms[algorithm];

            if (mechanism == null) mechanism = algorithm;

            if(selector != null) {
               var signer = selector.GetSigner(mechanism);
               if(signer != null) return signer;
            }

            // rest of the original method...
    }

That way users can implement custom ISignerSelector, to replace all or some of algorithms as required.

In my case I had to replace ECDsaSigner for different digests: "NONEwithECDSA", "SHA-1withECDSA", "SHA-224withECDSA", "SHA-256withECDSA", "SHA-384withECDSA", "SHA-512withECDSA". Although it is possible that we will have to support more in the future.