modifySecretKey: err out on revocation of nonexistent user id

4 files changed, 21 insertions, 0 deletions

diff --git a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java
index 91c95a873..9d4aa7dba 100644
--- a/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java
+++ b/OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperationTest.java
@@ -589,6 +589,13 @@ public class PgpKeyOperationTest {
                     ring.getMasterKeyId(), ((SignaturePacket) p).getKeyID());
         }
 
+        { // revocation of non-existent user id should fail
+            parcel.reset();
+            parcel.mRevokeUserIds.add("nonexistent");
+
+            assertModifyFailure("revocation of nonexistent user id should fail", modified, parcel);
+        }
+
     }
 
     @Test
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
index fe84b3802..06d8dff69 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java
@@ -403,6 +403,18 @@ public class PgpKeyOperation {
                 progress(R.string.progress_modify_revokeuid, (i-1) * (100 / saveParcel.mRevokeUserIds.size()));
                 String userId = saveParcel.mRevokeUserIds.get(i);
                 log.add(LogLevel.INFO, LogType.MSG_MF_UID_REVOKE, indent, userId);
+                // Make sure the user id exists (yes these are 10 LoC in Java!)
+                boolean exists = false;
+                for (String uid : new IterableIterator<String>(modifiedPublicKey.getUserIDs())) {
+                    if (userId.equals(uid)) {
+                        exists = true;
+                        break;
+                    }
+                }
+                if (!exists) {
+                    log.add(LogLevel.ERROR, LogType.MSG_MF_ERROR_NOEXIST_REVOKE, indent);
+                    return new EditKeyResult(EditKeyResult.RESULT_ERROR, log, null);
+                }
 
                 // a duplicate revocation will be removed during canonicalization, so no need to
                 // take care of that here.
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
index 25dac2139..4c93e2a91 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
@@ -359,6 +359,7 @@ public class OperationResultParcel implements Parcelable {
         MSG_MF_ERROR_KEYID (R.string.msg_mf_error_keyid),
         MSG_MF_ERROR_INTEGRITY (R.string.msg_mf_error_integrity),
         MSG_MF_ERROR_NOEXIST_PRIMARY (R.string.msg_mf_error_noexist_primary),
+        MSG_MF_ERROR_NOEXIST_REVOKE (R.string.msg_mf_error_noexist_revoke),
         MSG_MF_ERROR_REVOKED_PRIMARY (R.string.msg_mf_error_revoked_primary),
         MSG_MF_ERROR_PGP (R.string.msg_mf_error_pgp),
         MSG_MF_ERROR_SIG (R.string.msg_mf_error_sig),
diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml
index 6f77be78f..5eb594163 100644
--- a/OpenKeychain/src/main/res/values/strings.xml
+++ b/OpenKeychain/src/main/res/values/strings.xml
@@ -639,6 +639,7 @@
     <string name="msg_mf_error_keyid">No key ID. This is an internal error, please file a bug report!</string>
     <string name="msg_mf_error_integrity">Internal error, integrity check failed!</string>
     <string name="msg_mf_error_noexist_primary">Bad primary user id specified!</string>
+    <string name="msg_mf_error_noexist_revoke">Bad user id for revocation specified!</string>
     <string name="msg_mf_error_revoked_primary">Revoked user ids cannot be primary!</string>
     <string name="msg_mf_error_pgp">PGP internal exception!</string>
     <string name="msg_mf_error_sig">Signature exception!</string>