Merge pull request #2227 from open-keychain/usb-whitelist

Use different approach for whitelisting usb security tokens

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/Constants.java

@@ -143,6 +143,7 @@ public final class Constants {
         public static final String EXPERIMENTAL_ENABLE_WORD_CONFIRM = "experimentalEnableWordConfirm";
         public static final String EXPERIMENTAL_ENABLE_LINKED_IDENTITIES = "experimentalEnableLinkedIdentities";
         public static final String EXPERIMENTAL_ENABLE_KEYBASE = "experimentalEnableKeybase";
+        public static final String EXPERIMENTAL_USB_ALLOW_UNTESTED = "experimentalUsbAllowUntested";
         public static final String EXPERIMENTAL_SMARTPGP_VERIFY_AUTHORITY = "smartpgp_authorities_pref";
         public static final String EXPERIMENTAL_SMARTPGP_AUTHORITIES = "smartpgp_authorities";
 

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenConnection.java

@@ -189,10 +189,11 @@ public class SecurityTokenConnection {
     @VisibleForTesting
     void connectToDevice(Context context) throws IOException {
         // Connect on transport layer
-        mCardCapabilities = new CardCapabilities();
-
         mTransport.connect();
 
+        // dummy instance for initial communicate() calls
+        mCardCapabilities = new CardCapabilities();
+
         determineTokenType();
 
         CommandApdu select = commandFactory.createSelectFileOpenPgpCommand();
@@ -546,7 +547,7 @@ public class SecurityTokenConnection {
         }
 
         // Now we're ready to communicate with the token.
-        byte[] keyBytes = null;
+        byte[] keyBytes;
 
         try {
             secretKey.unlock(passphrase);
@@ -1015,10 +1016,9 @@ public class SecurityTokenConnection {
 
         TransportType transportType = mTransport.getTransportType();
 
-        SecurityTokenInfo info = SecurityTokenInfo
+        return SecurityTokenInfo
-                .create(transportType, tokenType, fingerprints, aid, userId, url, pwInfo[4], pwInfo[6], hasLifeCycleManagement);
+                .create(transportType, tokenType, fingerprints, aid, userId, url, pwInfo[4], pwInfo[6],
-
+                        hasLifeCycleManagement);
-        return info;
     }
 
     public static double parseOpenPgpVersion(final byte[] aid) {

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/SecurityTokenInfo.java

@@ -3,8 +3,10 @@ package org.sufficientlysecure.keychain.securitytoken;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -100,7 +102,7 @@ public abstract class SecurityTokenInfo implements Parcelable {
         NITROKEY_START_1_25_AND_NEWER, GNUK_OLD, GNUK_1_25_AND_NEWER, LEDGER_NANO_S, UNKNOWN
     }
 
-    private static final HashSet<TokenType> SUPPORTED_USB_TOKENS = new HashSet<>(Arrays.asList(
+    public static final Set<TokenType> SUPPORTED_USB_TOKENS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
             TokenType.YUBIKEY_NEO,
             TokenType.YUBIKEY_4,
             TokenType.NITROKEY_PRO,
@@ -109,23 +111,16 @@ public abstract class SecurityTokenInfo implements Parcelable {
             TokenType.NITROKEY_START_1_25_AND_NEWER,
             TokenType.GNUK_OLD,
             TokenType.GNUK_1_25_AND_NEWER
-    ));
+    )));
 
-    private static final HashSet<TokenType> SUPPORTED_USB_SETUP = new HashSet<>(Arrays.asList(
+    private static final Set<TokenType> SUPPORTED_USB_SETUP = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
             TokenType.YUBIKEY_NEO,
             TokenType.YUBIKEY_4,
             TokenType.NITROKEY_PRO,
             TokenType.NITROKEY_STORAGE,
             TokenType.NITROKEY_START_1_25_AND_NEWER,
             TokenType.GNUK_1_25_AND_NEWER
-    ));
+    )));
-
-    public boolean isSecurityTokenSupported() {
-        boolean isKnownSupported = SUPPORTED_USB_TOKENS.contains(getTokenType());
-        boolean isNfcTransport = getTransportType() == TransportType.NFC;
-
-        return isKnownSupported || isNfcTransport;
-    }
 
     public boolean isPutKeySupported() {
         boolean isKnownSupported = SUPPORTED_USB_SETUP.contains(getTokenType());

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/UsbConnectionDispatcher.java

@@ -26,6 +26,7 @@ import android.hardware.usb.UsbDevice;
 import android.hardware.usb.UsbManager;
 
 import org.sufficientlysecure.keychain.Constants;
+import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransport;
 import org.sufficientlysecure.keychain.ui.UsbEventReceiverActivity;
 import org.sufficientlysecure.keychain.util.Log;
 
@@ -50,7 +51,7 @@ public class UsbConnectionDispatcher {
                             false);
                     if (permission) {
                         Log.d(Constants.TAG, "Got permission for " + usbDevice.getDeviceName());
-                        mListener.usbDeviceDiscovered(usbDevice);
+                        sendUsbTransportDiscovered(usbDevice);
                     }
                     break;
                 }
