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

OpenPGP/GnuPG WOT (web of trust) implementation - trustdb.gpg #119

Closed
wants to merge 18 commits into from

Conversation

nlmarco
Copy link

@nlmarco nlmarco commented Sep 18, 2015

As already discussed on the mailing list, I implemented support for GnuPG's trustdb.gpg, which manages the web-of-trust.

My implementation is a new sub-project called "pgwot". It is optional, i.e. it does not affect anyone who does not want to use it.

The current feature set of the implementation is:

  1. Read trustdb.gpg.
    a) Read the (previously calculated) validity of a key.
    b) Read key properties like disabled or owner-trust.

  2. Write trustdb.gpg.
    a) Set a key's owner-trust.
    b) Set a key's disabled flag.
    c) Recalculate the validity of all public-keys.
    d) Create a new, fresh trustdb.gpg.

  3. There's a key registry used to efficiently look up keys. This is needed by the validity-calculation, but may be useful for other people, too.
    a) Look up a key by its ID.
    b) Look up a key by its fingerprint.
    c) Look up all keys that have been signed (a.k.a. certified) by a certain key (identified by ID or fingerprint).

The following features are still missing:

  1. Support trust models other than 'PGP'.
  2. Remove entries from the trustdb.gpg - e.g. when a key was removed from the key ring(s).
  3. Read configuration settings like "how many marginals are needed" from GnuPG (or any other?) configuration file.

I currently do not plan to implement these missing features, because they are IMHO not important at all.

@dschuermann
Copy link

You should adapt your code style to that of Bouncy Castle:

@nlmarco
Copy link
Author

nlmarco commented Sep 18, 2015

arg... the code really still uses spaces for indent?! ... a truly out-dated style :-(

Anyway, as bad as they may be, the styles of an existing project must of course be accepted by new contributions. Are there Eclipse formatting rules that I could import and simply apply?

@dschuermann
Copy link

I don't know of formatting rules as an xml, I only know the rules from reading the code.

* spaces instead of tabs as indents (even though this is a bad style)
* braces in next line.
@nlmarco
Copy link
Author

nlmarco commented Sep 18, 2015

Thanks. I just adjusted the 2 things you mentioned: 4 spaces indent + braces in next line.

Acceptable now? Or more changes needed?

@dschuermann
Copy link

I think so, yes. The bouncy castle maintainers need to decide in the end :) @cwgit

@bcgit bcgit self-assigned this Nov 9, 2015
@bcgit
Copy link
Collaborator

bcgit commented Nov 9, 2015

Just so you know, this hasn't been forgotten about, bit busy with the FIPS library at the moment.

@bcgit
Copy link
Collaborator

bcgit commented Jan 27, 2016

I've started looking a this. I think we'll put it in org.bouncycastle.openpgp.gpg.

One question what was the idea behind the PgpKey class?

I should have mentioned earlier that we need things to compile using JDK 1.5.

@dschuermann
Copy link

I would like to see this in a separate gradle module, because not every PGP project uses the WoT, which is not part of the OpenPGP standard.

@nlmarco
Copy link
Author

nlmarco commented Jan 27, 2016

@dschuermann I agree and that's the reason why I implemented it in a separate project.

@bcgit The PgpKey serves as a data structure element bundling all the various technical keys (main-key-pair (private+public), sub-key-pairs (private+public)) together into one object representing what the user considers being one single OpenPGP key. This, together with the PgpKeyRegistry[Impl] and other related classes (e.g. PgpKeyId or PgpUserId) is needed to calculate the trusts both efficiently and in a way that keeps the code easily readable.

If you use a key management tool like e.g. Thunderbird-Enigmail and you go to "Manage keys", then a PgpKey in my code is what such a tool usually shows there as one table row. There's one PgpKey instance per main-key (a.k.a. primary key).

The API provided by BC's low-level-classes (e.g. PGPPublicKeyRing, PGPSecretKeyRing) is insufficient for calculating trust, because these low-level-classes don't provide an easy way to quickly access the things that belong together (e.g. to answer the question: Which keys are signed by key 0x12345? => PgpKeyRegistry.getPgpKeyFingerprintsSignedBy(...)).

@nlmarco
Copy link
Author

nlmarco commented Jan 27, 2016

