PGP Decryption - Unnecessary exception stops execution

[Coderiffic68]

I encountered this issue in the nuget package (1.8.3), and spent hours heading in the wrong direction trying to fix it (which I finally did). Figured I'd post here so it can be addressed and a fix put into the build. Hopefully save someone else the massive headache.

The issue occurs during PGP decryption. Specifically, when attempting to execute this statement: PgpSecretKeyRingBundle pgpSec = new PgpSecretKeyRingBundle( PgpUtilities.GetDecoderStream(keyIn));

I would get this exception: Org.BouncyCastle.Bcpg.OpenPgp.PgpPublicKeyRing found where PgpSecretKeyRing expected

I went crazy trying to find some piece of code I was missing, or figure out how my private key was suddenly being seen as a public key. Even tried generating new keys from various different locations to see if that would resolve. I eventually pulled the whole bc-csharp codebase, directly referenced it instead of the nuget package, and found the problem.

In the Org.BouncyCastle.Bcpg.OpenPgp.PgpSecretKeyRingBundle public constructor, there is this code block:

foreach (object obj in e)
{
	PgpSecretKeyRing pgpSecret = obj as PgpSecretKeyRing;

	if (pgpSecret == null)
	{
		throw new PgpException(Platform.GetTypeName(obj) + " found where PgpSecretKeyRing expected");
	}

	long key = pgpSecret.GetPublicKey().KeyId;
	secretRings.Add(key, pgpSecret);
	order.Add(key);
}

What I discovered is that for my private key file, it seems there are two items in e. My private key, and I assume also my public key. So when it reached the for loop above, it would properly identify and get the private key, then continue on to the other object (not of type PgpSecretKeyRing) and end up throwing the exception. This then balks the code all the way out of BouncyCastle.

My working fix was to simply continue if the pgpSecret is null, and then throw an exception if order is empty.

foreach (object obj in e)
{
	PgpSecretKeyRing pgpSecret = obj as PgpSecretKeyRing;

	if (pgpSecret == null)
	{
		continue;
	}

	long key = pgpSecret.GetPublicKey().KeyId;
	secretRings.Add(key, pgpSecret);
	order.Add(key);
}
if(order.Count == 0)
{
	throw new PgpException("No private key found");
}

This may not be the most elegant, or overall correct fix, which is why I'm simply raising awareness.

Now that I fixed that, this works fantastically and I'm (finally) thrilled! Thanks

[jasoncocks]

I notice this issue is still open. I've run into the same issue today, and code references are still accurate. I have followed Coderiffic68 notes, and also fixed my issue. Is this issue going to be resolved in a manner consistent with Coderiffic68, or is my Secret Key invalid?

If I import the same Secret Key into GPG then I am able to decrypt without error.

[peterdettman]

I don't know whether it's valid to have anything other than a PgpSecretKeyRing show up there. The bc-java code doesn't allow it either and sees wider usage than bc-csharp, but it's possible some new feature was added. It's not crazy that e.g. the corresponding public key might be allowed to be present, but I'm not inclined to allow it in the default loading code until I understand what's going on.

However, if you want to accept such files, you can easily filter the objects yourself. Notice the code for the constructor you are using:

        public PgpSecretKeyRingBundle(
            Stream inputStream)
            : this(new PgpObjectFactory(inputStream).AllPgpObjects())
        {
        }

AllPgpObjects() is reading all the PGP objects from the stream, you can just filter them for PgpSecretKeyRing instances. Then pass the filtered list to the PgpSecretKeyRingBundle(IEnumerable e) constructor.

[johnaau]

I had the same problem and the source code change by @Coderiffic68 worked for me. Thank you @Coderiffic68!

[dghgit]

This is a bit of an odd one. Strictly speaking we shouldn't be accepting files like this by default - I can't talk for GPG but usually if someone gives us an encoding claiming to be X, it's not a great idea to accept it even when it's not. Would someone be able to say where they are coming from? As Peter suggested the correct answer is to use IEnumerable constructor as it means you at least know what you're dealing with.

[Coderiffic68]

Been a long time, but I can say my keys were generated using PGPTool (https://pgptool.github.io/).

[alecossio]

Hi, I've had the same issue on bcjava and dotnet, and the common element is that keys were generated with PGPTool