@@ -82,17 +83,24 @@ public class UsbConnectionDispatcher {
         // Note: we don't check devices VID/PID because
         // we check for permission instead.
         // We should have permission only for matching devices
-        for (UsbDevice device : mUsbManager.getDeviceList().values()) {
+        for (UsbDevice usbDevice : mUsbManager.getDeviceList().values()) {
-            if (mUsbManager.hasPermission(device)) {
+            if (mUsbManager.hasPermission(usbDevice)) {
-                if (mListener != null) {
+                sendUsbTransportDiscovered(usbDevice);
-                    mListener.usbDeviceDiscovered(device);
-                }
                 break;
             }
         }
     }
 
+    private void sendUsbTransportDiscovered(UsbDevice usbDevice) {
+        if (mListener == null) {
+            return;
+        }
+
+        UsbTransport usbTransport = UsbTransport.createUsbTransport(mActivity.getBaseContext(), usbDevice);
+        mListener.usbTransportDiscovered(usbTransport);
+    }
+
     public interface OnDiscoveredUsbDeviceListener {
-        void usbDeviceDiscovered(UsbDevice usbDevice);
+        void usbTransportDiscovered(UsbTransport usbTransport);
     }
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/UnsupportedUsbTokenException.java

@@ -0,0 +1,8 @@
+package org.sufficientlysecure.keychain.securitytoken.usb;
+
+
+public class UnsupportedUsbTokenException extends UsbTransportException {
+    UnsupportedUsbTokenException() {
+        super("This USB token is not supported!");
+    }
+}

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/securitytoken/usb/UsbTransport.java

@@ -20,6 +20,7 @@ package org.sufficientlysecure.keychain.securitytoken.usb;
 
 import java.io.IOException;
 
+import android.content.Context;
 import android.hardware.usb.UsbConstants;
 import android.hardware.usb.UsbDevice;
 import android.hardware.usb.UsbDeviceConnection;
@@ -39,6 +40,8 @@ import org.sufficientlysecure.keychain.securitytoken.SecurityTokenInfo.TokenType
 import org.sufficientlysecure.keychain.securitytoken.SecurityTokenInfo.TransportType;
 import org.sufficientlysecure.keychain.securitytoken.Transport;
 import org.sufficientlysecure.keychain.util.Log;
+import org.sufficientlysecure.keychain.util.Preferences;
+
 
 /**
  * Based on USB CCID Specification rev. 1.1
@@ -72,10 +75,19 @@ public class UsbTransport implements Transport {
     private UsbDeviceConnection usbConnection;
     private UsbInterface usbInterface;
     private CcidTransportProtocol ccidTransportProtocol;
+    private boolean allowUntestedUsbTokens;
 
-    public UsbTransport(UsbDevice usbDevice, UsbManager usbManager) {
+    public static UsbTransport createUsbTransport(Context context, UsbDevice usbDevice) {
+        UsbManager usbManager = (UsbManager) context.getSystemService(Context.USB_SERVICE);
+        boolean allowUntestedUsbTokens = Preferences.getPreferences(context).getExperimentalUsbAllowUntested();
+
+        return new UsbTransport(usbDevice, usbManager, allowUntestedUsbTokens);
+    }
+
+    private UsbTransport(UsbDevice usbDevice, UsbManager usbManager, boolean allowUntestedUsbTokens) {
         this.usbDevice = usbDevice;
         this.usbManager = usbManager;
+        this.allowUntestedUsbTokens = allowUntestedUsbTokens;
     }
 
     @Override
@@ -134,6 +146,13 @@ public class UsbTransport implements Transport {
             throw new UsbTransportException("USB error: failed to connect to device");
         }
 
+        boolean tokenTypeSupported = SecurityTokenInfo.SUPPORTED_USB_TOKENS.contains(getTokenTypeIfAvailable());
+        if (!allowUntestedUsbTokens && !tokenTypeSupported) {
+            usbConnection.close();
+            usbConnection = null;
+            throw new UnsupportedUsbTokenException();
+        }
+
         if (!usbConnection.claimInterface(usbInterface, true)) {
             throw new UsbTransportException("USB error: failed to claim interface");
         }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseSecurityTokenActivity.java

@@ -25,11 +25,8 @@ package org.sufficientlysecure.keychain.ui.base;
 import java.io.IOException;
 
 import android.app.Activity;
-import android.content.Context;
 import android.content.Intent;
 import android.content.pm.PackageManager;
-import android.hardware.usb.UsbDevice;
-import android.hardware.usb.UsbManager;
 import android.nfc.NfcAdapter;
 import android.nfc.Tag;
 import android.nfc.TagLostException;
@@ -48,6 +45,7 @@ import org.sufficientlysecure.keychain.securitytoken.SecurityTokenInfo;
 import org.sufficientlysecure.keychain.securitytoken.SecurityTokenInfo.TokenType;
 import org.sufficientlysecure.keychain.securitytoken.Transport;
 import org.sufficientlysecure.keychain.securitytoken.UsbConnectionDispatcher;
+import org.sufficientlysecure.keychain.securitytoken.usb.UnsupportedUsbTokenException;
 import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransport;
 import org.sufficientlysecure.keychain.securitytoken.usb.UsbTransportException;
 import org.sufficientlysecure.keychain.service.PassphraseCacheService;
@@ -62,6 +60,7 @@ import org.sufficientlysecure.keychain.ui.util.Notify.Style;
 import org.sufficientlysecure.keychain.util.Log;
 import org.sufficientlysecure.keychain.util.Passphrase;
 
+
 public abstract class BaseSecurityTokenActivity extends BaseActivity
         implements OnDiscoveredTagListener, UsbConnectionDispatcher.OnDiscoveredUsbDeviceListener {
     public static final int REQUEST_CODE_PIN = 1;
@@ -114,6 +113,7 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity
         onSecurityTokenError(error);
     }
 
+    @Override
     public void tagDiscovered(Tag tag) {
         // Actual NFC operations are executed in doInBackground to not block the UI thread
         if (!mTagHandlingEnabled) {
@@ -124,15 +124,13 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity
         securityTokenDiscovered(nfcTransport);
     }
 
-    public void usbDeviceDiscovered(UsbDevice usbDevice) {
+    @Override
+    public void usbTransportDiscovered(UsbTransport usbTransport) {
         // Actual USB operations are executed in doInBackground to not block the UI thread
         if (!mTagHandlingEnabled) {
             return;
         }
 
-        UsbManager usbManager = (UsbManager) getSystemService(Context.USB_SERVICE);
-
-        UsbTransport usbTransport = new UsbTransport(usbDevice, usbManager);
         securityTokenDiscovered(usbTransport);
     }
 
@@ -235,6 +233,11 @@ public abstract class BaseSecurityTokenActivity extends BaseActivity
     private void handleSecurityTokenError(SecurityTokenConnection stConnection, IOException e) {
         Log.d(Constants.TAG, "Exception in handleSecurityTokenError", e);
 
+        if (e instanceof UnsupportedUsbTokenException) {
+            onSecurityTokenError(getString(R.string.security_token_not_supported));
+            return;
+        }
+
         if (e instanceof TagLostException) {
             onSecurityTokenError(getString(R.string.security_token_error_tag_lost));
             return;

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/util/Preferences.java

@@ -402,6 +402,10 @@ public class Preferences {
         return mSharedPreferences.getBoolean(Pref.EXPERIMENTAL_ENABLE_KEYBASE, false);
     }
 
+    public boolean getExperimentalUsbAllowUntested() {
+        return mSharedPreferences.getBoolean(Pref.EXPERIMENTAL_USB_ALLOW_UNTESTED, false);
+    }
+
     public boolean getExperimentalSmartPGPAuthoritiesEnable() {
         return mSharedPreferences.getBoolean(Pref.EXPERIMENTAL_SMARTPGP_VERIFY_AUTHORITY, false);
     }

OpenKeychain/src/main/res/values/strings.xml

@@ -1597,6 +1597,7 @@
     <string name="security_token_error_iso_dep_not_supported">"Security Token does not support the required communication standard (ISO-DEP, ISO 14443-4)"</string>
     <string name="security_token_error_generic">"Communication error. Most probably, the Security Token has been taken off too early."</string>
     <string name="security_token_error_try_again">"Try again"</string>
+    <string name="security_token_not_supported">"This Security Token is not yet supported by OpenKeychain"</string>
     <string name="btn_delete_original">Delete original file</string>
 
     <string name="snack_encrypt_filenames_on">"Filenames <b>are</b> encrypted."</string>
@@ -1982,5 +1983,7 @@
     </plurals>
 
     <string name="button_locate_nfc">"Where is my NFC reader?"</string>
+    <string name="label_usb_untested_summary">If enabled, USB Smartcard readers can be used that have not been properly tested.</string>
+    <string name="label_usb_untested">Allow untested USB Devices</string>
 
 </resources>

OpenKeychain/src/main/res/xml/experimental_preferences.xml

@@ -35,6 +35,14 @@
         android:persistent="true"
         android:title="@string/label_theme" />
 
+
+    <SwitchPreference
+        android:defaultValue="false"
+        android:key="experimentalUsbAllowUntested"
+        android:persistent="true"
+        android:summary="@string/label_usb_untested_summary"
+        android:title="@string/label_usb_untested" />
+
     <SwitchPreference
         android:defaultValue="false"
         android:key="smartpgp_authorities_pref"