Summer of Code: Smack has OpenPGP Support!

I am very proud to announce, that Smack got support for OpenPGP for XMPP!

Today the Pull Request I worked on during my GSoC project was merged into Smacks master branch. Admittedly it will take a few months until smack-openpgp will be included in a Smack release, but that gives me time to further finalize the code and iron out any bugs that may be in there. If you want to try smack-openpgp for yourself, let me know of any issues you encounter 🙂

(Edit: There are snapshot releases of Smack available for testing)

Now Smack does support two end-to-end encryption methods, which complement each other perfectly. OMEMO is best for people that want to be safe from future attacks, while OpenPGP is more suited for users who appreciate being able to access their chat history at any given time. OpenPGP is therefore the better choice for web based applications, although it is perfectly possible to implement web based clients that do OMEMO (see for example the Wire web app, which does ratcheting similar to OMEMO).

What’s left to do now is updating smack-openpgp due to updates made to XEP-0373 and extensive testing against other implementations.

Happy Hacking!

Summer of Code: Finalizing the PR

Quick update:

Only a few days are left until the last and final Evaluation Phase.

I spent the week opening my pull request against Smacks master branch and adding a basic trust management implementation. Now the user is required to make decisions whether to trust a contacts key or not. However, the storage implementation is kept very modular, so an implementor can easily create a trust store implementation that realizes custom behaviour.

Smack-openpgp now allows users which did not subscribe to one another to exchange encryption keys quite easily. If a user receives an encrypted message, the implementation automatically fetches the senders keys to allow signature verification.

Furthermore there are more JUnit tests now, so that Smacks total test coverage actually increases when my PR gets merged 😀

Happy Hacking!

Summer of Code: First PGPainless Release!

I’m very happy and proud to announce the first alpha release of PGPainless!

PGPainless 0.0.1-alpha1 is the first non-snapshot release and is available from maven central. It was an interesting experience to go through the process of creating a release and I’m looking forward to have many more releases in the future 🙂

The current release contains a workaround for the bug I described in an earlier blog post. The issue was, that bouncycastle wouldn’t mark the public sub keys of a secret key ring as sub keys, which results in loss of keys if the user tries to create a public key ring from the exported public keys. My workaround fixes the issue by iterating through all sub keys of an existing key ring and converting the key packages of subkeys to subkey packages. The code is also available as a gist.

Ironically I had some issues related to OpenPGP during the release process. Releases to maven central have to be signed with an OpenPGP key, so I created a dedicated signing key pair using GnuPG, which I wanted to put into a separate GPG key box. After creating the key, I exported it using

gpg --armor --export-secret-keys [key-id] > pgpainless-singing-key.asc

imported it into a dedicated key box using

gpg --no-default-keyring --keyring pgpainless.gpg --import pgpainless-signing-key.asc

and deleted the key from my old key box, as well as the .asc-file. But when I tried to sign my release, I got the error, that a secret key would be needed. After checking the key box, I noticed, that only a public key was present.

Unfortunately that key had already been published to the key servers and I have no way to revoke it, due to lack of a secret key. I have no idea, what exactly happened or how it could happen, but its too late to recover the key.

Edit: I found my error: Importing a secret key is only possible with the flag `–allow-secret-key-import`, eg

gpg --import --allow-secret-key-import key.asc

So in the end I had to create a new OpenPGP key, which I now carefully backed up on a freshly bought USB stick which will be locked away for the event that I lose the copy on my work station. Better safe than sorry.

Happy Hacking!

Summer of Code: Plan for the grand finale

I passed the second evaluation phase 🙂 Now begins the final spurt, as the last month of GSoC has begun. My main goal can be summarized as follows: Get everything merged!

To get that done, I have to polish up my smack-openpgp branch which has grown to a size of 7000 loc. There are still some minor quirks, but Florian recommended to focus on the big picture instead of spending too much time on small details and edge cases.

I also have to release pgpainless to maven central and establish some kind of release cycle. It will be a future challenge for me personally to synchronize the releases of smack-openpgp and pgpainless.

But now enough talking, I have to get to work 🙂

Happy Hacking!

Summer of Code: Second evaluation phase

Quite some time has passed since I bothered you with my last post 🙂 A lot has happened since, I have been making steady process in both smack-openpgp, as well as pgpainless.

