Lower required Android API to 9 and use Animal Sniffer #18

vanitasvitae · 2018-05-02T16:17:17Z

Hi!
This PR removes the usage of some newer Java features like stream semantics in order to allow the library to be used in Android applications targeting API level 9 and above.
I hope I didn't break anything, but the test cases (which I had to alter as well) are all passing.

I created this PR as I'm participating in this years Google Summer of Code. As part of my project I want to utilize this library to implement XEP-0373 and XEP-0374 (OpenPGP for XMPP) for the XMPP client library Smack.

I appreciate any feedback :)

neuhalje · 2018-05-02T19:13:32Z

...projects/crypto/symmetric/keygeneration/impl/stretching/MasterKeyFromPasswordDerivation.java

-    buffer.get(identifierByteRepresentation);
-    return identifierByteRepresentation;
+    return identifier.getBytes(Charset.forName("UTF-8"));
+    // final ByteBuffer buffer = StandardCharsets.UTF_8.encode(CharBuffer.wrap(identifier));


Can this be removed? (the comments)

The commented stuff can go, I missed that bit. Thank you :)

neuhalje · 2018-05-02T19:14:58Z

...jects/crypto/bouncycastle/openpgp/keys/callbacks/RFC4880TestKeyringsDedicatedSigningKey.java

-  public final static Instant EXPIRED_KEY_CREATION_TIME = ZonedDateTime
-      .parse("2018-03-25T10:55:31Z").toInstant();
+  public final static Date EXPIRED_KEY_CREATION_TIME =
+          new Date(1521975331000L);


Please use a 'readable' date

I'll have to look into how to parse a date string directly into a Date, but I will see what I can do :)

new SimpleDateFormat("yyyy-MM-dd HH:mm").parse("2018-12-31 23:59") should so the trick :-)

neuhalje · 2018-05-02T19:15:36Z

...ects/crypto/bouncycastle/openpgp/roundtrip/EncryptionDecryptionRoundtripIntegrationTest.java

@@ -327,7 +329,7 @@ public void encrypt_decrypt_yieldsOriginalPlaintext()
    final OutputStream outputStream = BouncyGPG
        .encryptToStream()
        .withConfig(Configs.keyringConfigFromFilesForSender())
-        .setReferenceDateForKeyValidityTo(Instant.MAX)
+        .setReferenceDateForKeyValidityTo(new Date(Long.MAX_VALUE))


Honestly not sure if this is supported (haven't checked it though)

Instant.MAX will create an Instant representing "1000000000-12-31T23:59:59.999999999Z", while new Date(Long.MAX_VALUE) will represent "292278994-17-08T00:12:55MST". I'm not sure if this is an issue?

this is fine then. neither will hit any of us during our livetime :-)

neuhalje · 2018-05-02T19:42:21Z

.../java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/keys/keyrings/InMemoryKeyring.java

@@ -63,11 +63,13 @@ public void addPublicKey(byte[] encodedPublicKey) throws IOException, PGPExcepti
      this.publicKeyRings = PGPPublicKeyRingCollection
              .addPublicKeyRing(this.publicKeyRings, pgpPub);
    }
