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

Add support for XEP-0373, XEP-0374: OpenPGP for XMPP: Instant Messaging #254

Merged
merged 1 commit into from Jul 30, 2018

Conversation

@vanitasvitae
Copy link
Member

@vanitasvitae vanitasvitae commented Jul 20, 2018

This PR adds support for XEP-0373: OpenPGP for XMPP, as well as XEP-0374: OpenPGP for XMPP: Instant Messaging.

There is still some WiP with this PR:

  • Manual message decryption
  • PR has entries in XEP table
  • Move OX-IM to own package
  • Replace Metadata by PainlessResult
  • SecretKeyPasswordCallback and SecretKeyDecryptor should be merged into one callback type
  • Trust management
  • Edge case: Receiving a message from a stranger might fail decrypting due to missing keys
  • MissingPublicKeyCallback is not yet incorporated
  • PR is squashed

Still, I opened it at this stage in order to get feedback and to accelerate the merge process :)

@vanitasvitae vanitasvitae force-pushed the openpgp branch 2 times, most recently from 05ab8ba to 63fc56d Jul 21, 2018
@vanitasvitae vanitasvitae force-pushed the openpgp branch 6 times, most recently from b126f10 to 9352ede Jul 25, 2018
@@ -76,7 +76,7 @@ public DiscoverInfo parse(XmlPullParser parser, int initialDepth)
if (parser.getName().equals("feature")) {
// Create a new feature and add it to the discovered info.
boolean notADuplicateFeature = discoverInfo.addFeature(variable);
assert (notADuplicateFeature);
// assert (notADuplicateFeature);
Copy link
Member

@Flowdalic Flowdalic Jul 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be reverted. What was the backstory again?

Loading

@vanitasvitae
Copy link
Member Author

@vanitasvitae vanitasvitae commented Jul 26, 2018

Ah yeah, I insterted that to be able to run integration tests and probably committed it on accident someday :)

My ejabberd sends a duplicate feature, thats why I had to uncomment that line to prevent smack from aborting.

processone/ejabberd#2470

Loading

throwIfPubSubNotSupported(conThree);

Security.removeProvider(BouncyCastleProvider.PROVIDER_NAME);
Security.addProvider(new BouncyCastleProvider());
Copy link
Member

@Flowdalic Flowdalic Jul 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly is the reason for this dance?

Loading

Copy link
Member Author

@vanitasvitae vanitasvitae Jul 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Older Android Versions do ship with crippled BC. The above code deals with that by removing any registered BC providers before adding our shipped version.

I'll add that as a comment :)

Loading

Copy link
Member Author

@vanitasvitae vanitasvitae Jul 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, maybe this is unnecessary in the integration tests, but it is best practice afaiu.

Loading

Copy link
Member

@Flowdalic Flowdalic Jul 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just about to ask if the integration tests will ever run on Android. On the other hand, the idea to have the integration tests ready for Android doesn't appear to be to bad. So just add a comment explaining what is going on.

Loading

Copy link
Member

@Flowdalic Flowdalic Jul 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But wait, shouldn't the dance be performed by smack-android-extensions?

Loading

Copy link
Member Author

@vanitasvitae vanitasvitae Jul 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't hurt to do it on non-Android systems, so I don't really care.
smack-openpgp does it now in the PainlessOpenPgpProvider.

Loading

@vanitasvitae
Copy link
Member Author

@vanitasvitae vanitasvitae commented Jul 26, 2018

Squashed again and fixed the commit message :)

Loading

@vanitasvitae
Copy link
Member Author

@vanitasvitae vanitasvitae commented Jul 26, 2018

Added some more commits. Let me know, when I should squash again :)

Loading

settings.gradle Outdated
'smack-repl'
'smack-repl',
'smack-openpgp'

Copy link
Member

@Flowdalic Flowdalic Jul 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: This adds an extra newline, please remove it.

Loading

@Flowdalic
Copy link
Member

@Flowdalic Flowdalic commented Jul 28, 2018

One minor nit, then please squash and this is good to go.

Loading

@vanitasvitae
Copy link
Member Author

@vanitasvitae vanitasvitae commented Jul 28, 2018

Done.

Loading

@Flowdalic
Copy link
Member

@Flowdalic Flowdalic commented Jul 29, 2018

Please mention at least the JIRA issue key in the commit message.

Loading

@Flowdalic
Copy link
Member

@Flowdalic Flowdalic commented Jul 29, 2018

But not in the commit subject :/

Loading

@vanitasvitae
Copy link
Member Author

@vanitasvitae vanitasvitae commented Jul 29, 2018

I don't have access to my computer right now, will fix that later this evening.

Loading

@vanitasvitae
Copy link
Member Author

@vanitasvitae vanitasvitae commented Jul 29, 2018

@Flowdalic fixed :)

Loading

@Flowdalic Flowdalic merged commit 0c2efbd into igniterealtime:master Jul 30, 2018
2 checks passed
Loading
@vanitasvitae vanitasvitae deleted the openpgp branch Jul 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants