From 158263f2555eec0bb6e2b3738fa2edfbca71ae72 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Sun, 17 May 2015 00:35:10 +0200 Subject: apply promote operation to specific subkeys present on yubikey only --- .../keychain/pgp/CanonicalizedKeyRing.java | 7 ++++++- .../keychain/pgp/CanonicalizedPublicKeyRing.java | 17 +++++++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedKeyRing.java index 4adacaf23..432ba23e9 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedKeyRing.java @@ -19,6 +19,7 @@ package org.sufficientlysecure.keychain.pgp; import org.spongycastle.openpgp.PGPKeyRing; +import org.spongycastle.openpgp.PGPPublicKey; import org.sufficientlysecure.keychain.pgp.exception.PgpKeyNotFoundException; import org.sufficientlysecure.keychain.util.IterableIterator; @@ -127,7 +128,11 @@ public abstract class CanonicalizedKeyRing extends KeyRing { } public CanonicalizedPublicKey getPublicKey(long id) { - return new CanonicalizedPublicKey(this, getRing().getPublicKey(id)); + PGPPublicKey pubKey = getRing().getPublicKey(id); + if (pubKey == null) { + return null; + } + return new CanonicalizedPublicKey(this, pubKey); } public byte[] getEncoded() throws IOException { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedPublicKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedPublicKeyRing.java index 8432b8f9f..68fd4a428 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedPublicKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedPublicKeyRing.java @@ -103,9 +103,22 @@ public class CanonicalizedPublicKeyRing extends CanonicalizedKeyRing { } /** Create a dummy secret ring from this key */ - public UncachedKeyRing createDivertSecretRing (byte[] cardAid) { + public UncachedKeyRing createDivertSecretRing (byte[] cardAid, long[] subKeyIds) { PGPSecretKeyRing secRing = PGPSecretKeyRing.constructDummyFromPublic(getRing(), cardAid); - return new UncachedKeyRing(secRing); + + if (subKeyIds == null) { + return new UncachedKeyRing(secRing); + } + + // if only specific subkeys should be promoted, construct a + // stripped dummy, then move divert-to-card keys over + PGPSecretKeyRing newRing = PGPSecretKeyRing.constructDummyFromPublic(getRing()); + for (long subKeyId : subKeyIds) { + newRing = PGPSecretKeyRing.insertSecretKey(newRing, secRing.getSecretKey(subKeyId)); + } + + return new UncachedKeyRing(newRing); + } } \ No newline at end of file -- cgit v1.2.3 From f30900d085f38034708f62f94e46a38f222d4cb7 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Wed, 20 May 2015 12:47:02 +0200 Subject: add user id flags to UserAttribute self signatures --- .../keychain/pgp/PgpKeyOperation.java | 33 +++++++++++++--------- 1 file changed, 20 insertions(+), 13 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') 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 89db378a9..2f771d8f2 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -553,7 +553,8 @@ public class PgpKeyOperation { PGPSignature cert = generateUserAttributeSignature( getSignatureGenerator(masterSecretKey, cryptoInput), cryptoInput.getSignatureTime(), - masterPrivateKey, masterPublicKey, vector); + masterPrivateKey, masterPublicKey, vector, + masterKeyFlags, masterKeyExpiry); modifiedPublicKey = PGPPublicKey.addCertification(modifiedPublicKey, vector, cert); } catch (NfcInteractionNeeded e) { nfcSignOps.addHash(e.hashToSign, e.hashAlgo); @@ -1319,11 +1320,9 @@ public class PgpKeyOperation { } - private PGPSignature generateUserIdSignature( - PGPSignatureGenerator sGen, Date creationTime, - PGPPrivateKey masterPrivateKey, PGPPublicKey pKey, String userId, boolean primary, - int flags, long expiry) - throws IOException, PGPException, SignatureException { + private static PGPSignatureSubpacketGenerator generateHashedSelfSigSubpackets( + Date creationTime, PGPPublicKey pKey, boolean primary, int flags, long expiry + ) { PGPSignatureSubpacketGenerator hashedPacketsGen = new PGPSignatureSubpacketGenerator(); { @@ -1357,6 +1356,17 @@ public class PgpKeyOperation { } } + return hashedPacketsGen; + } + + private static PGPSignature generateUserIdSignature( + PGPSignatureGenerator sGen, Date creationTime, + PGPPrivateKey masterPrivateKey, PGPPublicKey pKey, String userId, boolean primary, + int flags, long expiry) + throws IOException, PGPException, SignatureException { + + PGPSignatureSubpacketGenerator hashedPacketsGen = + generateHashedSelfSigSubpackets(creationTime, pKey, primary, flags, expiry); sGen.setHashedSubpackets(hashedPacketsGen.generate()); sGen.init(PGPSignature.POSITIVE_CERTIFICATION, masterPrivateKey); return sGen.generateCertification(userId, pKey); @@ -1365,15 +1375,12 @@ public class PgpKeyOperation { private static PGPSignature generateUserAttributeSignature( PGPSignatureGenerator sGen, Date creationTime, PGPPrivateKey masterPrivateKey, PGPPublicKey pKey, - PGPUserAttributeSubpacketVector vector) + PGPUserAttributeSubpacketVector vector, + int flags, long expiry) throws IOException, PGPException, SignatureException { - PGPSignatureSubpacketGenerator hashedPacketsGen = new PGPSignatureSubpacketGenerator(); - { - /* critical subpackets: we consider those important for a modern pgp implementation */ - hashedPacketsGen.setSignatureCreationTime(true, creationTime); - } - + PGPSignatureSubpacketGenerator hashedPacketsGen = + generateHashedSelfSigSubpackets(creationTime, pKey, false, flags, expiry); sGen.setHashedSubpackets(hashedPacketsGen.generate()); sGen.init(PGPSignature.POSITIVE_CERTIFICATION, masterPrivateKey); return sGen.generateCertification(vector, pKey); -- cgit v1.2.3 From 5c8af1c5a5ad4be2bf3f2f657fe3fbd2f1fe8a24 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 28 May 2015 02:27:44 +0200 Subject: don't show allowed key list if no key exists, and some minor PgpDecryptVerify changes --- .../keychain/pgp/PgpDecryptVerify.java | 37 ++++++++++++++-------- 1 file changed, 24 insertions(+), 13 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java index f6580b85a..aa1125800 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpDecryptVerify.java @@ -384,7 +384,7 @@ public class PgpDecryptVerify extends BaseOperation { } if (enc == null) { - log.add(LogType.MSG_DC_ERROR_INVALID_SIGLIST, indent); + log.add(LogType.MSG_DC_ERROR_INVALID_DATA, indent); return new DecryptVerifyResult(DecryptVerifyResult.RESULT_ERROR, log); } @@ -419,6 +419,7 @@ public class PgpDecryptVerify extends BaseOperation { } Passphrase passphrase = null; + boolean skippedDisallowedKey = false; // go through all objects and find one we can decrypt while (it.hasNext()) { @@ -451,13 +452,6 @@ public class PgpDecryptVerify extends BaseOperation { log.add(LogType.MSG_DC_ASKIP_NO_KEY, indent + 1); continue; } - // get subkey which has been used for this encryption packet - secretEncryptionKey = secretKeyRing.getSecretKey(subKeyId); - if (secretEncryptionKey == null) { - // should actually never happen, so no need to be more specific. - log.add(LogType.MSG_DC_ASKIP_NO_KEY, indent + 1); - continue; - } // allow only specific keys for decryption? if (mAllowedKeyIds != null) { @@ -469,11 +463,20 @@ public class PgpDecryptVerify extends BaseOperation { if (!mAllowedKeyIds.contains(masterKeyId)) { // this key is in our db, but NOT allowed! // continue with the next packet in the while loop + skippedDisallowedKey = true; log.add(LogType.MSG_DC_ASKIP_NOT_ALLOWED, indent + 1); continue; } } + // get subkey which has been used for this encryption packet + secretEncryptionKey = secretKeyRing.getSecretKey(subKeyId); + if (secretEncryptionKey == null) { + // should actually never happen, so no need to be more specific. + log.add(LogType.MSG_DC_ASKIP_NO_KEY, indent + 1); + continue; + } + /* secret key exists in database and is allowed! */ asymmetricPacketFound = true; @@ -604,10 +607,18 @@ public class PgpDecryptVerify extends BaseOperation { } encryptedData = encryptedDataAsymmetric; } else { - // If we didn't find any useful data, error out + // there wasn't even any useful data + if (!anyPacketFound) { + log.add(LogType.MSG_DC_ERROR_NO_DATA, indent + 1); + return new DecryptVerifyResult(DecryptVerifyResult.RESULT_NO_DATA, log); + } + // there was data but key wasn't allowed + if (skippedDisallowedKey) { + log.add(LogType.MSG_DC_ERROR_NO_KEY, indent + 1); + return new DecryptVerifyResult(DecryptVerifyResult.RESULT_KEY_DISALLOWED, log); + } // no packet has been found where we have the corresponding secret key in our db - log.add( - anyPacketFound ? LogType.MSG_DC_ERROR_NO_KEY : LogType.MSG_DC_ERROR_NO_DATA, indent + 1); + log.add(LogType.MSG_DC_ERROR_NO_KEY, indent + 1); return new DecryptVerifyResult(DecryptVerifyResult.RESULT_ERROR, log); } @@ -910,7 +921,7 @@ public class PgpDecryptVerify extends BaseOperation { PGPSignatureList sigList = (PGPSignatureList) pgpFact.nextObject(); if (sigList == null) { - log.add(LogType.MSG_DC_ERROR_INVALID_SIGLIST, 0); + log.add(LogType.MSG_DC_ERROR_INVALID_DATA, 0); return new DecryptVerifyResult(DecryptVerifyResult.RESULT_ERROR, log); } @@ -993,7 +1004,7 @@ public class PgpDecryptVerify extends BaseOperation { } else if (o instanceof PGPSignatureList) { sigList = (PGPSignatureList) o; } else { - log.add(LogType.MSG_DC_ERROR_INVALID_SIGLIST, 0); + log.add(LogType.MSG_DC_ERROR_INVALID_DATA, 0); return new DecryptVerifyResult(DecryptVerifyResult.RESULT_ERROR, log); } -- cgit v1.2.3 From eb830c6786c500d69ce2a06203b7cf336ae8a9bf Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 28 May 2015 11:40:35 +0200 Subject: warn on signature earlier than key creation, err on significantly earlier --- .../keychain/pgp/UncachedKeyRing.java | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') 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 2bb4f7dc4..ecf68890e 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java @@ -820,6 +820,15 @@ public class UncachedKeyRing { continue; } + Date keyCreationTime = key.getCreationTime(), keyCreationTimeLenient; + { + Calendar keyCreationCal = Calendar.getInstance(); + keyCreationCal.setTime(keyCreationTime); + // allow for diverging clocks up to one day when checking creation time + keyCreationCal.add(Calendar.MINUTE, -5); + keyCreationTimeLenient = keyCreationCal.getTime(); + } + // A subkey needs exactly one subkey binding certificate, and optionally one revocation // certificate. PGPPublicKey modified = key; @@ -851,6 +860,18 @@ public class UncachedKeyRing { continue; } + if (cert.getCreationTime().before(keyCreationTime)) { + // Signature is earlier than key creation time + log.add(LogType.MSG_KC_SUB_BAD_TIME_EARLY, indent); + // due to an earlier accident, we generated keys which had creation timestamps + // a few seconds after their signature timestamp. for compatibility, we only + // error out with some margin of error + if (cert.getCreationTime().before(keyCreationTimeLenient)) { + badCerts += 1; + continue; + } + } + if (cert.isLocal()) { // Creation date in the future? No way! log.add(LogType.MSG_KC_SUB_BAD_LOCAL, indent); -- cgit v1.2.3 From 8de0d9e6da63c6755b86c6b9850def057e4bd8c0 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 28 May 2015 14:41:26 +0200 Subject: create keys with fixed timestamp --- .../sufficientlysecure/keychain/pgp/PgpKeyOperation.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp') 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 2f771d8f2..991d013ae 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -151,7 +151,7 @@ public class PgpKeyOperation { } /** Creates new secret key. */ - private PGPKeyPair createKey(SubkeyAdd add, OperationLog log, int indent) { + private PGPKeyPair createKey(SubkeyAdd add, Date creationTime, OperationLog log, int indent) { try { // Some safety checks @@ -249,7 +249,7 @@ public class PgpKeyOperation { } // build new key pair - return new JcaPGPKeyPair(algorithm, keyGen.generateKeyPair(), new Date()); + return new JcaPGPKeyPair(algorithm, keyGen.generateKeyPair(), creationTime); } catch(NoSuchProviderException | InvalidAlgorithmParameterException e) { throw new RuntimeException(e); @@ -295,8 +295,10 @@ public class PgpKeyOperation { return new PgpEditKeyResult(PgpEditKeyResult.RESULT_ERROR, log, null); } + Date creationTime = new Date(); + subProgressPush(10, 30); - PGPKeyPair keyPair = createKey(add, log, indent); + PGPKeyPair keyPair = createKey(add, creationTime, log, indent); subProgressPop(); // return null if this failed (an error will already have been logged by createKey) @@ -323,8 +325,8 @@ public class PgpKeyOperation { masterSecretKey.getEncoded(), new JcaKeyFingerprintCalculator()); subProgressPush(50, 100); - CryptoInputParcel cryptoInput = new CryptoInputParcel(new Date(), new Passphrase("")); - return internal(sKR, masterSecretKey, add.mFlags, add.mExpiry, cryptoInput, saveParcel, log); + CryptoInputParcel cryptoInput = new CryptoInputParcel(creationTime, new Passphrase("")); + return internal(sKR, masterSecretKey, add.mFlags, add.mExpiry, cryptoInput, saveParcel, log, indent); } catch (PGPException e) { log.add(LogType.MSG_CR_ERROR_INTERNAL_PGP, indent); @@ -900,7 +902,7 @@ public class PgpKeyOperation { (i-1) * (100 / saveParcel.mAddSubKeys.size()), i * (100 / saveParcel.mAddSubKeys.size()) ); - PGPKeyPair keyPair = createKey(add, log, indent); + PGPKeyPair keyPair = createKey(add, cryptoInput.getSignatureTime(), log, indent); subProgressPop(); if (keyPair == null) { log.add(LogType.MSG_MF_ERROR_PGP, indent +1); -- cgit v1.2.3