Fix issues pointed out by code review

Author: vanitasvitae

smack-core/src/main/java/org/jivesoftware/smack/util/RandomUtils.java

@@ -0,0 +1,36 @@
+/**
+ *
+ * Copyright © 2019 Paul Schaub
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jivesoftware.smack.util;
+
+import java.security.SecureRandom;
+
+public class RandomUtils {
+
+    private static final SecureRandom SECURE_RANDOM = new SecureRandom();
+
+    /**
+     * Generate a securely random byte array.
+     *
+     * @param len length of the byte array
+     * @return byte array
+     */
+    public static byte[] secureRandomBytes(int len) {
+        byte[] bytes = new byte[len];
+        SECURE_RANDOM.nextBytes(bytes);
+        return bytes;
+    }
+}

smack-core/src/main/java/org/jivesoftware/smack/util/StringUtils.java

@@ -246,7 +246,8 @@ public static String encodeHex(byte[] bytes) {
 
     /**
      * Convert a hexadecimal String to bytes.
-     * Stolen from https://stackoverflow.com/a/140861/11150851
+     *
+     * Source: https://stackoverflow.com/a/140861/11150851
      *
      * @param s hex string
      * @return byte array

smack-experimental/src/main/java/org/jivesoftware/smackx/httpfileupload/HttpFileUploadManager.java

@@ -24,12 +24,10 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.HttpURLConnection;
-import java.net.MalformedURLException;
 import java.net.URL;
 import java.security.InvalidAlgorithmParameterException;
 import java.security.InvalidKeyException;
 import java.security.NoSuchAlgorithmException;
-import java.security.SecureRandom;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -38,11 +36,7 @@
 import java.util.logging.Logger;
 import javax.crypto.Cipher;
 import javax.crypto.CipherInputStream;
-import javax.crypto.KeyGenerator;
 import javax.crypto.NoSuchPaddingException;
-import javax.crypto.SecretKey;
-import javax.crypto.spec.IvParameterSpec;
-import javax.crypto.spec.SecretKeySpec;
 import javax.net.ssl.HttpsURLConnection;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLSocketFactory;
@@ -55,14 +49,14 @@
 import org.jivesoftware.smack.XMPPConnection;
 import org.jivesoftware.smack.XMPPConnectionRegistry;
 import org.jivesoftware.smack.XMPPException;
-import org.jivesoftware.smack.util.Objects;
-import org.jivesoftware.smack.util.StringUtils;
 import org.jivesoftware.smackx.disco.ServiceDiscoveryManager;
 import org.jivesoftware.smackx.disco.packet.DiscoverInfo;
 import org.jivesoftware.smackx.httpfileupload.UploadService.Version;
 import org.jivesoftware.smackx.httpfileupload.element.Slot;
 import org.jivesoftware.smackx.httpfileupload.element.SlotRequest;
 import org.jivesoftware.smackx.httpfileupload.element.SlotRequest_V0_2;
+import org.jivesoftware.smackx.omemo_media_sharing.AesgcmUrl;
+import org.jivesoftware.smackx.omemo_media_sharing.OmemoMediaSharingUtils;
 import org.jivesoftware.smackx.xdata.FormField;
 import org.jivesoftware.smackx.xdata.packet.DataForm;
 
@@ -95,7 +89,6 @@ public final class HttpFileUploadManager extends Manager {
     public static final String NAMESPACE_0_2 = "urn:xmpp:http:upload";
 
     private static final Logger LOGGER = Logger.getLogger(HttpFileUploadManager.class.getName());
-    private static final SecureRandom SECURE_RANDOM = new SecureRandom();
 
     static {
         XMPPConnectionRegistry.addConnectionCreationListener(new ConnectionCreationListener() {
@@ -579,234 +572,4 @@ public static UploadService.Version namespaceToVersion(String namespace) {
     private static boolean containsHttpFileUploadNamespace(DiscoverInfo discoverInfo) {
         return discoverInfo.containsFeature(NAMESPACE) || discoverInfo.containsFeature(NAMESPACE_0_2);
     }
-
-    /**
-     * Generate a securely random byte array.
-     *
-     * @param len length of the byte array
-     * @return byte array
-     */
-    private static byte[] secureRandomBytes(int len) {
-        byte[] bytes = new byte[len];
-        SECURE_RANDOM.nextBytes(bytes);
-        return bytes;
-    }
-
-    /**
-     * Utility code for XEP-XXXX: OMEMO Media Sharing.
-     *
-     * @see <a href="https://xmpp.org/extensions/inbox/omemo-media-sharing.html">XEP-XXXX: OMEMO Media Sharing</a>
-     */
-    static class OmemoMediaSharingUtils {
-
-        private static final String KEYTYPE = "AES";
-        private static final String CIPHERMODE = "AES/GCM/NoPadding";
-        // 256 bit = 32 byte
-        private static final int LEN_KEY = 32;
-        private static final int LEN_KEY_BITS = LEN_KEY * 8;
-
-        private static final int LEN_IV_12 = 12;
-        private static final int LEN_IV_16 = 16;
-        // Note: Contrary to what the ProtoXEP states, 16 byte IV length is used in the wild instead of 12.
-        // At some point we should switch to 12 bytes though.
-        private static final int LEN_IV = LEN_IV_16;
-
-        static byte[] generateRandomIV() {
-            return generateRandomIV(LEN_IV);
-        }
-
-        static byte[] generateRandomIV(int len) {
-            return secureRandomBytes(len);
-        }
-
-        /**
-         * Generate a random 256 bit AES key.
-         *
-         * @return encoded AES key
-         * @throws NoSuchAlgorithmException if the JVM doesn't provide the given key type.
-         */
-        static byte[] generateRandomKey() throws NoSuchAlgorithmException {
-            KeyGenerator generator = KeyGenerator.getInstance(KEYTYPE);
-            generator.init(LEN_KEY_BITS);
-            return generator.generateKey().getEncoded();
-        }
-
-        /**
-         * Create a {@link Cipher} from a given key and iv which is in encryption mode.
-         *
-         * @param key aes encryption key
-         * @param iv initialization vector
-         *
-         * @return cipher in encryption mode
-         *
-         * @throws NoSuchPaddingException if the JVM doesn't provide the padding specified in the ciphermode.
-         * @throws NoSuchAlgorithmException if the JVM doesn't provide the encryption method specified in the ciphermode.
-         * @throws InvalidAlgorithmParameterException if the cipher cannot be initiated.
-         * @throws InvalidKeyException if the key is invalid.
-         */
-        private static Cipher encryptionCipherFrom(byte[] key, byte[] iv)
-                throws NoSuchPaddingException, NoSuchAlgorithmException, InvalidAlgorithmParameterException,
-                InvalidKeyException {
-            SecretKey secretKey = new SecretKeySpec(key, KEYTYPE);
-            IvParameterSpec ivSpec = new IvParameterSpec(iv);
-            Cipher cipher = Cipher.getInstance(CIPHERMODE);
-            cipher.init(Cipher.ENCRYPT_MODE, secretKey, ivSpec);
-            return cipher;
-        }
-
-        /**
-         * Create a {@link Cipher} from a given key and iv which is in decryption mode.
-         *
-         * @param key aes encryption key
-         * @param iv initialization vector
-         *
-         * @return cipher in decryption mode
-         *
-         * @throws NoSuchPaddingException if the JVM doesn't provide the padding specified in the ciphermode.
-         * @throws NoSuchAlgorithmException if the JVM doesn't provide the encryption method specified in the ciphermode.
-         * @throws InvalidAlgorithmParameterException if the cipher cannot be initiated.
-         * @throws InvalidKeyException if the key is invalid.
-         */
-        private static Cipher decryptionCipherFrom(byte[] key, byte[] iv)
-                throws NoSuchPaddingException, NoSuchAlgorithmException, InvalidAlgorithmParameterException,
-                InvalidKeyException {
-            SecretKey secretKey = new SecretKeySpec(key, KEYTYPE);
-            IvParameterSpec ivSpec = new IvParameterSpec(iv);
-            Cipher cipher = Cipher.getInstance(CIPHERMODE);
-            cipher.init(Cipher.ENCRYPT_MODE, secretKey, ivSpec);
-            return cipher;
-        }
-    }
-
-    /**
-     * This class represents a aesgcm URL as described in XEP-XXXX: OMEMO Media Sharing.
-     * As the builtin {@link URL} class cannot handle the aesgcm protocol identifier, this class
-     * is used as a utility class that bundles together a {@link URL}, key and IV.
-     *
-     * @see <a href="https://xmpp.org/extensions/inbox/omemo-media-sharing.html">XEP-XXXX: OMEMO Media Sharing</a>
-     */
-    public static class AesgcmUrl {
-
-        public static final String PROTOCOL = "aesgcm";
-
-        private final URL httpsUrl;
-        private final byte[] keyBytes;
-        private final byte[] ivBytes;
-
-        /**
-         * Private constructor that constructs the {@link AesgcmUrl} from a normal https {@link URL}, a key and iv.
-         *
-         * @param httpsUrl normal https url as given by the {@link Slot}.
-         * @param key byte array of an encoded 256 bit aes key
-         * @param iv 16 or 12 byte initialization vector
-         */
-        AesgcmUrl(URL httpsUrl, byte[] key, byte[] iv) {
-            this.httpsUrl = Objects.requireNonNull(httpsUrl);
-            this.keyBytes = Objects.requireNonNull(key);
-            this.ivBytes = Objects.requireNonNull(iv);
-        }
-
-        /**
-         * Parse a {@link AesgcmUrl} from a {@link String}.
-         * The parsed object will provide a normal {@link URL} under which the offered file can be downloaded,
-         * as well as a {@link Cipher} that can be used to decrypt it.
-         *
-         * @param aesgcmUrlString aesgcm URL as a {@link String}
-         */
-        public AesgcmUrl(String aesgcmUrlString) {
-            if (!aesgcmUrlString.startsWith(PROTOCOL)) {
-                throw new IllegalArgumentException("Provided String does not resemble a aesgcm URL.");
-            }
-
-            // Convert aesgcm Url to https URL
-            this.httpsUrl = extractHttpsUrl(aesgcmUrlString);
-
-            // Extract IV and Key
-            byte[][] ivAndKey = extractIVAndKey(aesgcmUrlString);
-            this.ivBytes = ivAndKey[0];
-            this.keyBytes = ivAndKey[1];
-        }
-
-        /**
-         * Return a https {@link URL} under which the file can be downloaded.
-         *
-         * @return https URL
-         */
-        public URL getDownloadUrl() {
-            return httpsUrl;
-        }
-
-        /**
-         * Returns the {@link String} representation of this aesgcm URL.
-         *
-         * @return aesgcm URL with key and IV.
-         */
-        public String getAesgcmUrl() {
-            String aesgcmUrl = httpsUrl.toString().replaceFirst(httpsUrl.getProtocol(), PROTOCOL);
-            return aesgcmUrl + "#" + StringUtils.encodeHex(ivBytes) + StringUtils.encodeHex(keyBytes);
-        }
-
-        /**
-         * Returns a {@link Cipher} in decryption mode, which can be used to decrypt the offered file.
-         *
-         * @return cipher
-         *
-         * @throws NoSuchPaddingException if the JVM cannot provide the specified cipher mode
-         * @throws NoSuchAlgorithmException if the JVM cannot provide the specified cipher mode
-         * @throws InvalidAlgorithmParameterException if the JVM cannot provide the specified cipher
-         *                                            (eg. if no BC provider is added)
-         * @throws InvalidKeyException if the provided key is invalid
-         */
-        public Cipher getDecryptionCipher() throws NoSuchPaddingException, NoSuchAlgorithmException,
-                InvalidAlgorithmParameterException, InvalidKeyException {
-            return OmemoMediaSharingUtils.decryptionCipherFrom(keyBytes, ivBytes);
-        }
-
-        private static URL extractHttpsUrl(String aesgcmUrlString) {
-            // aesgcm -> https
-            String httpsUrlString = aesgcmUrlString.replaceFirst(PROTOCOL, "https");
-            // remove #ref
-            httpsUrlString = httpsUrlString.substring(0, httpsUrlString.indexOf("#"));
-
-            try {
-                return new URL(httpsUrlString);
-            } catch (MalformedURLException e) {
-                throw new AssertionError("Failed to convert aesgcm URL to https URL: '" + aesgcmUrlString + "'", e);
-            }
-        }
-
-        private static byte[][] extractIVAndKey(String aesgcmUrlString) {
-            int startOfRef = aesgcmUrlString.lastIndexOf("#");
-            if (startOfRef == -1) {
-                throw new IllegalArgumentException("The provided aesgcm Url does not have a ref part which is " +
-                        "supposed to contain the encryption key for file encryption.");
-            }
-
-            String ref = aesgcmUrlString.substring(startOfRef + 1);
-            byte[] refBytes = StringUtils.hexStringToByteArray(ref);
-
-            byte[] key = new byte[32];
-            byte[] iv;
-            int ivLen;
-            // determine the length of the initialization vector part
-            switch (refBytes.length) {
-                // 32 bytes key + 16 bytes IV
-                case 48:
-                    ivLen = 16;
-                    break;
-
-                // 32 bytes key + 12 bytes IV
-                case 44:
-                    ivLen = 12;
-                    break;
-                default:
-                    throw new IllegalArgumentException("Provided URL has an invalid ref tag (" + ref.length() + "): '" + ref + "'");
-            }
-            iv = new byte[ivLen];
-            System.arraycopy(refBytes, 0, iv, 0, ivLen);
-            System.arraycopy(refBytes, ivLen, key, 0, 32);
-
-            return new byte[][] {iv,key};
-        }
-    }
 }