Signing public PGP keys

[polterguy]

Howdy,

For the record, I might have misunderstood some of the basics here, since first of all BC's documentation is literally non-existent, and there is almost no online examples of how to do this, and I am also not a PGP expert in anyways. So it might be a bug in my code ...

Anyway, I am trying to sign public PGP keys using something that resembles the "DirectKeySignature" example from Bouncy Castle. It seems like the signing process works correctly, however the way I have understood this, is that this process returns a new public key, that needs to be imported back into my OpenPgpContext.

I've struggled with this for a couple of days, and I found that the OpenPgpContext.Import method at roughly line 1938 in "OpenPgpContext.cs" seems to check if the ID for the key the caller is trying to import already exists, and if it does, it returns early, never importing the key.

The way I've understood this, is that when you sign a public PGP key, you're given a new key back (my understanding might be flawed here though) - If I am right though, we'll need to be able to import public keys that already exists in the PublicKeyRingBundle, otherwise it becomes impossible to sign somebody's public key, and put it back into the OpenPgpContext using the Import (PgpPublicKeyRing keyring) method, or ...?

I tried forking MimeKit in its entirety, and removed this check, to see if I got another result, and I still struggled with the same problem, so it might be my code that is flawed.

Below is an example of what I have so far ...

/// <summary>
///     Signs the given public key(s).
/// </summary>
/// <param name="context">Application Context</param>
/// <param name="e">Active Event arguments</param>
[ActiveEvent(Name = "p5.crypto.sign-public-key")]
private static void p5_crypto_sign_public_key(ApplicationContext context, ActiveEventArgs e)
{
    // Figuring out which private key to use for signing, and doing some basic sanity check.
    var fingerprint = e.Args.GetExChildValue("private-key", context, "").ToLower ();
    if (fingerprint == "")
        throw new LambdaException("No [private-key] argument supplied to [p5.crypto.sign-public-key]", e.Args, context);

    // Finding password to use to extract private key from GnuPG context, and doing some basic sanity check.
    var password = e.Args.GetExChildValue("password", context, "");
    if (password == "")
        throw new LambdaException("No [password] argument supplied to [p5.crypto.sign-public-key] to extract your private key", e.Args, context);

    // Retrieving our private key to use for signing public key from GnuPG database.
    PgpSecretKey signingKey = null;
        using (var ctx = new GnuPrivacyContext()) {

            // Iterating all secret keyrings.
            foreach (PgpSecretKeyRing idxRing in ctx.SecretKeyRingBundle.GetKeyRings()) {

                // Iterating all keys in currently iterated secret keyring.
                foreach (PgpSecretKey idxSecretKey in idxRing.GetSecretKeys ()) {

                    // Checking if caller provided filters, and if not, yielding "everything".
                    if (BitConverter.ToString (idxSecretKey.PublicKey.GetFingerprint ()).Replace ("-", "").ToLower() == fingerprint) {

                        // No filters provided, matching everything.
                        signingKey = idxSecretKey;
                        break;
                    }
                }
            if (signingKey != null)
                break;
        }
    }

    // Using common helper to iterate all public keys caller wants to sign.
    PgpPublicKeyRing sRing = null;
    ObjectIterator.MatchingPublicKeys (context, e.Args, delegate (PgpPublicKey idxKey) {

        // Retrieving fingerprint of currently iterated key, and returning to caller.
        var node = e.Args.Add (BitConverter.ToString (idxKey.GetFingerprint ()).Replace ("-", "").ToLower ()).LastChild;

        // Doing the actual signing of currently iterated public key.
        sRing = new PgpPublicKeyRing (new MemoryStream (SignPublicKey (signingKey, password, idxKey), false));
    });

    // Creating new GnuPG context and importing signed key into context.
    using (var ctx = new GnuPrivacyContext()) {

        // Importing signed key.
        ctx.Import(sRing);

        // Returning the fingerprint of the key just signed.
        e.Args.Add(BitConverter.ToString (sRing.GetPublicKey ().GetFingerprint ()).Replace ("-", ""));
    }
}

/*
 * Helper for above.
 */
