mattosaurus / PgpCore

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

Encryption using multiple public keys #151

Closed PrzemyslawKlys closed 2 years ago

PrzemyslawKlys commented 2 years ago

I use your library as part of my PowerShell community project called PSPGP

One of the users opened an issue:

I've tried to implement it by just passing multiple PublicKeys to [PgpCore.EncryptionKeys]::new($PublicKeys) because it seems that your library does support this

image

Any idea what I'm doing wrong?

mattosaurus commented 2 years ago

This should work with multiple public keys but it's possible I've broken the functionality somehow. I'll add some more tests in and see if it's actually working or not.

mattosaurus commented 2 years ago

So it seems like this does still work (at least according to the tests).

https://github.com/mattosaurus/PgpCore/blob/22b90f0a45258f1232e03b9f095841dcd75afd15/PgpCore.Tests/UnitTests/UnitTestsAsync.cs#L2325-L2363

Is it possible you're passing in a null or blank key as one of the arguments?

Also, the EncryptArmoredString method takes the actual key contents rather than the path to the key as arguments just in case you're passing them through that way.

If the above doesn't help send me over the C# code that gets called by your PS script (along with test keys) and I'll see if I can debug it.

PrzemyslawKlys commented 2 years ago

Ye, i'm passing the path to the file, not the actual content.

For single file I was using FileInfo and passing it as that and it worked. When passing two, it wasn't.

[System.IO.FileInfo]::new($FilePathPubc)

Now that I pass the content using Get-Content -Raw it doesn't throw errors and works fine

    $PublicKeys = foreach ($FilePathPubc in $FilePathPublic) {
        if (Test-Path -LiteralPath $FilePathPubc) {
            #[System.IO.FileInfo]::new($FilePathPubc)
            Get-Content -Raw -LiteralPath $FilePathPubc
        } else {
            if ($PSBoundParameters.ErrorAction -eq 'Stop') {
                throw
            } else {
                Write-Warning -Message "Protect-PGP - Public key doesn't exists $($FilePathPubc): $($_.Exception.Message)"
                return
            }
        }
    }

This means PowerShell users will be able to do

Protect-PGP -FilePathPublic @("$PSScriptRoot\Keys\PublicPGP.asc", "$PSScriptRoot\Keys\PublicPGP2.asc") -String "This is string to encrypt"

and get

-----BEGIN PGP MESSAGE-----
Version: BCPG C# v1.8.9.0

hIwDIiIVJOv90IMBA/9EMLy4xpODVg/vceXEvRsm3edtmEfVsyzlUIaD+eW6bnTo
FjEhee4/J6H8WAfghUYEdaIbq+7hNDHYeKMrX7VtauFmEq7bp4AuXqCxLrSvnOyY
jUtMcaRfxGKKoPmiQi6dWZNVcs2HikiIVYUfLBXmlEgXeL4rTP3uiZVDdbyLFNJG
AcBk3YlyS9oz00bVLYYmsfg0pRq0Z4mNNGm0BTPQq2YoSpuQ95xxx4Mj4lOnrI4U
wwDP3yXUy1CEQC3C57NaaOPbmmBXGw==
=Bn8X
-----END PGP MESSAGE-----
PrzemyslawKlys commented 2 years ago

Thank you for your help, and sorry for wrong issue.

PrzemyslawKlys commented 2 years ago

There is one thing tho - you do support multiple public keys as FileInfo - yet it doesn't work then

image

mattosaurus commented 2 years ago

Yep, though the test for this is also passing.

https://github.com/mattosaurus/PgpCore/blob/22b90f0a45258f1232e03b9f095841dcd75afd15/PgpCore.Tests/UnitTests/UnitTestsAsync.cs#L385-L423

If you try and encrypt using C# directly rather than via PS do you get the same issue?

PrzemyslawKlys commented 2 years ago

You're showing 2 tests that do a similar thing - aka encrypt a string or encrypt a file with multiple public keys. And both work for me after I've changed to loading key content and providing keys rather than files.

What works:

  PGP pgpEncrypt = new PGP(encryptionKeys);
  PGP pgpDecrypt = new PGP(decryptionKeys);

What doesn't work is the way of initializing new PGP. If you initialize it with a list of FileInfo objects it seems to throw an error.

So your tests only test for one scenario. I think adding another test such as

 List<FileInfo> keys = new List<FileInfo>()
            {
                testFactory.PublicKey,
                testFactory2.PublicKey
            };

should show it's not working that way.

mattosaurus commented 2 years ago

Sorry, I think I'm missing something. The above code does use a list of public keys.

     List<FileInfo> keys = new List<FileInfo>() 
     { 
         testFactory.PublicKeyFileInfo, 
         testFactory2.PublicKeyFileInfo 
     }; 

     EncryptionKeys encryptionKeys = new EncryptionKeys(keys, testFactory.PrivateKeyFileInfo, testFactory.Password); 
     EncryptionKeys decryptionKeys = new EncryptionKeys(testFactory2.PrivateKeyFileInfo, testFactory2.Password); 

     PGP pgpEncrypt = new PGP(encryptionKeys);

Test here where the file is just encrypted.

https://github.com/mattosaurus/PgpCore/blob/22b90f0a45258f1232e03b9f095841dcd75afd15/PgpCore.Tests/UnitTests/UnitTestsAsync.cs#L280-L310

Are you able to provide a full test or example for me to look into please?

PrzemyslawKlys commented 2 years ago

Never mind. I am dumb.


// Load keys
string publicKey = File.ReadAllText(@"C:\Support\GitHub\PSPGP\Examples\Keys\PublicPGP1.asc");
string publicKey2 = File.ReadAllText(@"C:\Support\GitHub\PSPGP\Examples\Keys\PublicPGP2.asc");

EncryptionKeys encryptionKeys = new EncryptionKeys(publicKey);
PGP pgp = new PGP(encryptionKeys);

// Encrypt and Sign
string encryptedSignedContent = pgp.EncryptArmoredString("String to encrypt");
Console.WriteLine(encryptedSignedContent);

List<string> publicKeyList = new List<string>();
publicKeyList.Add(publicKey);
publicKeyList.Add(publicKey2);

encryptionKeys = new EncryptionKeys(publicKeyList);
pgp = new PGP(encryptionKeys);

// Encrypt and Sign
encryptedSignedContent = pgp.EncryptArmoredString("String to encrypt");
Console.WriteLine(encryptedSignedContent);

List<FileInfo> publicFileList = new List<FileInfo>();
FileInfo file1 = new FileInfo(@"C:\Support\GitHub\PSPGP\Examples\Keys\PublicPGP1.asc");
FileInfo file2 = new FileInfo(@"C:\Support\GitHub\PSPGP\Examples\Keys\PublicPGP1.asc");
publicFileList.Add(file1);
publicFileList.Add(file2);

var encryptionKeys1 = new EncryptionKeys(publicFileList);
pgp = new PGP(encryptionKeys1);

// Encrypt and Sign
encryptedSignedContent = pgp.EncryptArmoredString("String to encrypt");
Console.WriteLine(encryptedSignedContent);

The problem for me was coming from that I was using an Array of objects rather then creating generic list.

 $PublicKeysTest = [System.Collections.Generic.List[System.IO.FileInfo]]::new()

Sorry for wasting your time.