diff options
3 files changed, 49 insertions, 5 deletions
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java index 8019b2b52..e309ed632 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java @@ -3,6 +3,8 @@ package org.sufficientlysecure.keychain.pgp; import org.spongycastle.bcpg.ArmoredOutputStream; import org.spongycastle.bcpg.S2K; import org.spongycastle.bcpg.SignatureSubpacketTags; +import org.spongycastle.bcpg.sig.KeyFlags; +import org.spongycastle.openpgp.PGPKeyFlags; import org.spongycastle.openpgp.PGPKeyRing; import org.spongycastle.openpgp.PGPObjectFactory; import org.spongycastle.openpgp.PGPPublicKey; @@ -10,6 +12,7 @@ import org.spongycastle.openpgp.PGPPublicKeyRing; import org.spongycastle.openpgp.PGPSecretKey; import org.spongycastle.openpgp.PGPSecretKeyRing; import org.spongycastle.openpgp.PGPSignature; +import org.spongycastle.openpgp.PGPSignatureList; import org.spongycastle.openpgp.PGPUtil; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; @@ -298,7 +301,6 @@ public class UncachedKeyRing { WrappedSignature cert = new WrappedSignature(zert); long certId = cert.getKeyId(); - int type = zert.getSignatureType(); if (type != PGPSignature.DEFAULT_CERTIFICATION && type != PGPSignature.NO_CERTIFICATION @@ -435,9 +437,12 @@ public class UncachedKeyRing { // certificate. PGPPublicKey modified = key; PGPSignature selfCert = null, revocation = null; - for (PGPSignature zig : new IterableIterator<PGPSignature>(key.getSignatures())) { + uids: for (PGPSignature zig : new IterableIterator<PGPSignature>(key.getSignatures())) { // remove from keyring (for now) modified = PGPPublicKey.removeCertification(modified, zig); + // add this too, easier than adding it for every single "continue" case + removedCerts += 1; + WrappedSignature cert = new WrappedSignature(zig); int type = cert.getSignatureType(); @@ -467,7 +472,6 @@ public class UncachedKeyRing { } if (type == PGPSignature.SUBKEY_BINDING) { - // TODO verify primary key binding signature for signing keys! // make sure the certificate checks out try { @@ -481,10 +485,42 @@ public class UncachedKeyRing { continue; } + if (zig.getHashedSubPackets().hasSubpacket(SignatureSubpacketTags.KEY_FLAGS)) { + int flags = ((KeyFlags) zig.getHashedSubPackets() + .getSubpacket(SignatureSubpacketTags.KEY_FLAGS)).getFlags(); + // If this subkey is allowed to sign data, + if ((flags & PGPKeyFlags.CAN_SIGN) == PGPKeyFlags.CAN_SIGN) { + try { + PGPSignatureList list = zig.getUnhashedSubPackets().getEmbeddedSignatures(); + boolean ok = false; + for (int i = 0; i < list.size(); i++) { + WrappedSignature subsig = new WrappedSignature(list.get(i)); + if (subsig.getSignatureType() == PGPSignature.PRIMARYKEY_BINDING) { + subsig.init(key); + if (subsig.verifySignature(masterKey, key)) { + ok = true; + } else { + log.add(LogLevel.WARN, LogType.MSG_KC_SUB_PRIMARY_BAD, null, indent); + continue uids; + } + } + } + if (!ok) { + log.add(LogLevel.WARN, LogType.MSG_KC_SUB_PRIMARY_NONE, null, indent); + continue; + } + } catch (Exception e) { + log.add(LogLevel.WARN, LogType.MSG_KC_SUB_PRIMARY_BAD_ERR, null, indent); + continue; + } + } + } + // if we already have a cert, and this one is not newer: skip it if (selfCert != null && selfCert.getCreationTime().before(cert.getCreationTime())) { continue; } + selfCert = zig; // if this is newer than a possibly existing revocation, drop that one if (revocation != null && selfCert.getCreationTime().after(revocation.getCreationTime())) { @@ -525,9 +561,11 @@ public class UncachedKeyRing { // re-add certification modified = PGPPublicKey.addCertification(modified, selfCert); + removedCerts -= 1; // add revocation, if any if (revocation != null) { modified = PGPPublicKey.addCertification(modified, revocation); + removedCerts -= 1; } // replace pubkey in keyring ring = PGPPublicKeyRing.insertPublicKey(ring, modified); 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 701285fa8..b5f01ce4d 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java @@ -169,6 +169,9 @@ public class OperationResultParcel implements Parcelable { MSG_KC_SUB_BAD_KEYID(R.string.msg_kc_sub_bad_keyid), MSG_KC_SUB_BAD_TIME(R.string.msg_kc_sub_bad_time), MSG_KC_SUB_BAD_TYPE(R.string.msg_kc_sub_bad_type), + MSG_KC_SUB_PRIMARY_BAD(R.string.msg_kc_sub_primary_bad), + MSG_KC_SUB_PRIMARY_BAD_ERR(R.string.msg_kc_sub_primary_bad_err), + MSG_KC_SUB_PRIMARY_NONE(R.string.msg_kc_sub_primary_none), MSG_KC_SUB_NO_CERT(R.string.msg_kc_sub_no_cert), MSG_KC_SUB_REVOKE_BAD_ERR (R.string.msg_kc_sub_revoke_bad_err), MSG_KC_SUB_REVOKE_BAD (R.string.msg_kc_sub_revoke_bad), diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index 29bcdd679..b415c6d42 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -514,8 +514,8 @@ <string name="msg_ip_fail_op_ex">Operation failed due to database error</string> <string name="msg_ip_fail_remote_ex">Operation failed due to internal error</string> <string name="msg_ip">Importing public keyring %s</string> - <string name="msg_ip_insert_keyring">Inserting keyring data</string> - <string name="msg_ip_insert_subkeys">Inserting subkeys</string> + <string name="msg_ip_insert_keyring">Encoding keyring data</string> + <string name="msg_ip_insert_subkeys">Evaluating subkeys</string> <string name="msg_ip_prepare">Preparing database operations</string> <string name="msg_ip_prepare_success">OK</string> <string name="msg_ip_preserving_secret">Preserving available secret key</string> @@ -570,6 +570,9 @@ <string name="msg_kc_sub_bad_keyid">Subkey binding issuer id mismatch</string> <string name="msg_kc_sub_bad_time">Removing subkey binding certificate with future timestamp</string> <string name="msg_kc_sub_bad_type">Unknown subkey certificate type: %s</string> + <string name="msg_kc_sub_primary_bad">Removing subkey binding certificate due to invalid primary binding certificate</string> + <string name="msg_kc_sub_primary_bad_err">Removing subkey binding certificate due to bad primary binding certificate</string> + <string name="msg_kc_sub_primary_none">Removing subkey binding certificate due to missing primary binding certificate</string> <string name="msg_kc_sub_no_cert">No valid certificate found for %s, removing from ring</string> <string name="msg_kc_sub_revoke_bad_err">Removing bad subkey revocation key</string> <string name="msg_kc_sub_revoke_bad">Removing bad subkey revocation key</string> |