private static byte[] SignPublicKey(
    PgpSecretKey secretKey,
    string password,
    PgpPublicKey keyToBeSigned)
{
    // Extracting private key, and getting ready to create a signature.
    PgpPrivateKey pgpPrivKey = secretKey.ExtractPrivateKey (password.ToCharArray());
    PgpSignatureGenerator sGen = new PgpSignatureGenerator (secretKey.PublicKey.Algorithm, HashAlgorithmTag.Sha1);
    sGen.InitSign (PgpSignature.DirectKey, pgpPrivKey);

    // Creating a stream to wrap the results of operation.
    Stream os = new MemoryStream();
    BcpgOutputStream bOut = new BcpgOutputStream (os);
    sGen.GenerateOnePassVersion (false).Encode (bOut);

    // Creating a generator.
    PgpSignatureSubpacketGenerator spGen = new PgpSignatureSubpacketGenerator();
    PgpSignatureSubpacketVector packetVector = spGen.Generate();
    sGen.SetHashedSubpackets (packetVector);
    bOut.Flush();

    // Returning the signed public key.
    return PgpPublicKey.AddCertification (keyToBeSigned, sGen.Generate()).GetEncoded();
}

In the above code, the ObjectIterator.MatchingPublicKeys simply invokes the given delegate for each public PGP key matching some "filter criteria" (it yields one example for my test code).

As I wrote earlier, I tried to remove the check for if key already exists in the MimeKit code itself at line 1938 in OpenPgpContext.cs, but it still didn't work.

If you have some advice to me, or agree with my logic in regards to that we'll need to be able to import keys already existing after having them signed, I'd love to get some feedback on this.

Thx,

Thomas

[jstedfast]

Hmmm, I'll have to investigate this since I also don't know the answer (like you said, that id check in MimeKit might need to be removed, but it may also require other changes).

Thanks for the bug report. I probably won't get a chance to look at this for a few days (in the process of relocating to another city), but if you discover anything, let me know - I'm interested in knowing the answer to your questions even if it turns out they aren't a bug in MimeKit.

I'm also open to adding other convenience methods to OpenPgpContext to aid in some of the more common operations that people will likely need to do (and signing other keys seems like it might be on that list).

While on this topic, I also just recently learned that the GnuPG keyring format changed with v2.0 or 2.1 and it no longer uses the KeyRingBundle format, so I might need to figure out a way to interoperate with that as well.

[polterguy]

Thx, funny you mentioned "convenience methods", because on my way to do some shopping today, I pondered this problem, and realised the correct way to do this, would be to create a new OpenPgpContext convenience class, with supporting functions, since my current code is a mess in regards to these issues.

I was thinking about creating this myself, and try to figure out these parts myself, since I assume the C# community would literally love such a thing, due to BC being considered a "black magic skill" almost! However, if you'd like to incorporate this directly in MimeKit, nothing would be better than that ...

What I'd need for myself, so far (I might extend this list though later as I proceed), would be something like the following;

• Create PGP key pair (with "auto import" into GnuPG database. Optional parameter here might be an additional "seed" to BC, to increase its entropy in random number generation)
• List public/private keys (with delegate to yield matches, or something)
• Get private/public key (armored format)
• Sign public key (with another private key from GnuPG, would require caller to supply pwd to private key)
• Delete private/public key
• Import public/private key(s)

The above list, I think, at least so far, is the stuff I have so far, and would probably be enough for me to create a wrapper around the "ghist" of the GnuPG database, which I need in order to be able to have complete key management from my app, which you can see en early beta video of here - Which basically is a GMail alternative, with PGP support.

I already have implementations of the above methods, but the code s#$ks to be honest with you, since I basically had to "slash code it", due to that BC is literally chemically cleansed for documentation out there, and a nightmare to figure out how to use, due to no examples out there, and literally zero hits on Google about the concept.

I will throw in this question at the BC mailing lists though, which I just started subscribing to yesterday, and check out if they can advice in these matters ...

Thx,

.t

[polterguy]