-    catch (IOException e) {
+    catch (Exception e) {


Why that? have you experienced 'non-IOSExceptions'?

I want to make sure that the Streams are also closed on PGPExceptions.

neuhalje · 2018-05-02T19:43:17Z

Hi Paul,

thanks for the contribution! Although possible, I am a bit reluctant to force the library to Java 7 features. But this is not necessarily a blocker (for now).

Are you restricted to Android API 9? Java 8 is supported with SDK 24. SDK 24 is used on ~35% of all devices.

From what I have heard Bouncy Castle does not run well on android, but that could be old news.

Have you already tried to run Bouncy Castle on API v9 devices? (see the linked Spongy Castle issue)

vanitasvitae · 2018-05-02T19:51:42Z

I am a bit reluctant to force the library to Java 7 features.

I'm also fine with maintaining my own fork for now :) I just hope that my contributions might benefit others, so I try to upstream as much as possible :D

neuhalje · 2018-05-02T19:58:05Z

I am all for "upstream" commits :-). The codebases does not use many Java 8 features, so this is no real loss (yet). If my lib helps your project I am willing to merge.

I'll do a more thorough review tomorrow (or the day after)!

Out of curiosity: Both XEPs state

WARNING: This document has been automatically Deferred after 12 months of inactivity in its previous Experimental state. Implementation of the protocol described herein is not recommended for production systems. However, exploratory implementations are encouraged to resume the standards process.

vanitasvitae · 2018-05-02T20:53:44Z

WARNING: This document has been automatically Deferred after 12 months of inactivity in its previous Experimental state. Implementation of the protocol described herein is not recommended for production systems. However, exploratory implementations are encouraged to resume the standards process.

That only means that there was no input on the XEP after one year. There are no implementations afaik, so I'm kinda sailing into unknown waters :)

neuhalje

The major thing is: this is breaking the API (Instant --> Date). Can you relax your API requirements?

neuhalje · 2018-05-04T05:54:51Z

.../java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/BuildDecryptionInputStreamAPI.java

-      BuildDecryptionInputStreamAPI.this.keySelectionStrategy = new Rfc4880KeySelectionStrategy(
-          Instant.now());
+      BuildDecryptionInputStreamAPI.this.keySelectionStrategy =
+              new Rfc4880KeySelectionStrategy(new Date());
    }

    public Validation setReferenceDateForKeyValidityTo(


Changing public Validation setReferenceDateForKeyValidityTo(Instant) to public Validation setReferenceDateForKeyValidityTo(Date) would break the published API. Although 2.1 is not yet used widely this is not good. Unfortunately Instant got added to android in API Level 26.

The best way would be to loosen your "support (very) old devices" requirement.

neuhalje · 2018-05-04T05:56:51Z

...java/name/neuhalfen/projects/crypto/bouncycastle/openpgp/BuildEncryptionOutputStreamAPI.java

    }

    public WithAlgorithmSuite setReferenceDateForKeyValidityTo(
-        Instant dateOfTimestampVerification) {
+        Date dateOfTimestampVerification) {


See above (breaks API)

neuhalje · 2018-05-04T05:57:36Z

...uhalfen/projects/crypto/bouncycastle/openpgp/keys/callbacks/Rfc4880KeySelectionStrategy.java


  /**
   * The date used for key expiration date checks as "now".
   *
   * @return dateOfTimestampVerification
   */
-  protected Instant getDateOfTimestampVerification() {
+  protected Date getDateOfTimestampVerification() {
    return dateOfTimestampVerification;


Also breaks API, it is possible but unlikely that this will cause a major fuss (I'll skip over the next "breaking changes" )

neuhalje · 2018-05-04T06:03:07Z

...uhalfen/projects/crypto/bouncycastle/openpgp/keys/callbacks/Rfc4880KeySelectionStrategy.java

@@ -116,52 +119,61 @@ public PGPPublicKey selectPublicKey(PURPOSE purpose, String uid, KeyringConfig k

    final PGPSecretKeyRingCollection secretKeyRings = keyringConfig.getSecretKeyRings();

+    PGPPublicKey publicKey = null;


Add a comment to the method (if there is none yet) to say that the last matching key is selected ("use the newest").

neuhalje · 2018-05-04T06:03:55Z

...uhalfen/projects/crypto/bouncycastle/openpgp/keys/callbacks/Rfc4880KeySelectionStrategy.java

-            .filter(hasPrivateKey(secretKeyRings))
-            .reduce((a, b) -> b)
-            .orElse(null);
+        for (PGPPublicKeyRing ring : publicKeyrings) {


Maybe move this to a dedicated method?

neuhalje · 2018-05-04T06:06:12Z

...uhalfen/projects/crypto/bouncycastle/openpgp/keys/callbacks/Rfc4880KeySelectionStrategy.java

@@ -179,10 +191,10 @@ protected boolean isExpired(PGPPublicKey pubKey) {
    final boolean isExpired;

    if (hasExpiryDate) {
-      final Instant expiryDate = pubKey.getCreationTime().toInstant()
-          .plusSeconds(pubKey.getValidSeconds());
+      final Date expiryDate = new Date(pubKey.getCreationTime().getTime()


This needs a comment, or better yet: Using the (ugly) Calendar API:

https://stackoverflow.com/questions/8035125/java-how-to-add-seconds-to-timestamp

neuhalje · 2018-05-04T06:09:32Z

...ects/crypto/bouncycastle/openpgp/roundtrip/EncryptionDecryptionRoundtripIntegrationTest.java

@@ -327,7 +329,7 @@ public void encrypt_decrypt_yieldsOriginalPlaintext()
    final OutputStream outputStream = BouncyGPG
        .encryptToStream()
        .withConfig(Configs.keyringConfigFromFilesForSender())
-        .setReferenceDateForKeyValidityTo(Instant.MAX)
+        .setReferenceDateForKeyValidityTo(new Date(Long.MAX_VALUE))


this is fine then. neither will hit any of us during our livetime :-)

vanitasvitae · 2018-05-05T17:38:30Z

The best way would be to loosen your "support (very) old devices" requirement.

Unfortunately that is not an option for me. Smack is apparently used primarily in developing countries, so having support for old devices is a requirement.

I think for now I will go with creating a fork of your project that fits my needs :)

neuhalje · 2018-05-06T12:44:38Z

No worries! If I can help you, just give me a note (create an issue :-) ), and if it is good for bouncy_gpg I'll add it, so you can merge into your fork.

vanitasvitae added 5 commits April 25, 2018 15:12

Add .iml files to gitignore

9890764

Enable building on Android API 9

39a7587

Use animalsniffer

5c914f7

Also catch PGPExceptions

1d27969

Fix codacy issues

781cba5

neuhalje reviewed May 2, 2018

View reviewed changes

Remove commented code, use readable date format

f937267

neuhalje reviewed May 4, 2018

View reviewed changes

neuhalje closed this May 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lower required Android API to 9 and use Animal Sniffer #18

Lower required Android API to 9 and use Animal Sniffer #18

vanitasvitae commented May 2, 2018 •

edited

Loading

neuhalje May 2, 2018 •

edited

Loading

vanitasvitae May 2, 2018

neuhalje May 2, 2018

vanitasvitae May 2, 2018

neuhalje May 2, 2018

neuhalje May 2, 2018

vanitasvitae May 3, 2018

neuhalje May 4, 2018

neuhalje May 2, 2018

vanitasvitae May 2, 2018

neuhalje commented May 2, 2018

vanitasvitae commented May 2, 2018

neuhalje commented May 2, 2018

vanitasvitae commented May 2, 2018

neuhalje left a comment •

edited

Loading

neuhalje May 4, 2018

neuhalje May 4, 2018

neuhalje May 4, 2018

neuhalje May 4, 2018

neuhalje May 4, 2018

neuhalje May 4, 2018

neuhalje May 4, 2018

vanitasvitae commented May 5, 2018

neuhalje commented May 6, 2018 •

edited

Loading

		@@ -116,52 +119,61 @@ public PGPPublicKey selectPublicKey(PURPOSE purpose, String uid, KeyringConfig k

		final PGPSecretKeyRingCollection secretKeyRings = keyringConfig.getSecretKeyRings();

		PGPPublicKey publicKey = null;

Lower required Android API to 9 and use Animal Sniffer #18

Lower required Android API to 9 and use Animal Sniffer #18

Conversation

vanitasvitae commented May 2, 2018 • edited Loading

neuhalje May 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neuhalje commented May 2, 2018

vanitasvitae commented May 2, 2018

neuhalje commented May 2, 2018

vanitasvitae commented May 2, 2018

neuhalje left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vanitasvitae commented May 5, 2018

neuhalje commented May 6, 2018 • edited Loading

vanitasvitae commented May 2, 2018 •

edited

Loading

neuhalje May 2, 2018 •

edited

Loading

neuhalje left a comment •

edited

Loading

neuhalje commented May 6, 2018 •

edited

Loading