From 4dfe32120efc2cbc849cf0df30bb0bdca878d9d8 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Mon, 22 Oct 2018 14:16:53 +0200 Subject: [PATCH 1/2] fix encryption key selection (fixes #2403) --- .../src/main/sqldelight/org/sufficientlysecure/keychain/Keys.sq | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OpenKeychain/src/main/sqldelight/org/sufficientlysecure/keychain/Keys.sq b/OpenKeychain/src/main/sqldelight/org/sufficientlysecure/keychain/Keys.sq index cf69f8057..73209f029 100644 --- a/OpenKeychain/src/main/sqldelight/org/sufficientlysecure/keychain/Keys.sq +++ b/OpenKeychain/src/main/sqldelight/org/sufficientlysecure/keychain/Keys.sq @@ -116,7 +116,7 @@ SELECT fingerprint selectEffectiveEncryptionKeyIdsByMasterKeyId: SELECT key_id FROM validKeys - WHERE has_secret > 1 AND can_encrypt = 1 AND master_key_id = ?; + WHERE can_encrypt = 1 AND master_key_id = ?; selectEffectiveSignKeyIdByMasterKeyId: SELECT key_id From abf331ddcee0f6de18c3e9852b6e2441e0a9220a Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Mon, 22 Oct 2018 14:26:33 +0200 Subject: [PATCH 2/2] add unit test for encryption key selection --- .../AuthenticationOperationTest.java | 21 +----- .../keychain/provider/KeyRepositoryTest.java | 71 +++++++++++++++++++ 2 files changed, 73 insertions(+), 19 deletions(-) create mode 100644 OpenKeychain/src/test/java/org/sufficientlysecure/keychain/provider/KeyRepositoryTest.java diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/AuthenticationOperationTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/AuthenticationOperationTest.java index e22585d7c..bfce5bf0e 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/AuthenticationOperationTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/AuthenticationOperationTest.java @@ -39,17 +39,16 @@ 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.UncachedKeyRing; import org.sufficientlysecure.keychain.daos.KeyRepository; import org.sufficientlysecure.keychain.daos.KeyWritableRepository; +import org.sufficientlysecure.keychain.pgp.CanonicalizedPublicKey; +import org.sufficientlysecure.keychain.pgp.UncachedKeyRing; import org.sufficientlysecure.keychain.service.input.CryptoInputParcel; import org.sufficientlysecure.keychain.ssh.AuthenticationData; import org.sufficientlysecure.keychain.ssh.AuthenticationOperation; import org.sufficientlysecure.keychain.ssh.AuthenticationParcel; import org.sufficientlysecure.keychain.ssh.AuthenticationResult; import org.sufficientlysecure.keychain.support.KeyringTestingHelper; -import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils; import org.sufficientlysecure.keychain.util.Passphrase; @RunWith(KeychainTestRunner.class) @@ -59,7 +58,6 @@ public class AuthenticationOperationTest { private static UncachedKeyRing mStaticRingEcDsa; private static UncachedKeyRing mStaticRingEdDsa; private static UncachedKeyRing mStaticRingDsa; - private static UncachedKeyRing mStaticRingRevoked; private static Passphrase mKeyPhrase; private static PrintStream oldShadowStream; @@ -134,8 +132,6 @@ public class AuthenticationOperationTest { mStaticRingEcDsa = KeyringTestingHelper.readRingFromResource("/test-keys/authenticate_ecdsa.sec"); mStaticRingEdDsa = KeyringTestingHelper.readRingFromResource("/test-keys/authenticate_eddsa.sec"); mStaticRingDsa = KeyringTestingHelper.readRingFromResource("/test-keys/authenticate_dsa.sec"); - - mStaticRingRevoked = KeyringTestingHelper.readRingFromResource("/test-keys/authenticate_multisub_with_revoked.asc"); } @Before @@ -150,7 +146,6 @@ public class AuthenticationOperationTest { databaseInteractor.saveSecretKeyRing(mStaticRingEcDsa); databaseInteractor.saveSecretKeyRing(mStaticRingEdDsa); databaseInteractor.saveSecretKeyRing(mStaticRingDsa); - databaseInteractor.saveSecretKeyRing(mStaticRingRevoked); // ok NOW log verbosely! ShadowLog.stream = System.out; @@ -399,16 +394,4 @@ public class AuthenticationOperationTest { Assert.assertFalse("authentication must fail with selected key disallowed", result.success()); } } - - @Test - public void testKeySelection() throws Exception { - long expectedAuthSubKeyId = KeyFormattingUtils.convertKeyIdHexToKeyId("0xcf64ee600f6fec9c"); - - KeyRepository keyRepository = KeyRepository.create(RuntimeEnvironment.application); - - long masterKeyId = mStaticRingRevoked.getMasterKeyId(); - long authSubKeyId = keyRepository.getEffectiveAuthenticationKeyId(masterKeyId); - - Assert.assertEquals(expectedAuthSubKeyId, authSubKeyId); - } } diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/provider/KeyRepositoryTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/provider/KeyRepositoryTest.java new file mode 100644 index 000000000..f7e3e7f62 --- /dev/null +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/provider/KeyRepositoryTest.java @@ -0,0 +1,71 @@ +/* + * Copyright (C) 2014 Dominik Schürmann + * Copyright (C) 2014 Vincent Breitmoser + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package org.sufficientlysecure.keychain.provider; + + +import java.util.List; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RuntimeEnvironment; +import org.sufficientlysecure.keychain.KeychainTestRunner; +import org.sufficientlysecure.keychain.daos.KeyRepository; +import org.sufficientlysecure.keychain.daos.KeyWritableRepository; +import org.sufficientlysecure.keychain.pgp.UncachedKeyRing; +import org.sufficientlysecure.keychain.support.KeyringTestingHelper; +import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils; + + +@RunWith(KeychainTestRunner.class) +public class KeyRepositoryTest { + + + private UncachedKeyRing testKeyring; + + @Before + public void setUp() throws Exception { + testKeyring = KeyringTestingHelper.readRingFromResource("/test-keys/authenticate_multisub_with_revoked.asc"); + + KeyWritableRepository databaseInteractor = + KeyWritableRepository.create(RuntimeEnvironment.application); + databaseInteractor.saveSecretKeyRing(testKeyring); + } + + @Test + public void testKeySelection() throws Exception { + long expectedAuthSubKeyId = KeyFormattingUtils.convertKeyIdHexToKeyId("0xcf64ee600f6fec9c"); + long expectedEncryptSubKeyId = KeyFormattingUtils.convertKeyIdHexToKeyId("0xDA7207E385A44339"); + long expectedSignSubKeyId = KeyFormattingUtils.convertKeyIdHexToKeyId("0xB8ECA89E054028D5"); + + KeyRepository keyRepository = KeyRepository.create(RuntimeEnvironment.application); + + long masterKeyId = testKeyring.getMasterKeyId(); + long authSubKeyId = keyRepository.getEffectiveAuthenticationKeyId(masterKeyId); + long signSubKeyId = keyRepository.getSecretSignId(masterKeyId); + List publicEncryptionIds = keyRepository.getPublicEncryptionIds(masterKeyId); + + Assert.assertEquals(expectedAuthSubKeyId, authSubKeyId); + Assert.assertEquals(expectedSignSubKeyId, signSubKeyId); + Assert.assertEquals(1, publicEncryptionIds.size()); + Assert.assertEquals(expectedEncryptSubKeyId, (long) publicEncryptionIds.get(0)); + } + +} \ No newline at end of file