For the record, when I instead of importing the key, simply return its armored result after having signed the key, and paste the armored result into my GnuPG database (GPG Keychain on Mac OS X will notice if you have a key on your clipboard) - It will say that a "signature was added to the key".

However, when I view the details of the key in GPG Keychain, I don't see my signature. Though, when I enumerate all signatures for the key, it returns an additional signature.

This implies I assume, that even though my code is obviously buggy, since the signature doesn't show up in GPG Keychain - It still means that the same key needs to be possible to import multiple times I believe ...

Have a nice moving, and take your time :)

[jstedfast]

Your list looks like a good set of functions to provide and more-or-less matches what I was thinking.

FWIW, I think the Export() method can be used to get the public key in armored format (not sure about private key off the top of my head, but I don't think there's a way to do that atm since my original plan was for exporting keys for use with sending via email and you don't want to export private keys and send them over email 😉 ).

[polterguy]

Totally agree, but people need to have access to physical backups of their keys, in case of server crashes, etc - And since these often are stored on thumb drives, etc - A way to export also the private key is necessary one way or another. But thx for the tip, although it was kind of obvious ... ;)

But better to give one obvious tip too much than one to little ... :D

.t

[jstedfast]

I've removed the id checks so re-importing should work now.

I'll have to look into the rest of it later when I get more time.

I'll also look into adding a way to export the private key :)

[polterguy]

For the record, I figured out why my key wouldn't show up in GPG Keychain, and was impossible to import into any key servers. And the reason was that my original code was using the DirectKey without any Notation Data. For my app, I changed it to CasualCertification, since my signatures aren't always bullet proof. That made the key possible to import to a key server, after signing it, which would make the signature for the public key show up ...

In case some other guys are reading this in the future, the entire code for creating a signature is as follows ...

private static byte[] SignPublicKey(
    PgpSecretKey secretKey,
    string password,
    PgpPublicKey keyToBeSigned)
{
    // Extracting private key, and getting ready to create a signature.
    PgpPrivateKey pgpPrivKey = secretKey.ExtractPrivateKey (password.ToCharArray());
    PgpSignatureGenerator sGen = new PgpSignatureGenerator (secretKey.PublicKey.Algorithm, HashAlgorithmTag.Sha1);
    sGen.InitSign (PgpSignature.CasualCertification, pgpPrivKey);

    // Creating a stream to wrap the results of operation.
    Stream os = new MemoryStream();
    BcpgOutputStream bOut = new BcpgOutputStream (os);
    sGen.GenerateOnePassVersion (false).Encode (bOut);

    // Creating a generator.
    PgpSignatureSubpacketGenerator spGen = new PgpSignatureSubpacketGenerator();
    PgpSignatureSubpacketVector packetVector = spGen.Generate();
    sGen.SetHashedSubpackets (packetVector);
    bOut.Flush();

    // Returning the signed public key.
    return PgpPublicKey.AddCertification (keyToBeSigned, sGen.Generate()).GetEncoded();
}

The above adds a "Casual signature" for the key.

The above also seems to be bad news for your check if the key already exists, since I am now able to sign a public key, and import it back into both the Ubuntu key server, and my local GPG Keychain - But I am not able to do it using the OpenPgpContext, since it won't import something that exists from before due to the "check if ID exists in bundle from before" logic ...

Sorry, seems like you've got some coding to do when you're done moving, unless you'd like me to have a go at it for you ...?

If you give me a hint in regards to all the places that needs to be updated, I could try to create a pull request for you ...?

[polterguy]

Hehe, you removed the flawed code faster than I could type out the question of if I should do it for you ...

Too funny ...!! :D

Unfortunately I am not using the code directly anymore, since I don't want it intermixed with my own project ...

As in, when will this change meet my local NuGet package ...? :)

[jstedfast]

I'll try to make a release this weekend :)

I'm hoping to have some time by then.

[polterguy]

Just remembered one more convenience method. Revoke key ...

My guess is that this is probably done through a similar process as signing a public key, except you use the same private key to create a revocation key for its public part, or something ... Then you probably get a public key, which you can upload to key servers, etc ...

[jstedfast]

