mattosaurus / PgpCore

.NET Core class library for using PGP
MIT License
244 stars 98 forks source link

GetPgpInspectBaseResult failed with null object reference #301

Closed elim-mt closed 1 week ago

elim-mt commented 4 weeks ago

Calling the InspectAsync method failed inside the method:

private PgpInspectBaseResult GetPgpInspectBaseResult(Stream inputStream) at line 103 (PGPInspectSync.cs) privateKey = EncryptionKeys.FindSecretKey(publicKeyEncryptedData.KeyId);

With the EncryptionKeys being null and failing when attempting to execute the FindSecretKey method.

mattosaurus commented 4 weeks ago

Hi @elim-mt, do you have an example of the code you're using to setup and call the InspectAsync method?

elim-mt commented 3 weeks ago

Yes, I have created an example to show case this issue. Please kindly find it in the attachment. PGPCoreInspectExample.zip

Thank you

elim-mt commented 2 weeks ago

hi @mattosaurus , would you be able to kindly shed some light to this issue? Thanks

mattosaurus commented 2 weeks ago

Hi, apologies I'm on holiday at the moment but I'll take a look next week.

mattosaurus commented 1 week ago

Hi, the issue is you need to provide the relevant keys to inspect the message.

        public static async Task<bool> IsPgpEncryptedAndSignedAsync(string content)
        {

            if (string.IsNullOrEmpty(content)) throw new ArgumentNullException(nameof(content));

            EncryptionKeys encryptionKeys = new EncryptionKeys(new FileInfo(senderPublicKeyfilePath), new FileInfo(receiverPrivateKeyFilePath), receiverPrivateSecrets);
            PGP pgp = new PGP(encryptionKeys);

            PgpInspectBaseResult? result;
            result = await pgp.InspectAsync(content); 

            if (result != null)
                return (result.IsEncrypted && result.IsSigned);

            return false;

        }
elim-mt commented 1 week ago

Thank you so much for explaining this. Amending my code with your suggestion works. However, I feel that the code inside PGPInspectSync can be a little safer rather than just assuming that the EncryptionKeys object has been set.

Furthermore, it would appear that the InspectAsync function works fantastic if we are testing if a payload is encrypted and/or signed with known keys. However, if one simply wants to know if they payload has been encrypted or signed without passing any encryption keys for inspection, then this wouldn't work so well.

In any case, thank you.

mattosaurus commented 1 week ago

Yes, I think when I was adding these methods I just wanted to make them as complete as possible. However I agree there's scope for updating them to only check properties with the provided keys (if any).