Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional PgpContext class, in-between OpenPgpContext and CryptographyContext #577

Closed
polterguy opened this issue Jun 3, 2020 · 7 comments
Labels
enhancement New feature or request

Comments

@polterguy
Copy link

polterguy commented Jun 3, 2020

The more I think about this problem, the more I realise that the library would probably benefit from an additional class, in-between CryptographyContext and OpenPgpContext, which is 100% "generic", and does not rely upon file system, OpenPGP, or anything else, with a couple of virtual methods to lookup private/public keys, allowing the class itself to take care of everything related to PGP encryption/decryption/signing/verification - But without relying upon the file system, expecting the consumer to implement a handful of abstract methods, to retrieve keys.

I actually tried to simply copy and paste the original OpenPgpContext, to remove all parts relying upon the file system, but quickly realised that this is flat our impossible, due to some of the methods it uses are internal, and cannot be accessed outside of the assembly itself - In addition to the general complexity of implementing this correctly.

What I am thinking, is to refactor and move all the parts related to PGP into a sub-class, that is injected in-between the above two classes, making this class public, allowing consumers to inherit directly from it, which becomes a slightly more "manual" job compared to simply subclassing the OpenPgpContext, but also allowing the user to completely control how to persist keys.

At the very least, this would be more in the spirit of LSP ... ;)

That way, I wouldn't have to inherit from OpenPgpContext, which anyways is wrong, in regards to LSP ...

Would you be willing to do such a thing ...?

I am assuming I cannot be the only guy on the planet wanting to handle my own keys in regards to PGP :)

@polterguy
Copy link
Author

Alternatively, if I do this, would you be interested in merging in such a change ...?

@polterguy
Copy link
Author

I kind of did this ... ;)

#576

@jstedfast
Copy link
Owner

I considered that as well when I made the other method virtualization changes. When I wrote the original OpenPgpContext class, I didn't really forsee anyone storing/retrieving keys from anywhere other than a local keyring. And realistically, it was designed around using it with GnuPG < 2.1.0 because GnuPG's keyring format had been, at the time, based on the keyring bundle format.

Starting with GnuPG 2.1.0, it is now some other format that I haven't been able to figure out. So yea, the design is suboptimal now.

@polterguy
Copy link
Author

Great, I just provided a PR for you, doing all the heavy lifting :)
You might have to clean it up slightly (documenting exceptions, following your existing nDoc standard), but I'm pretty sure it should work perfectly. All Unit Tests seems to pass, but there is that "download public key" failure, which I haven't been able to figure out. I actually suspect the key server might be down though ...!! :/

I tried to keep as close to the existing coding standard as I could, and I also kept the key server parts in the base class, allowing for automatic retrieval, importing into preferred key repository by overloading Import. Hopefully this is something you like. Maybe change the base class' name? Possibly PgpContextBase or something ...?

I could really need this for my vacation BTW, which starts Friday, since I intend to "go berserk" with your little lib, implementing something that has the potential for keeping millions of public keys around :)

It's a "Direct Democracy" thing, where people vote with cryptographically signed PGP MIME messages, over email ...

... pretty cool I think 🎱

@polterguy
Copy link
Author

How's this Pull Request doing? Do you want me to modify anything ...? :)

@jstedfast
Copy link
Owner

Hey, sorry - I didn't have time to take a close look at it yesterday (a quick overview looked good to me, though). I'm actually about to start looking it over closer right now before work.

@jstedfast jstedfast added the enhancement New feature or request label Jun 4, 2020
@polterguy
Copy link
Author

NP :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants