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

WIP: keyserver search and get support #229

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

anarcat
Copy link
Contributor

@anarcat anarcat commented Nov 10, 2017

Basic primitives to search keyservers and fetch associated keys.

There are a lot of issues with this code, but it is a first proof of
concept. The first issue is we do not return a meaningful
datastructure in search: we should really return a NamedTuple or,
better, existing sturctures like PGPKey and PGPUID, but I couldn't
figure out how to attach all data fields. Also, key fields like
primary key expiration date are missing.

More importantly, we do not check at all what we get from the
keyserver. We blindly load this in a PGP key and hope for the
best. Maybe that's alright: we do not know what the caller wants to do
and maybe that should be left to the caller to do proper
verification. But we may want to check if the key we get matches the
fingerprint we load and raise otherwise.

There's also the question of how to do HTTP requests. Here we
introduce a dependency on the requests module, which works well in
my tests, but maybe a little advanced. It will allow us to easily do
HTTPS checks eventually, however. We also don't support hkp://
protocols - maybe that's just a requests adapter we should be
writing, actually.

There's also the question of how to run HTTP requests in the test
suite. In another project, I have used the betamax module to record
HTTP queries and replay them so that we could have a stable testbed
while offline, but also keep the network inputs controlled and
constant so that even if they change (e.g. a key is changed, the test
suite is unaffected.

Finally, I have no idea where to put this code. I put it in types.py
because that is where existing from_blob and from_file functions
were, so I figured it would make sense to put them there. But I really
don't know.

Basic primitives to search keyservers and fetch associated keys.

There are a lot of issues with this code, but it is a first proof of
concept. The first issue is we do not return a meaningful
datastructure in `search`: we should really return a NamedTuple or,
better, existing sturctures like PGPKey and PGPUID, but I couldn't
figure out how to attach all data fields. Also, key fields like
primary key expiration date are missing.

More importantly, we do not check at all what we get from the
keyserver. We blindly load this in a PGP key and hope for the
best. Maybe that's alright: we do not know what the caller wants to do
and maybe that should be left to the caller to do proper
verification. But we may want to check if the key we get matches the
fingerprint we load and raise otherwise.

There's also the question of how to do HTTP requests. Here we
introduce a dependency on the `requests` module, which works well in
my tests, but maybe a little advanced. It *will* allow us to easily do
HTTPS checks eventually, however. We also don't support hkp://
protocols - maybe that's just a requests `adapter` we should be
writing, actually.

There's also the question of how to run HTTP requests in the test
suite. In another project, I have used the `betamax` module to record
HTTP queries and replay them so that we could have a stable testbed
while offline, but also keep the network inputs controlled and
constant so that even if they change (e.g. a key is changed, the test
suite is unaffected.

Finally, I have no idea where to put this code. I put it in `types.py`
because that is where existing `from_blob` and `from_file` functions
were, so I figured it would make sense to put them there. But I really
don't know.
@anarcat
Copy link
Contributor Author

anarcat commented Nov 10, 2017

obviously, this is not ready to be merged, but i was looking at the hkp RFC and i was taking down notes, which turned into ipython code, which turned into code, which turned into unit tests, which turned into a bunch of questions i felt were better explained by an actual PR than just ... questions. :)

let me know if i should stop or if this is going somewhat the right way.

@anarcat
Copy link
Contributor Author

anarcat commented Nov 10, 2017

after discussing this with @Commod0re, it seems this should be either a separate or contrib module, or at least moved to a separate package, for example pgpy.network that would be better self-contained. that package could be first maintained out of tree (e.g. monkeysphere.network) while PGPy matures in its core functionality. then the code be merged when it's more mature and when pgpy is ready.

@anarcat
Copy link
Contributor Author

anarcat commented Nov 10, 2017

another conclusion reached in the discussion: the data structures returned by search should probably be more something like a tuple, because it would be very difficult to craft a fake PGPKey using the material available in the index output, and we don't want to do a get for every key we get.

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

Successfully merging this pull request may close these issues.

1 participant