One big step that I took was to get rid of smack-openpgp-bouncycastle, which now has been merged into smack-openpgp. Having modular code may be worthwhile, however it poses some big challenges. The biggest problem with having smack-openpgp not depend on smack-openpgp-bouncycastle was, that I could not use classes that represent encryption keys directly in smack-openpgp. Instead I had to create interfaces that encapsule functionality and call those in order to get stuff done from inside smack-openpgp. Last week me and flow decided that it would make my job a lot easier if we just got rid of smack-openpgp-bouncycastle by merging the two modules. In case there will be another implementation at some point, the code would still be modular enough to allow extension by overriding classes and methods.

Now smack-openpgp depends on pgpainless directly, which means that I don’t have to create duplicate code to get bundled information from pgpainless to smack-openpgp for instance. This change gave me a huge performance boost in the development process, as it makes the next steps much more clear for me due to less abstraction.

I rewrote the whole storage backend of smack-openpgp, keeping everything as modular as possible. Now there are 3 different store types. One store is responsible for keys, another one for metadata and a third one for trust decisions. For all of those I created a file-based implementation which just writes information to files. An implementor can for example chose to write information to a database instead. For all those store classes I wrote a parametrized junit test, meaning new implementations can easily be tested by simply inserting an instance of the new store into an array.

Unfortunately I stumbled across yet another bug in bouncycastle, which makes it necessary to implement a workaround in my project until a patched version of bouncycastle is released.
The issue was, that a key ring which consists of a master key and some subkeys was not exported correctly. The subkeys would be exported as normal keys, which caused the constructor of the key ring to skip those, as it expected sub keys, not normal keys. That lead to the subkeys getting lost, which caused smack-openpgp to be unable to encrypt messages for contacts which use a master key and subkeys for OpenPGP.

This bug has been fixed pretty fast by the bouncycastle team and the minimal test I created to illustrate my problem has been incorporated into bouncycastle. Thank you 🙂

Currently I’m working on a workaround for the bug in smack-openpgp, but that work is already working. Next I will polish up my test client and do some more field testing to iron out all the edge cases I probably overlooked 🙂

Happy Hacking!

Summer of Code: Checkstyle to the rescue!

Today I added some checkstyle rules to PGPainless.Checkstyle is a gradle plugin, which checks the source code for style violations.

Some say, strict checkstyle rules are unnecessary and that it is annoying to be held back from pushing a commit to the master branch only to fix “style issues” for half an hour. I must say, in the beginning I thought the same way. I was annoyed thinking “why does it matter, if a line comment ends with a period or not?” But after being forced to give it a try when I first became a contributor to the Smack project, I became a fan of it. In the beginning I had to often recommit my changes because they broke the checkstyle rules. For example I often forgot to leave spaces between mathematical operators. I would write “i = 5+5;” instead of “i = 5 + 5;”. But after some amount of time, I got less and less warnings.

I adopted most of the (honestly *very* strict) rules in Smacks rule set to my own coding style. I like how it automatically leads to cleaner, more uniform code (not that it is impossible to write garbage with it of course). For that reason, I decided to put those rules into place in PGPainless today (I only left one rule out, because who the hell cares about the alphabetical sorting of imports???).

At some point, PGPainless will be released as a maven artifact. In preparation for this historical event, I bought the domain pgpainless.org. For now it is just a forwarding to the PGPainless git repository, but I will likely setup a small website with documentation etc. at some point.

During my testing of Smacks OX implementation, I came across an interesting problem. When a user queries a PubSub node in Smack, Smack first does a disco#info query on that node to determine, whether it is a LeafNode or a CollectionNode. This normally works fine. However, it becomes more and more popular to make use of the PubSub access model ‘open’. The open access model makes a PubSub node accessible to entities (like other users) which are not in the contact list of the user. This enables the use of OMEMO in group chats, where not every participant is in your contact list for example.

The problem is that a server which allows access to open PubSub nodes, does not necessarily allow the disco#info query. The question is: Should disco#info queries on open PubSub nodes be allowed or not? An argument against it is, that it might allow “jid-harvesting”. An attacker might use disco#info queries on open PubSub nodes in order to determine, whether the user exists or not. This is a bad thing, because it allows spammers to collect the Jabber IDs of potential victims. On the other hand however, the attacker could simply do a direct PubSub query on the open node and the result would be the same. The benefit of allowing disco#info queries would be, that you can in fact determine the node type.
For now my mail to the standards mailing list remained unanswered, but I think that there should be a well defined expected behavior for this edge case.

For now I worked around the issue by using Javas reflections to access the LeafNode constructor directly, avoiding the disco#info query.

Other than that, I didn’t get a whole lot done this week. Unlike the demotivating week though, this time the reason was primarily exciting new hardware 😀

Happy Hacking!