Just wanted to be more precise & correct: Of course, the sub-keys are instances of PgpKey, too. They back-reference the master-key via their PgpKey.masterKey property. The master-keys have their sub-keys in the PgpKey.subKeys property. Hence, navigating is very easy and fast.

So to correct my previous statement: Every PgpKey having its masterKey == null (i.e. being a master-key itself) corresponds to what a tool like Enigmail shows as single row in its "Manage keys" table.

@bcgit
Copy link
Collaborator

bcgit commented Jan 28, 2016

Maybe I'll make it org.bouncycastle.gpg

With the key registry class, these operations are being done over key ring collections aren't they?

@nlmarco
Copy link
Author

nlmarco commented Jan 28, 2016

Yes, trust calculations are always done over all keys in ~/.gnupg/pubring.gpg (and ~/.gnupg/secring.gpg). This is necessary, because there is transitive trust.

A web-of-trust by definition always contains multiple nodes (otherwise it's not a web). The image shown here illustrates this very well: http://books.gigatux.nl/mirror/securitytools/ddu/ch09lev1sec1.html#ch09lev2sec15 (e.g. Tony ==trusts==> Sally ==trusts==> Jane)

Renaming the project to org.bouncycastle.gpg is a good idea. Maybe other GnuPG-specific things, besides the WOT, are going to be added in the future.

@nlmarco
Copy link
Author

nlmarco commented Jan 28, 2016

A bit more clarification: When using GnuPG, the user's point of view is to possess one single key ring. Technically, this single key ring is composed of three files, all of which are located in the directoy ~/.gnupg/:

  1. pubring.gpg
  2. secring.gpg
  3. trustdb.gpg

The files 1. and 2. are OpenPGP-standardized files. They can be read and written using BC's PGPPublicKeyRingCollection and PGPSecretKeyRingCollection.

So technically, we have 2 key-ring-collections, but semantically, the user understands this as being one single key ring. The user usually is not aware of the fact that what he considers a single key is actually technically already a key-ring consisting of a master-key and multiple sub-keys.

The file 3. is GnuPG-specific. It has the same file extension, but its file format is totally different. Reading and writing this file is implemented in my TrustDbIo class. This file is used to store additional data locally.

IIRC (it's a while since I implemented this), the user can store information only per master-key, but additional (calculated) information might be stored per sub-key, as well.

The most important information stored per master-key and modified by the user is the so-called owner-trust. This is a trust-level assigned to the owner of the key specifying how much the local user trusts this owner in his role as notary. It thus is the user's answer to the following questions (and maybe more):

  • How well does this person understand the system?
  • How diligently does this person verify the identity of another key before signing another key (i.e. certifying this other key's authenticity)?
  • Would this person certify another key's authenticity (= sign it) in order to betray me?

We're not talking about the usual trust alone. For example, I trust my mother completely and could thus answer the last question with a clear NEVER, but I would still assign only no or very low owner-trust to her master-key, because she doesn't understand the system (OpenPGP+WOT) at all ;-)

Based on the owner-trust and on the key-signatures (which are stored in the public keys, i.e. in the file pubring.gpg) the trust-database calculates transitive (indirect) trust.

Hence, whenever I change the owner-trust of a master-key, the web-of-trust needs to be recalculated as not only one single key's trust changes, but also all transitively signed (= certified) keys.

Needless to say, calculating the transitive trust is (especially with large WOTs) time-consuming. That's why the trustdb.gpg stores the calculated trust for each key persistently. And my TrustDb interface provides the API to read both this calculated data as well as the data assigned by the user (owner-trust, disabled etc.).

@bcgit
Copy link
Collaborator

bcgit commented Jan 29, 2016

I've checked in some initial work, at the moment I've added some extra functionality to the KeyRing, KeyRingCollection and PGPSignature classes in the regular API which I'm hoping will be enough to allow you to remove the key package. Also internal.Util appears to duplicate functionality provided by org.bouncycastle.util.Pack and org.bouncycastle.util.encoders.Hex.

@bcgit
Copy link
Collaborator

bcgit commented Apr 12, 2016

Fips is finally submitted. Are you able to remove the key package?

@nlmarco
Copy link
Author

nlmarco commented Apr 12, 2016

Sorry, I'm currently very busy with other projects - including my new-born child. Therefore, I didn't have time to reply to this, earlier.

Well, I took a look at your extensions a while ago and unfortunately, I don't have a work-space available right now (I'm on a business trip), hence I can't point out things in detail. But I still remember that your extensions were quite marginal compared to the functionality I put into the ...wot.key-package. So, no, it is definitely not possible to remove this package. And I think it makes no sense to try - I'll say more about this later.

To prevent misunderstandings - maybe I overlooked sth. - please kindly tell me, which precise extensions you made to replace the following classes:

PgpKeyRegistry[Impl]
PgpKey

Obviously, there are quite a few more classes in my ...wot.key-package and all of them would require a replacement, if the entire package was to be dropped. But the two classes mentioned above are the most complex ones, providing a way to easily and quickly navigate between keys and sub-keys, look up keys by fingerprint or key-id and so on - all functionality needed by the trust calculation.

Now, why do I think that trying to remove the ...wot.key-package makes no sense? Because the basic functionality provided by BC so far is minimalistic and sufficient. It does not contain any high-level logic related to key or key file management. It is focused on just reading an InputStream (or writing an OutputStream) and provide the contents as they are - in a pretty raw form as a simple List.

This raw form is sufficient for most use cases and since nobody required high-level logic, before, I fail to see why we should push this stuff into the base, now.

The trust calculation requires these high level data structures, because it must navigate back and forth across all keys. It is definitely a bad idea to do such navigation on raw lists (provided by the low-level BC code), because this would scale extremely bad (exponential!). Hence, the PgpKeyRegistry uses maps that provide a well-scaling (constant!) high-level access to keys. And - as expected - trust calculation is thus very fast - even with bigger key rings (I personally have a few hundred master-keys (with each at least 2 sub-keys).

If someone wants to use the PgpKeyRegistry, he can add a dependency on the new wot-module. In this case, it likely doesn't hurt that there is the trust-code, too. But all those who need PGP as lean as possible don't get any new additional code bloated into their dependencies.

That's why I think it makes sense to keep the wot-module separately and to keep the high-level key management (i.e. PgpKeyRegistry and all related classes in ...wot.key) in this separate module - where it currently is.

@bcgit
Copy link
Collaborator

bcgit commented Apr 12, 2016

I guess congratulations are in order! I can certainly understand why you're a bit busy. My son was born 8 years ago, don't think I've even recovered now!

In terms of changes, for example PGPPublicKeyRingCollection now has methods for finding keys, or key rings, by fingerprint, and by signature key ID.

The way I'm trying to play this is your patch has 2 aspects, one is about the key searching, which as you say, can be improved in the base API. This is general improvement that will be of benefit to other people using the APIs for different purposes as well. The second aspect is concerning the GPG WOT model, which isn't of quite the same benefit, but I'm nonetheless sure some people will find useful.

So the first thing I'm trying to do is update the API for the general improvement, once we've done that we'll be able to work out how best to package the WOT classes. At the moment I'm not especially worried about efficiency, correctness of the API is the starting point, the internals can always be made faster.

One thing I should probably also mention is that you'll probably notice in the main API the File class isn't used a lot. We've found it's a mistake with a lot of things to assume the data will be in a file, storing information like keyrings in a database is surprisingly common.

@nlmarco
Copy link
Author

nlmarco commented Mar 17, 2017

Hi again,

I needed this lib in a new, separate project and thus decided to invest a few hours into setting up a repository, a proper Jenkins build job and deployment into a Maven repository. See here:

https://github.com/subshare/bc-pgp-wot/

I think that our opinions are too different and most importantly this pull request consumes too much time from me. You think that you do not need HashMaps? Fine. But I strongly disagree! And I certainly won't change my code to exponentially worse performance iterating in a multiple nested way over potentially very long lists.

I have explained in detail why I did the things the way I did them on 2016-04-12 and before. You obviously disagree. That's fine for me -- I can't force you to accept my code donation ;-)

Please don't get me wrong! I really appreciate the work you did and continue to do with BouncyCastle! I'm really grateful and I use this library whenever I have to do encryption stuff. But my time is very limited and from the above discussion I conclude that we won't reach a consensus quickly.

However, if you change your mind, you are very welcome to still include my WOT! Please let me know, then. But please do not pull this pull request, anymore! I have made further changes, because I totally agree that accessing java.io.File should be abstracted away from the actual code. And I just did this -- see subshare/bc-pgp-wot#2

@nlmarco nlmarco closed this Mar 17, 2017
@bcgit bcgit removed their assignment May 26, 2020
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.

None yet

3 participants