From 800955d5a8afbf749b98b642d249bff65e60731a Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Thu, 14 Jun 2018 16:02:53 +0200 Subject: [PATCH] Fix some runtime bugs --- .../AbstractPainlessOpenPgpStore.java | 6 ++ .../FileBasedPainlessOpenPgpStore.java | 9 +- .../bouncycastle/PainlessOpenPgpProvider.java | 85 ++++++++++++++++--- .../smackx/ox/OpenPgpManager.java | 6 +- .../smackx/ox/OpenPgpProvider.java | 6 +- .../smackx/ox/util/PubSubDelegate.java | 11 ++- 6 files changed, 105 insertions(+), 18 deletions(-) diff --git a/smack-openpgp-bouncycastle/src/main/java/org/jivesoftware/smackx/ox/bouncycastle/AbstractPainlessOpenPgpStore.java b/smack-openpgp-bouncycastle/src/main/java/org/jivesoftware/smackx/ox/bouncycastle/AbstractPainlessOpenPgpStore.java index 9b355018b..2f05d9999 100644 --- a/smack-openpgp-bouncycastle/src/main/java/org/jivesoftware/smackx/ox/bouncycastle/AbstractPainlessOpenPgpStore.java +++ b/smack-openpgp-bouncycastle/src/main/java/org/jivesoftware/smackx/ox/bouncycastle/AbstractPainlessOpenPgpStore.java @@ -66,6 +66,9 @@ public abstract class AbstractPainlessOpenPgpStore implements PainlessOpenPgpSto } byte[] bytes = loadPublicKeyRingBytes(owner); + if (bytes == null) { + return null; + } keyRing = new PGPPublicKeyRingCollection(bytes, fingerprintCalculator); publicKeyRings.put(owner, keyRing); @@ -81,6 +84,9 @@ public abstract class AbstractPainlessOpenPgpStore implements PainlessOpenPgpSto } byte[] bytes = loadSecretKeyRingBytes(owner); + if (bytes == null) { + return null; + } keyRing = new PGPSecretKeyRingCollection(bytes, fingerprintCalculator); secretKeyRings.put(owner, keyRing); diff --git a/smack-openpgp-bouncycastle/src/main/java/org/jivesoftware/smackx/ox/bouncycastle/FileBasedPainlessOpenPgpStore.java b/smack-openpgp-bouncycastle/src/main/java/org/jivesoftware/smackx/ox/bouncycastle/FileBasedPainlessOpenPgpStore.java index a99b92a81..862b13272 100644 --- a/smack-openpgp-bouncycastle/src/main/java/org/jivesoftware/smackx/ox/bouncycastle/FileBasedPainlessOpenPgpStore.java +++ b/smack-openpgp-bouncycastle/src/main/java/org/jivesoftware/smackx/ox/bouncycastle/FileBasedPainlessOpenPgpStore.java @@ -27,6 +27,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.nio.charset.Charset; import java.nio.file.Files; +import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.HashSet; @@ -160,7 +161,7 @@ public class FileBasedPainlessOpenPgpStore extends AbstractPainlessOpenPgpStore Set fingerprints = new HashSet<>(); try { PGPSecretKeyRingCollection secretKeys = getSecretKeyRings(owner); - for (PGPSecretKeyRing s : secretKeys) { + for (PGPSecretKeyRing s : secretKeys != null ? secretKeys : Collections.emptySet()) { fingerprints.add(PainlessOpenPgpProvider.getFingerprint(s.getPublicKey())); } } catch (IOException | PGPException e) { @@ -176,7 +177,7 @@ public class FileBasedPainlessOpenPgpStore extends AbstractPainlessOpenPgpStore try { PGPPublicKeyRingCollection publicKeys = getPublicKeyRings(contact); Set fingerprints = new HashSet<>(); - for (PGPPublicKeyRing ring : publicKeys) { + for (PGPPublicKeyRing ring : publicKeys != null ? publicKeys : Collections.emptySet()) { OpenPgpV4Fingerprint fingerprint = PainlessOpenPgpProvider.getFingerprint(ring.getPublicKey()); fingerprints.add(fingerprint); } @@ -252,8 +253,8 @@ public class FileBasedPainlessOpenPgpStore extends AbstractPainlessOpenPgpStore LOGGER.log(Level.WARNING, "Encountered illegal line in file " + file.getAbsolutePath() + ": " + lineNr, e); } - reader.close(); } + reader.close(); } catch (IOException e) { LOGGER.log(Level.WARNING, "Could not read fingerprints and dates from file " + file.getAbsolutePath(), e); if (reader != null) { @@ -379,7 +380,7 @@ public class FileBasedPainlessOpenPgpStore extends AbstractPainlessOpenPgpStore } private File getContactsPath(BareJid owner, boolean create) throws IOException { - File path = new File(getContactsPath(create) + owner.toString()); + File path = new File(getContactsPath(create), owner.toString()); if (create && !path.exists()) { createDirectoryOrThrow(path); } diff --git a/smack-openpgp-bouncycastle/src/main/java/org/jivesoftware/smackx/ox/bouncycastle/PainlessOpenPgpProvider.java b/smack-openpgp-bouncycastle/src/main/java/org/jivesoftware/smackx/ox/bouncycastle/PainlessOpenPgpProvider.java index 726a91488..d29a92bae 100644 --- a/smack-openpgp-bouncycastle/src/main/java/org/jivesoftware/smackx/ox/bouncycastle/PainlessOpenPgpProvider.java +++ b/smack-openpgp-bouncycastle/src/main/java/org/jivesoftware/smackx/ox/bouncycastle/PainlessOpenPgpProvider.java @@ -24,12 +24,15 @@ import java.io.OutputStream; import java.security.InvalidAlgorithmParameterException; import java.security.NoSuchAlgorithmException; import java.security.NoSuchProviderException; +import java.util.Collections; import java.util.HashSet; +import java.util.Iterator; import java.util.Set; import org.jivesoftware.smack.util.MultiMap; import org.jivesoftware.smackx.ox.OpenPgpProvider; import org.jivesoftware.smackx.ox.OpenPgpV4Fingerprint; +import org.jivesoftware.smackx.ox.bouncycastle.selection_strategy.BareJidUserId; import org.jivesoftware.smackx.ox.callback.SmackMissingOpenPgpPublicKeyCallback; import org.jivesoftware.smackx.ox.element.CryptElement; import org.jivesoftware.smackx.ox.element.SignElement; @@ -46,11 +49,14 @@ import de.vanitasvitae.crypto.pgpainless.decryption_verification.DecryptionStrea import de.vanitasvitae.crypto.pgpainless.decryption_verification.MissingPublicKeyCallback; import de.vanitasvitae.crypto.pgpainless.decryption_verification.PainlessResult; import de.vanitasvitae.crypto.pgpainless.key.generation.type.length.RsaLength; +import de.vanitasvitae.crypto.pgpainless.util.BCUtil; import org.bouncycastle.openpgp.PGPException; import org.bouncycastle.openpgp.PGPPublicKey; import org.bouncycastle.openpgp.PGPPublicKeyRing; import org.bouncycastle.openpgp.PGPPublicKeyRingCollection; import org.bouncycastle.openpgp.PGPSecretKeyRing; +import org.bouncycastle.openpgp.PGPSecretKeyRingCollection; +import org.bouncycastle.openpgp.operator.bc.BcKeyFingerprintCalculator; import org.bouncycastle.util.encoders.Hex; import org.bouncycastle.util.io.Streams; import org.jxmpp.jid.BareJid; @@ -72,7 +78,7 @@ public class PainlessOpenPgpProvider implements OpenPgpProvider { throws MissingOpenPgpKeyPairException, MissingOpenPgpPublicKeyException, SmackOpenPgpException, IOException { - Set allRecipientsKeys = getEncryptionKeys(encryptionKeys); + PGPPublicKeyRing[] validEncryptionKeys = getEncryptionKeys(encryptionKeys); PGPSecretKeyRing signingKeyRing = getSigningKey(signingKey); InputStream fromPlain = element.toInputStream(); @@ -81,7 +87,7 @@ public class PainlessOpenPgpProvider implements OpenPgpProvider { try { encryptor = PGPainless.createEncryptor() .onOutputStream(encrypted) - .toRecipients((PGPPublicKeyRing[]) allRecipientsKeys.toArray()) + .toRecipients(validEncryptionKeys) .usingSecureAlgorithms() .signWith(store.getSecretKeyProtector(), signingKeyRing) .noArmor(); @@ -129,7 +135,7 @@ public class PainlessOpenPgpProvider implements OpenPgpProvider { @Override public byte[] encrypt(CryptElement element, MultiMap encryptionKeyFingerprints) throws MissingOpenPgpPublicKeyException, IOException, SmackOpenPgpException { - Set allRecipientsKeys = getEncryptionKeys(encryptionKeyFingerprints); + PGPPublicKeyRing[] allRecipientsKeys = getEncryptionKeys(encryptionKeyFingerprints); InputStream fromPlain = element.toInputStream(); ByteArrayOutputStream encrypted = new ByteArrayOutputStream(); @@ -137,7 +143,7 @@ public class PainlessOpenPgpProvider implements OpenPgpProvider { try { encryptor = PGPainless.createEncryptor() .onOutputStream(encrypted) - .toRecipients((PGPPublicKeyRing[]) allRecipientsKeys.toArray()) + .toRecipients(allRecipientsKeys) .usingSecureAlgorithms() .doNotSign() .noArmor(); @@ -210,7 +216,7 @@ public class PainlessOpenPgpProvider implements OpenPgpProvider { return store; } - private Set getEncryptionKeys(MultiMap encryptionKeys) + private PGPPublicKeyRing[] getEncryptionKeys(MultiMap encryptionKeys) throws IOException, SmackOpenPgpException { Set allRecipientsKeys = new HashSet<>(); @@ -227,7 +233,13 @@ public class PainlessOpenPgpProvider implements OpenPgpProvider { } } - return allRecipientsKeys; + PGPPublicKeyRing[] allEncryptionKeys = new PGPPublicKeyRing[allRecipientsKeys.size()]; + Iterator iterator = allRecipientsKeys.iterator(); + for (int i = 0; i < allEncryptionKeys.length; i++) { + allEncryptionKeys[i] = iterator.next(); + } + + return allEncryptionKeys; } private PGPSecretKeyRing getSigningKey(OpenPgpV4Fingerprint signingKey) @@ -256,13 +268,66 @@ public class PainlessOpenPgpProvider implements OpenPgpProvider { } @Override - public OpenPgpV4Fingerprint importPublicKey(BareJid owner, byte[] bytes) throws MissingUserIdOnKeyException { - return null; + public OpenPgpV4Fingerprint importPublicKey(BareJid owner, byte[] bytes) + throws MissingUserIdOnKeyException, IOException, SmackOpenPgpException { + PGPPublicKeyRing publicKeys = new PGPPublicKeyRing(bytes, new BcKeyFingerprintCalculator()); + return importPublicKey(owner, publicKeys); + } + + public OpenPgpV4Fingerprint importPublicKey(BareJid owner, PGPPublicKeyRing ring) + throws SmackOpenPgpException, IOException, MissingUserIdOnKeyException { + if (!new BareJidUserId.PubRingSelectionStrategy().accept(owner, ring)) { + throw new MissingUserIdOnKeyException(owner, ring.getPublicKey().getKeyID()); + } + try { + PGPPublicKeyRingCollection publicKeyRings = getStore().getPublicKeyRings(owner); + if (publicKeyRings == null) { + publicKeyRings = new PGPPublicKeyRingCollection(Collections.singleton(ring)); + } else { + publicKeyRings = PGPPublicKeyRingCollection.addPublicKeyRing(publicKeyRings, ring); + } + getStore().storePublicKeyRing(owner, publicKeyRings); + } catch (PGPException e) { + throw new SmackOpenPgpException(e); + } + return getFingerprint(ring.getPublicKey()); } @Override - public OpenPgpV4Fingerprint importSecretKey(BareJid owner, byte[] bytes) throws MissingUserIdOnKeyException { - return null; + public OpenPgpV4Fingerprint importSecretKey(BareJid owner, byte[] bytes) + throws MissingUserIdOnKeyException, SmackOpenPgpException, IOException { + PGPSecretKeyRing secretKeys; + try { + secretKeys = new PGPSecretKeyRing(bytes, new BcKeyFingerprintCalculator()); + } catch (PGPException | IOException e) { + throw new SmackOpenPgpException("Could not deserialize PGP secret key of " + owner.toString(), e); + } + + if (!new BareJidUserId.SecRingSelectionStrategy().accept(owner, secretKeys)) { + throw new MissingUserIdOnKeyException(owner, secretKeys.getPublicKey().getKeyID()); + } + + PGPSecretKeyRingCollection secretKeyRings; + try { + secretKeyRings = getStore().getSecretKeyRings(owner); + } catch (PGPException | IOException e) { + throw new SmackOpenPgpException("Could not load secret key ring of " + owner.toString(), e); + } + if (secretKeyRings == null) { + try { + secretKeyRings = new PGPSecretKeyRingCollection(Collections.singleton(secretKeys)); + } catch (IOException | PGPException e) { + throw new SmackOpenPgpException("Could not create SecretKeyRingCollection from SecretKeyRing.", e); + } + } else { + secretKeyRings = PGPSecretKeyRingCollection.addSecretKeyRing(secretKeyRings, secretKeys); + } + getStore().storeSecretKeyRing(owner, secretKeyRings); + + PGPPublicKeyRing publicKeys = BCUtil.publicKeyRingFromSecretKeyRing(secretKeys); + importPublicKey(owner, publicKeys); + + return getFingerprint(publicKeys.getPublicKey()); } public static OpenPgpV4Fingerprint getFingerprint(PGPPublicKey publicKey) { diff --git a/smack-openpgp/src/main/java/org/jivesoftware/smackx/ox/OpenPgpManager.java b/smack-openpgp/src/main/java/org/jivesoftware/smackx/ox/OpenPgpManager.java index 2672c2093..5ca8846b8 100644 --- a/smack-openpgp/src/main/java/org/jivesoftware/smackx/ox/OpenPgpManager.java +++ b/smack-openpgp/src/main/java/org/jivesoftware/smackx/ox/OpenPgpManager.java @@ -317,14 +317,18 @@ public final class OpenPgpManager extends Manager { if (pubkeyElement == null) { continue; } + processPublicKey(pubkeyElement, jid); available.add(f); + } catch (PubSubException.NotAPubSubNodeException | PubSubException.NotALeafNodeException e) { LOGGER.log(Level.WARNING, "Could not fetch public key " + f.toString() + " of user " + jid.toString(), e); unfetched.put(f, e); } catch (MissingUserIdOnKeyException e) { LOGGER.log(Level.WARNING, "Key does not contain user-id of " + jid + ". Ignoring the key.", e); unfetched.put(f, e); + } catch (IOException e) { + LOGGER.log(Level.WARNING, "Could not import key " + f.toString() + " of user " + jid.toString(), e); } } } @@ -382,7 +386,7 @@ public final class OpenPgpManager extends Manager { */ private void processPublicKey(PubkeyElement pubkeyElement, BareJid owner) - throws MissingUserIdOnKeyException { + throws MissingUserIdOnKeyException, IOException, SmackOpenPgpException { byte[] base64 = pubkeyElement.getDataElement().getB64Data(); provider.importPublicKey(owner, Base64.decode(base64)); } diff --git a/smack-openpgp/src/main/java/org/jivesoftware/smackx/ox/OpenPgpProvider.java b/smack-openpgp/src/main/java/org/jivesoftware/smackx/ox/OpenPgpProvider.java index a9f1a194e..9fb9e0b0f 100644 --- a/smack-openpgp/src/main/java/org/jivesoftware/smackx/ox/OpenPgpProvider.java +++ b/smack-openpgp/src/main/java/org/jivesoftware/smackx/ox/OpenPgpProvider.java @@ -133,9 +133,11 @@ public interface OpenPgpProvider { throws SmackOpenPgpException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, NoSuchProviderException, IOException; - OpenPgpV4Fingerprint importPublicKey(BareJid owner, byte[] bytes) throws MissingUserIdOnKeyException; + OpenPgpV4Fingerprint importPublicKey(BareJid owner, byte[] bytes) + throws MissingUserIdOnKeyException, IOException, SmackOpenPgpException; - OpenPgpV4Fingerprint importSecretKey(BareJid owner, byte[] bytes) throws MissingUserIdOnKeyException; + OpenPgpV4Fingerprint importSecretKey(BareJid owner, byte[] bytes) + throws MissingUserIdOnKeyException, SmackOpenPgpException, IOException; OpenPgpStore getStore(); } diff --git a/smack-openpgp/src/main/java/org/jivesoftware/smackx/ox/util/PubSubDelegate.java b/smack-openpgp/src/main/java/org/jivesoftware/smackx/ox/util/PubSubDelegate.java index c3de9f042..805d41ab4 100644 --- a/smack-openpgp/src/main/java/org/jivesoftware/smackx/ox/util/PubSubDelegate.java +++ b/smack-openpgp/src/main/java/org/jivesoftware/smackx/ox/util/PubSubDelegate.java @@ -24,6 +24,7 @@ import java.util.logging.Logger; import org.jivesoftware.smack.SmackException; import org.jivesoftware.smack.XMPPConnection; import org.jivesoftware.smack.XMPPException; +import org.jivesoftware.smack.packet.StanzaError; import org.jivesoftware.smackx.disco.ServiceDiscoveryManager; import org.jivesoftware.smackx.ox.OpenPgpV4Fingerprint; import org.jivesoftware.smackx.ox.element.PubkeyElement; @@ -205,7 +206,15 @@ public class PubSubDelegate { throws XMPPException.XMPPErrorException, SmackException.NotConnectedException, InterruptedException, SmackException.NoResponseException { PubSubManager pm = PubSubManager.getInstance(connection, connection.getUser().asBareJid()); - pm.deleteNode(PEP_NODE_PUBLIC_KEYS); + try { + pm.deleteNode(PEP_NODE_PUBLIC_KEYS); + } catch (XMPPException.XMPPErrorException e) { + if (e.getXMPPError().getCondition() == StanzaError.Condition.item_not_found) { + LOGGER.log(Level.FINE, "Node does not exist. No need to delete it."); + } else { + throw e; + } + } } /**