I started working on some extra Export() methods that can be used to export to a stream instead of a MimePart. I also worked on adding some public methods to iterate over the public keys/keyrings (including an override that matches against a MailboxAddress).

I'll do the same for secret keys as well.

[jstedfast]

I wonder if your code could be simplified down to this?

        public void SignKey (PgpSecretKey secretKey, PgpPublicKey publicKey, DigestAlgorithm digestAlgo = DigestAlgorithm.Sha1)
        {
            if (secretKey == null)
                throw new ArgumentNullException (nameof (secretKey));

            if (publicKey == null)
                throw new ArgumentNullException (nameof (publicKey));

            var privateKey = GetPrivateKey (secretKey);
            var masterKey = secretKey.PublicKey;

            var signatureGenerator = new PgpSignatureGenerator (masterKey.Algorithm, GetHashAlgorithm (digestAlgo));

            signatureGenerator.InitSign (PgpSignature.DefaultCertification, privateKey);

            var signature = signatureGenerator.GenerateCertification (masterKey, publicKey);
            var signedKey = PgpPublicKey.AddCertification (publicKey, signature);
            var keyring = new PgpPublicKeyRing (signedKey.GetEncoded ());

            Import (keyring);
        }

[polterguy]

You might very well be right. The code I used, I got from the examples in BC. I admire their devs for what they have done with cryptography, but I somehow suspect that C# isn't their strongest side. Their code suffers from e.g. IEnumerable interfaces, instead of IEnumerable, and a lot of other "design flaws" ...

In fact, I have been seriously considering forking BC, or alternatively wrap the entire library ...

[jstedfast]

Yea, BC is very .NET 1.1ish and also very direct-port-from-Java. The maintainer of the C# lib seems open to taking patches for .NETification of the API to use generics/etc, but I don't think he has a lot of time to review stuff and has been fairly slow to respond to pull requests and such. I'm waiting for him to accept a PR to make it build for .NET Core and once that goes in, I'm hoping to start submitting patches to clean up the API a bit.

[polterguy]

Just updated from NuGet, and got the following exception when I tried to sign a public PGP key with my private key ...

[ArgumentException] - Bundle already contains a key with a keyId for the passed in ring.

I know you fixed this, but obviously there are some more checks in your code, where you do some checking ...?

[polterguy]

PS! Stacktrace ...

at Org.BouncyCastle.Bcpg.OpenPgp.PgpPublicKeyRingBundle.AddPublicKeyRing (Org.BouncyCastle.Bcpg.OpenPgp.PgpPublicKeyRingBundle bundle, Org.BouncyCastle.Bcpg.OpenPgp.PgpPublicKeyRing publicKeyRing) [0x0001f] in <2c1e8153b25b4cde9676eedaf8a00392>:0 
  at MimeKit.Cryptography.OpenPgpContext.Import (Org.BouncyCastle.Bcpg.OpenPgp.PgpPublicKeyRing keyring) [0x00015] in <1da9d8633a0d4fbfb21d2bbfdef7d4c9>:0 
  at p5.mime.GnuPGKeys.p5_crypto_sign_public_key (p5.core.ApplicationContext context, p5.core.ActiveEventArgs e) [0x001eb] in /Users/thomashansen/Documents/phosphorusfive/plugins/extras/p5.mime/GnuPGKeys.cs:226 
  at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&)
  at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00032] in /private/tmp/source-mono-2017-02/bockbuild-2017-02/profiles/mono-mac-xamarin/build-root/mono-x86/mcs/class/corlib/System.Reflection/MonoMethod.cs:305 
--- End of stack trace from previous location where exception was thrown ---

[jstedfast]

Hmmm, looks like BC has their own logic to prevent adding a keyring that is already in the bundle?

That said, I've added some Delete() methods to delete public and secret keyrings.

That leaves a SignKey() method as the last remaining method missing...

[polterguy]

Thx, are you going to add it in MimeKit, or is that the remaining parts up to me in my stuff ...?

[jstedfast]

Nah, I've added it to MimeKit... just needed to figure out how to do it :)

[polterguy]

Remind me to ship you some flowers one of these days ... ;)