From 437da180fc6bd427a50ef0cc65b9b379cbd646d3 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Sun, 6 Apr 2014 00:58:11 +0200 Subject: certs: properly handle primary flag and revocation of uids --- .../keychain/provider/KeychainContract.java | 4 +- .../keychain/provider/KeychainDatabase.java | 1 + .../keychain/provider/KeychainProvider.java | 2 +- .../keychain/provider/ProviderHelper.java | 113 ++++++++++++++------- .../keychain/ui/ViewCertActivity.java | 55 +++++----- 5 files changed, 109 insertions(+), 66 deletions(-) diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainContract.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainContract.java index 4a2dd2da2..28d54d818 100644 --- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainContract.java +++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainContract.java @@ -52,6 +52,7 @@ public class KeychainContract { String USER_ID = "user_id"; // not a database id String RANK = "rank"; // ONLY used for sorting! no key, no nothing! String IS_PRIMARY = "is_primary"; + String IS_REVOKED = "is_revoked"; } interface CertsColumns { @@ -107,7 +108,8 @@ public class KeychainContract { public static final String PATH_ACCOUNTS = "accounts"; public static class KeyRings implements BaseColumns, KeysColumns, UserIdsColumns { - public static final String MASTER_KEY_ID = "master_key_id"; + public static final String MASTER_KEY_ID = KeysColumns.MASTER_KEY_ID; + public static final String IS_REVOKED = KeysColumns.IS_REVOKED; public static final String HAS_SECRET = "has_secret"; public static final Uri CONTENT_URI = BASE_CONTENT_URI_INTERNAL.buildUpon() diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java index c704da0e7..2bd1c13a0 100644 --- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java +++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java @@ -96,6 +96,7 @@ public class KeychainDatabase extends SQLiteOpenHelper { + UserIdsColumns.USER_ID + " CHARMANDER, " + UserIdsColumns.IS_PRIMARY + " BOOLEAN, " + + UserIdsColumns.IS_REVOKED + " BOOLEAN, " + UserIdsColumns.RANK+ " INTEGER, " + "PRIMARY KEY(" + UserIdsColumns.MASTER_KEY_ID + ", " + UserIdsColumns.USER_ID + "), " diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java index 0b15187dd..7365005e4 100644 --- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java +++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java @@ -249,7 +249,7 @@ public class KeychainProvider extends ContentProvider { projectionMap.put(KeyRings.MASTER_KEY_ID, Tables.KEYS + "." + Keys.MASTER_KEY_ID); projectionMap.put(KeyRings.KEY_ID, Keys.KEY_ID); projectionMap.put(KeyRings.KEY_SIZE, Keys.KEY_SIZE); - projectionMap.put(KeyRings.IS_REVOKED, Keys.IS_REVOKED); + projectionMap.put(KeyRings.IS_REVOKED, Tables.KEYS + "." + Keys.IS_REVOKED); projectionMap.put(KeyRings.CAN_CERTIFY, Keys.CAN_CERTIFY); projectionMap.put(KeyRings.CAN_ENCRYPT, Keys.CAN_ENCRYPT); projectionMap.put(KeyRings.CAN_SIGN, Keys.CAN_SIGN); diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java index 1b653ae06..b2d41a151 100644 --- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java +++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java @@ -32,7 +32,6 @@ import android.os.RemoteException; import org.spongycastle.bcpg.SignatureSubpacketTags; import org.spongycastle.bcpg.sig.SignatureExpirationTime; import org.spongycastle.openpgp.PGPException; -import org.spongycastle.openpgp.PGPSecretKey; import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentVerifierBuilderProvider; import org.spongycastle.openpgp.PGPKeyRing; import org.spongycastle.openpgp.PGPPublicKey; @@ -57,8 +56,10 @@ import org.sufficientlysecure.keychain.util.Log; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.Date; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.HashSet; import java.util.Set; @@ -229,19 +230,40 @@ public class ProviderHelper { if(secretRing != null) allKeyRings.put(secretRing.getSecretKey().getKeyID(), secretRing); - int userIdRank = 0; - for (String userId : new IterableIterator(masterKey.getUserIDs())) { - operations.add(buildUserIdOperations(context, masterKeyId, userId, userIdRank)); + // classify and order user ids. primary are moved to the front, revoked to the back, + // otherwise the order in the keyfile is preserved. + List uids = new ArrayList(); - // HashMap certs = new HashMap(); + for (String userId : new IterableIterator(masterKey.getUserIDs())) { + UserIdItem item = new UserIdItem(); + uids.add(item); + item.userId = userId; // look through signatures for this specific key for (PGPSignature cert : new IterableIterator( masterKey.getSignaturesForID(userId))) { long certId = cert.getKeyID(); - int verified = 0; - // verify from the key itself try { + // self signature + if(certId == masterKeyId) { + cert.init( + new JcaPGPContentVerifierBuilderProvider().setProvider( + Constants.BOUNCY_CASTLE_PROVIDER_NAME), + masterKey); + if(!cert.verifyCertification(userId, masterKey)) { + // not verified?! dang! TODO notify user? this is kinda serious... + Log.e(Constants.TAG, "Could not verify self signature for " + userId + "!"); + continue; + } + // is this the first, or a more recent certificate? + if(item.selfCert == null || + item.selfCert.getCreationTime().before(cert.getCreationTime())) { + item.selfCert = cert; + item.isPrimary = cert.getHashedSubPackets().isPrimaryUserID(); + item.isRevoked = + cert.getSignatureType() == PGPSignature.CERTIFICATION_REVOCATION; + } + } // verify signatures from known private keys if(allKeyRings.containsKey(certId)) { // mark them as verified @@ -249,20 +271,9 @@ public class ProviderHelper { new JcaPGPContentVerifierBuilderProvider().setProvider( Constants.BOUNCY_CASTLE_PROVIDER_NAME), allKeyRings.get(certId).getPublicKey()); - verified = cert.verifyCertification(userId, masterKey) ? Certs.VERIFIED_SECRET : 0; - Log.d(Constants.TAG, "Verified sig for " + userId + " " + verified + " from " - + PgpKeyHelper.convertKeyIdToHex(certId) - ); - // if that didn't work out, is it at least an own signature? - } else if(certId == masterKeyId) { - cert.init( - new JcaPGPContentVerifierBuilderProvider().setProvider( - Constants.BOUNCY_CASTLE_PROVIDER_NAME), - masterKey); - verified = cert.verifyCertification(userId, masterKey) ? Certs.VERIFIED_SELF : 0; - Log.d(Constants.TAG, "Verified sig for " + userId + " " + verified + " from " - + PgpKeyHelper.convertKeyIdToHex(certId) - ); + if(cert.verifyCertification(userId, masterKey)) { + item.trustedCerts.add(cert); + } } } catch(SignatureException e) { Log.e(Constants.TAG, "Signature verification failed! " @@ -275,15 +286,25 @@ public class ProviderHelper { + " from " + PgpKeyHelper.convertKeyIdToHex(cert.getKeyID()), e); } - Log.d(Constants.TAG, "sig for " + userId + " from " - + PgpKeyHelper.convertKeyIdToHex(cert.getKeyID()) - ); - // regardless of verification, save the certification - operations.add(buildCertOperations( - context, masterKeyId, userIdRank, cert, verified)); } + } - ++userIdRank; + // primary before regular before revoked (see UserIdItem.compareTo) + // this is a stable sort, so the order of keys is otherwise preserved. + Collections.sort(uids); + // iterate and put into db + for(int userIdRank = 0; userIdRank < uids.size(); userIdRank++) { + UserIdItem item = uids.get(userIdRank); + operations.add(buildUserIdOperations(masterKeyId, item, userIdRank)); + // no self cert is bad, but allowed by the rfc... + if(item.selfCert != null) { + operations.add(buildCertOperations( + masterKeyId, userIdRank, item.selfCert, Certs.VERIFIED_SELF)); + } + for(int i = 0; i < item.trustedCerts.size(); i++) { + operations.add(buildCertOperations( + masterKeyId, userIdRank, item.trustedCerts.get(i), Certs.VERIFIED_SECRET)); + } } try { @@ -301,6 +322,25 @@ public class ProviderHelper { } + private static class UserIdItem implements Comparable { + String userId; + boolean isPrimary = false; + boolean isRevoked = false; + PGPSignature selfCert; + List trustedCerts = new ArrayList(); + + @Override + public int compareTo(UserIdItem o) { + // if one key is primary but the other isn't, the primary one always comes first + if(isPrimary != o.isPrimary) + return isPrimary ? -1 : 1; + // revoked keys always come last! + if(isRevoked != o.isRevoked) + return isRevoked ? 1 : -1; + return 0; + } + } + /** * Saves a PGPSecretKeyRing in the DB. This will only work if a corresponding public keyring * is already in the database! @@ -368,11 +408,10 @@ public class ProviderHelper { /** * Build ContentProviderOperation to add PGPPublicKey to database corresponding to a keyRing */ - private static ContentProviderOperation buildCertOperations(Context context, - long masterKeyId, - int rank, - PGPSignature cert, - int verified) + private static ContentProviderOperation buildCertOperations(long masterKeyId, + int rank, + PGPSignature cert, + int verified) throws IOException { ContentValues values = new ContentValues(); values.put(Certs.MASTER_KEY_ID, masterKeyId); @@ -395,11 +434,13 @@ public class ProviderHelper { /** * Build ContentProviderOperation to add PublicUserIds to database corresponding to a keyRing */ - private static ContentProviderOperation buildUserIdOperations(Context context, - long masterKeyId, String userId, int rank) { + private static ContentProviderOperation buildUserIdOperations(long masterKeyId, UserIdItem item, + int rank) { ContentValues values = new ContentValues(); values.put(UserIds.MASTER_KEY_ID, masterKeyId); - values.put(UserIds.USER_ID, userId); + values.put(UserIds.USER_ID, item.userId); + values.put(UserIds.IS_PRIMARY, item.isPrimary); + values.put(UserIds.IS_REVOKED, item.isRevoked); values.put(UserIds.RANK, rank); Uri uri = UserIds.buildUserIdsUri(Long.toString(masterKeyId)); diff --git a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewCertActivity.java b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewCertActivity.java index ca548ac34..e5f2fb173 100644 --- a/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewCertActivity.java +++ b/OpenPGP-Keychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewCertActivity.java @@ -54,18 +54,21 @@ public class ViewCertActivity extends ActionBarActivity static final String[] PROJECTION = new String[] { Certs.MASTER_KEY_ID, Certs.USER_ID, + Certs.TYPE, Certs.CREATION, + Certs.EXPIRY, Certs.KEY_ID_CERTIFIER, Certs.SIGNER_UID, Certs.TYPE }; private static final int INDEX_MASTER_KEY_ID = 1; private static final int INDEX_USER_ID = 2; - private static final int INDEX_CREATION = 3; - private static final int INDEX_KEY_ID_CERTIFIER = 4; - private static final int INDEX_UID_CERTIFIER = 5; - private static final int INDEX_KEY_DATA = 6; - private static final int INDEX_KEY_TYPE = 6; + private static final int INDEX_TYPE = 3; + private static final int INDEX_CREATION = 4; + private static final int INDEX_EXPIRY = 5; + private static final int INDEX_KEY_ID_CERTIFIER = 6; + private static final int INDEX_UID_CERTIFIER = 7; + private static final int INDEX_KEY_TYPE = 8; private Uri mDataUri; @@ -133,30 +136,26 @@ public class ViewCertActivity extends ActionBarActivity else mSignerUid.setText(R.string.unknown_uid); - byte[] sigData = data.getBlob(INDEX_KEY_DATA); - PGPSignature sig = PgpConversionHelper.BytesToPGPSignature(sigData); - if(sig != null) { - String algorithmStr = PgpKeyHelper.getAlgorithmInfo(sig.getKeyAlgorithm(), 0); - mAlgorithm.setText(algorithmStr); - - switch(sig.getSignatureType()) { - case PGPSignature.DEFAULT_CERTIFICATION: - mType.setText(R.string.sig_type_default); break; - case PGPSignature.NO_CERTIFICATION: - mType.setText(R.string.sig_type_none); break; - case PGPSignature.CASUAL_CERTIFICATION: - mType.setText(R.string.sig_type_casual); break; - case PGPSignature.POSITIVE_CERTIFICATION: - mType.setText(R.string.sig_type_positive); break; - } + // String algorithmStr = PgpKeyHelper.getAlgorithmInfo(sig.getKeyAlgorithm(), 0); + // mAlgorithm.setText(algorithmStr); + + switch(data.getInt(INDEX_TYPE)) { + case PGPSignature.DEFAULT_CERTIFICATION: + mType.setText(R.string.sig_type_default); break; + case PGPSignature.NO_CERTIFICATION: + mType.setText(R.string.sig_type_none); break; + case PGPSignature.CASUAL_CERTIFICATION: + mType.setText(R.string.sig_type_casual); break; + case PGPSignature.POSITIVE_CERTIFICATION: + mType.setText(R.string.sig_type_positive); break; + } - long expiry = sig.getHashedSubPackets().getSignatureExpirationTime(); - if(expiry == 0) - mExpiry.setText("never"); - else { - Date expiryDate = new Date(creationDate.getTime() + expiry * 1000); - mExpiry.setText(DateFormat.getDateFormat(getApplicationContext()).format(expiryDate)); - } + long expiry = data.getLong(INDEX_EXPIRY); + if(expiry == 0) + mExpiry.setText("never"); + else { + Date expiryDate = new Date(creationDate.getTime() + expiry * 1000); + mExpiry.setText(DateFormat.getDateFormat(getApplicationContext()).format(expiryDate)); } } } -- cgit v1.2.3