Improve error handling for curve OID to SSH curve identifier translation

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/SshPublicKey.java

@@ -29,6 +29,8 @@ import org.sufficientlysecure.keychain.ssh.key.SshEd25519PublicKey;
 import org.sufficientlysecure.keychain.ssh.key.SshRSAPublicKey;
 import org.sufficientlysecure.keychain.ssh.utils.SshUtils;
 
+import java.security.NoSuchAlgorithmException;
+
 public class SshPublicKey {
     private final static String TAG = "SshPublicKey";
 
@@ -38,7 +40,7 @@ public class SshPublicKey {
         mPublicKey = publicKey;
     }
 
-    public String getEncodedKey() throws PgpGeneralException {
+    public String getEncodedKey() throws PgpGeneralException, NoSuchAlgorithmException {
         PGPPublicKey key = mPublicKey.getPublicKey();
 
         switch (key.getAlgorithm()) {
@@ -51,9 +53,8 @@ public class SshPublicKey {
             case PGPPublicKey.DSA:
                 return encodeDSAKey(key);
             default:
-                break;
+                throw new PgpGeneralException("Unknown key algorithm");
         }
-        throw new PgpGeneralException("Unknown algorithm");
     }
 
     private String encodeRSAKey(PGPPublicKey publicKey) {
@@ -64,7 +65,7 @@ public class SshPublicKey {
         return pubkey.getPublicKeyBlob();
     }
 
-    private String encodeECKey(PGPPublicKey publicKey) {
+    private String encodeECKey(PGPPublicKey publicKey) throws NoSuchAlgorithmException {
         ECPublicBCPGKey publicBCPGKey = (ECPublicBCPGKey) publicKey.getPublicKeyPacket().getKey();
 
         String curveName = SshUtils.getCurveName(mPublicKey.getCurveOid());

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/SshAuthenticationService.java

@@ -192,15 +192,15 @@ public class SshAuthenticationService extends Service {
         } else if (authResult.success()) {
             byte[] rawSignature = authResult.getSignature();
             byte[] sshSignature;
-            if (authSubKeyAlgorithm == PublicKeyAlgorithmTags.ECDSA) {
+            try {
-                sshSignature = SshSignatureConverter.getSshSignatureEcDsa(rawSignature, authSubKeyCurveOid);
+                if (authSubKeyAlgorithm == PublicKeyAlgorithmTags.ECDSA) {
-            } else {
+                    sshSignature = SshSignatureConverter.getSshSignatureEcDsa(rawSignature, authSubKeyCurveOid);
-                try {
+                } else {
                     sshSignature = SshSignatureConverter.getSshSignature(rawSignature, authSubKeyAlgorithm);
-                } catch (NoSuchAlgorithmException e) {
-                    return createExceptionErrorResult(SshAuthenticationApiError.INTERNAL_ERROR,
-                            "Error converting signature", e);
                 }
+            } catch (NoSuchAlgorithmException e) {
+                return createExceptionErrorResult(SshAuthenticationApiError.INTERNAL_ERROR,
+                        "Error converting signature", e);
             }
             return new SigningResponse(sshSignature).toIntent();
         } else {
@@ -351,7 +351,7 @@ public class SshAuthenticationService extends Service {
         SshPublicKey sshPublicKey = new SshPublicKey(publicKey);
         try {
             sshPublicKeyBlob = sshPublicKey.getEncodedKey();
-        } catch (PgpGeneralException e) {
+        } catch (PgpGeneralException | NoSuchAlgorithmException e) {
             return createExceptionErrorResult(SshAuthenticationApiError.GENERIC_ERROR,
                     "Error converting public key to SSH format", e);
         }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ssh/signature/SshSignatureConverter.java

@@ -128,7 +128,7 @@ public class SshSignatureConverter {
         return signature.getBytes();
     }
 
-    public static byte[] getSshSignatureEcDsa(byte[] rawSignature, String curveOid) {
+    public static byte[] getSshSignatureEcDsa(byte[] rawSignature, String curveOid) throws NoSuchAlgorithmException {
         SshEncodedData signature = new SshEncodedData();
         signature.putString("ecdsa-sha2-" + SshUtils.getCurveName(curveOid));
         signature.putString(getEcDsaSignatureBlob(rawSignature));

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ssh/utils/SshUtils.java

@@ -17,9 +17,11 @@
 
 package org.sufficientlysecure.keychain.ssh.utils;
 
+import java.security.NoSuchAlgorithmException;
+
 public class SshUtils {
 
-    public static String getCurveName(String curveOid) {
+    public static String getCurveName(String curveOid) throws NoSuchAlgorithmException {
         // see RFC5656 section 10.{1,2}
         switch (curveOid) {
             // REQUIRED curves
@@ -32,26 +34,26 @@ public class SshUtils {
 
             // RECOMMENDED curves
             case "1.3.132.0.1":
-                return     "1.3.132.0.1";
+                return "1.3.132.0.1";
             case "1.2.840.10045.3.1.1":
-                return  "1.2.840.10045.3.1.1";
+                return "1.2.840.10045.3.1.1";
             case "1.3.132.0.33":
-                return     "1.3.132.0.33";
+                return "1.3.132.0.33";
             case "1.3.132.0.26":
-                return     "1.3.132.0.26";
+                return "1.3.132.0.26";
             case "1.3.132.0.27":
-                return     "1.3.132.0.27";
+                return "1.3.132.0.27";
             case "1.3.132.0.16":
-                return     "1.3.132.0.16";
+                return "1.3.132.0.16";
             case "1.3.132.0.36":
-                return     "1.3.132.0.36";
+                return "1.3.132.0.36";
             case "1.3.132.0.37":
-                return     "1.3.132.0.37";
+                return "1.3.132.0.37";
             case "1.3.132.0.38":
-                return     "1.3.132.0.38";
+                return "1.3.132.0.38";
 
             default:
-                return null;
+                throw new NoSuchAlgorithmException("Can't translate curve OID to SSH curve identifier");
         }
     }
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyAdvShareFragment.java

@@ -69,6 +69,7 @@ import java.io.BufferedWriter;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.OutputStreamWriter;
+import java.security.NoSuchAlgorithmException;
 
 public class ViewKeyAdvShareFragment extends LoaderFragment implements
         LoaderManager.LoaderCallbacks<Cursor> {
@@ -223,7 +224,8 @@ public class ViewKeyAdvShareFragment extends LoaderFragment implements
     }
 
     private String getShareKeyContent(boolean asSshKey)
-            throws PgpKeyNotFoundException, KeyRepository.NotFoundException, IOException, PgpGeneralException {
+            throws PgpKeyNotFoundException, KeyRepository.NotFoundException, IOException, PgpGeneralException,
+            NoSuchAlgorithmException {
 
         KeyRepository keyRepository = KeyRepository.create(getContext());
 
@@ -303,7 +305,7 @@ public class ViewKeyAdvShareFragment extends LoaderFragment implements
             Intent shareChooser = Intent.createChooser(sendIntent, title);
 
             startActivity(shareChooser);
-        } catch (PgpGeneralException | IOException e) {
+        } catch (PgpGeneralException | IOException | NoSuchAlgorithmException e) {
             Log.e(Constants.TAG, "error processing key!", e);
             Notify.create(activity, R.string.error_key_processing, Notify.Style.ERROR).show();
         } catch (PgpKeyNotFoundException | KeyRepository.NotFoundException e) {

OpenKeychain/src/test/java/org/sufficientlysecure/keychain/ssh/signature/SshSignatureConverterTest.java

@@ -0,0 +1,74 @@
+package org.sufficientlysecure.keychain.ssh.signature;
+
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RuntimeEnvironment;
+import org.robolectric.shadows.ShadowLog;
+import org.sufficientlysecure.keychain.KeychainTestRunner;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedPublicKey;
+import org.sufficientlysecure.keychain.pgp.SshPublicKey;
+import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
+import org.sufficientlysecure.keychain.provider.KeyRepository;
+import org.sufficientlysecure.keychain.provider.KeyWritableRepository;
+import org.sufficientlysecure.keychain.support.KeyringTestingHelper;
+import org.sufficientlysecure.keychain.util.Passphrase;
+
+import java.io.PrintStream;
+import java.security.Security;
+
+
+@RunWith(KeychainTestRunner.class)
+public class SshSignatureConverterTest {
+
+    private static UncachedKeyRing mStaticRingEcDsa;
+    private static Passphrase mKeyPhrase;
+
+    private static PrintStream oldShadowStream;
+
+    @BeforeClass
+    public static void setUpOnce() throws Exception {
+        Security.insertProviderAt(new BouncyCastleProvider(), 1);
+        oldShadowStream = ShadowLog.stream;
+        // ShadowLog.stream = System.out;
+
+        mKeyPhrase = new Passphrase("x");
+        mStaticRingEcDsa = KeyringTestingHelper.readRingFromResource("/test-keys/authenticate_ecdsa.sec");
+    }
+
+    @Before
+    public void setUp() {
+        KeyWritableRepository databaseInteractor =
+                KeyWritableRepository.create(RuntimeEnvironment.application);
+
+        // don't log verbosely here, we're not here to test imports
+        ShadowLog.stream = oldShadowStream;
+
+        databaseInteractor.saveSecretKeyRing(mStaticRingEcDsa);
+
+        // ok NOW log verbosely!
+        ShadowLog.stream = System.out;
+    }
+
+    @Test
+    public void testECDSA() throws Exception {
+        KeyRepository keyRepository = KeyRepository.create(RuntimeEnvironment.application);
+
+        long masterKeyId = mStaticRingEcDsa.getMasterKeyId();
+        long authSubKeyId = keyRepository.getCachedPublicKeyRing(masterKeyId).getSecretAuthenticationId();
+        CanonicalizedPublicKey canonicalizedPublicKey = keyRepository.getCanonicalizedPublicKeyRing(masterKeyId)
+                .getPublicKey(authSubKeyId);
+
+        SshPublicKey publicKeyUtils = new SshPublicKey(canonicalizedPublicKey);
+        String publicKeyBlob = publicKeyUtils.getEncodedKey();
+
+        String publicKeyBlobExpected = "ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTY"
+                + "AAABBBJm2rlv9/8dgVm6VbN9OJDK1pA1Cb7HjJZv+zyiZGbpUrNWN81L1z45mnOfYafAzZMZ9SBy4J954wjp4d/pICIg=";
+
+        Assert.assertEquals("Public key blobs must be equal", publicKeyBlobExpected, publicKeyBlob);
+
+    }
+}