make getSignId a secret key operation, and respect unavailable keys

This one should remedy #811, but waiting for a test
This commit is contained in:
Vincent Breitmoser 2014-10-02 19:23:08 +02:00
parent 9a296c012d
commit 37cb5c4c78
8 changed files with 38 additions and 112 deletions

View File

@ -78,7 +78,7 @@ public abstract class CanonicalizedKeyRing extends KeyRing {
public long getEncryptId() throws PgpGeneralException {
for(CanonicalizedPublicKey key : publicKeyIterator()) {
if(key.canEncrypt()) {
if (key.canEncrypt() && key.isValid()) {
return key.getKeyId();
}
}
@ -94,24 +94,6 @@ public abstract class CanonicalizedKeyRing extends KeyRing {
}
}
public long getSignId() throws PgpGeneralException {
for(CanonicalizedPublicKey key : publicKeyIterator()) {
if(key.canSign()) {
return key.getKeyId();
}
}
throw new PgpGeneralException("No valid signing key found!");
}
public boolean hasSign() throws PgpGeneralException {
try {
getSignId();
return true;
} catch (PgpGeneralException e) {
return false;
}
}
public void encode(OutputStream stream) throws IOException {
getRing().encode(stream);
}

View File

@ -104,4 +104,10 @@ public class CanonicalizedPublicKey extends UncachedPublicKey {
public Integer getKeyUsage() {
return super.getKeyUsage();
}
/** Returns whether this key is valid, ie not expired or revoked. */
public boolean isValid() {
return !isRevoked() && !isExpired();
}
}

View File

@ -30,6 +30,8 @@ import org.spongycastle.openpgp.operator.PBESecretKeyDecryptor;
import org.spongycastle.openpgp.operator.jcajce.JcePBESecretKeyDecryptorBuilder;
import org.sufficientlysecure.keychain.Constants;
import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
import org.sufficientlysecure.keychain.provider.KeychainContract;
import org.sufficientlysecure.keychain.provider.ProviderHelper;
import org.sufficientlysecure.keychain.util.IterableIterator;
import org.sufficientlysecure.keychain.util.Log;
@ -74,43 +76,18 @@ public class CanonicalizedSecretKeyRing extends CanonicalizedKeyRing {
return new CanonicalizedSecretKey(this, mRing.getSecretKey(id));
}
/** Getter that returns the subkey that should be used for signing. */
CanonicalizedSecretKey getSigningSubKey() throws PgpGeneralException {
PGPSecretKey key = mRing.getSecretKey(getSignId());
if(key != null) {
CanonicalizedSecretKey cKey = new CanonicalizedSecretKey(this, key);
if(!cKey.canSign()) {
throw new PgpGeneralException("key error");
}
return cKey;
}
// TODO handle with proper exception
throw new PgpGeneralException("no signing key available");
}
public boolean hasPassphrase() {
PGPSecretKey secretKey = null;
boolean foundValidKey = false;
for (Iterator keys = mRing.getSecretKeys(); keys.hasNext(); ) {
secretKey = (PGPSecretKey) keys.next();
if (!secretKey.isPrivateKeyEmpty()) {
foundValidKey = true;
break;
/** Returns the key id which should be used for signing.
*
* This method returns keys which are actually available (ie. secret available, and not stripped,
* revoked, or expired), hence only works on keyrings where a secret key is available!
*/
public long getSecretSignId() throws PgpGeneralException {
for(CanonicalizedSecretKey key : secretKeyIterator()) {
if (key.canSign() && key.isValid() && key.getSecretKeyType().isUsable()) {
return key.getKeyId();
}
}
if(!foundValidKey) {
return false;
}
try {
PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder()
.setProvider("SC").build("".toCharArray());
PGPPrivateKey testKey = secretKey.extractPrivateKey(keyDecryptor);
return testKey == null;
} catch(PGPException e) {
// this means the crc check failed -> passphrase required
return true;
}
throw new PgpGeneralException("no valid signing key available");
}
public IterableIterator<CanonicalizedSecretKey> secretKeyIterator() {

View File

@ -56,10 +56,6 @@ public abstract class KeyRing {
abstract public boolean hasEncrypt() throws PgpGeneralException;
abstract public long getSignId() throws PgpGeneralException;
abstract public boolean hasSign() throws PgpGeneralException;
abstract public int getVerified() throws PgpGeneralException;
private static final Pattern USER_ID_PATTERN = Pattern.compile("^(.*?)(?: \\((.*)\\))?(?: <(.*)>)?$");

View File

@ -51,12 +51,9 @@ public class UncachedPublicKey {
/** The revocation signature is NOT checked here, so this may be false! */
public boolean isRevoked() {
for (PGPSignature sig : new IterableIterator<PGPSignature>(
mPublicKey.getSignaturesOfType(isMasterKey() ? PGPSignature.KEY_REVOCATION
: PGPSignature.SUBKEY_REVOCATION))) {
return true;
}
return false;
return mPublicKey.getSignaturesOfType(isMasterKey()
? PGPSignature.KEY_REVOCATION
: PGPSignature.SUBKEY_REVOCATION).hasNext();
}
public Date getCreationTime() {

View File

@ -135,65 +135,33 @@ public class CachedPublicKeyRing extends KeyRing {
@Override
public long getEncryptId() throws PgpGeneralException {
try {
Cursor subkeys = getSubkeys();
if (subkeys != null) {
try {
while (subkeys.moveToNext()) {
if (subkeys.getInt(subkeys.getColumnIndexOrThrow(KeychainContract.Keys.CAN_ENCRYPT)) != 0) {
return subkeys.getLong(subkeys.getColumnIndexOrThrow(KeychainContract.Keys.KEY_ID));
}
}
} finally {
subkeys.close();
}
}
} catch(Exception e) {
throw new PgpGeneralException(e);
}
throw new PgpGeneralException("No encrypt key found");
}
@Override
public boolean hasEncrypt() throws PgpGeneralException {
try {
Object data = mProviderHelper.getGenericData(mUri,
KeychainContract.KeyRings.HAS_ENCRYPT,
KeyRings.HAS_ENCRYPT,
ProviderHelper.FIELD_TYPE_INTEGER);
return (Long) data > 0;
return (Long) data;
} catch(ProviderHelper.NotFoundException e) {
throw new PgpGeneralException(e);
}
}
@Override
public long getSignId() throws PgpGeneralException {
try {
Cursor subkeys = getSubkeys();
if (subkeys != null) {
try {
while (subkeys.moveToNext()) {
if (subkeys.getInt(subkeys.getColumnIndexOrThrow(KeychainContract.Keys.CAN_SIGN)) != 0) {
return subkeys.getLong(subkeys.getColumnIndexOrThrow(KeychainContract.Keys.KEY_ID));
}
}
} finally {
subkeys.close();
}
}
} catch(Exception e) {
throw new PgpGeneralException(e);
}
throw new PgpGeneralException("No sign key found");
public boolean hasEncrypt() throws PgpGeneralException {
return getEncryptId() != 0;
}
@Override
public boolean hasSign() throws PgpGeneralException {
/** Returns the key id which should be used for signing.
*
* This method returns keys which are actually available (ie. secret available, and not stripped,
* revoked, or expired), hence only works on keyrings where a secret key is available!
*
*/
public long getSecretSignId() throws PgpGeneralException {
try {
Object data = mProviderHelper.getGenericData(mUri,
KeychainContract.KeyRings.HAS_SIGN,
KeyRings.HAS_SIGN,
ProviderHelper.FIELD_TYPE_INTEGER);
return (Long) data > 0;
return (Long) data;
} catch(ProviderHelper.NotFoundException e) {
throw new PgpGeneralException(e);
}

View File

@ -293,7 +293,7 @@ public class OpenPgpService extends RemoteService {
// Find the appropriate subkey to sign with
CachedPublicKeyRing signingRing =
new ProviderHelper(this).getCachedPublicKeyRing(accSettings.getKeyId());
final long sigSubKeyId = signingRing.getSignId();
final long sigSubKeyId = signingRing.getSecretSignId();
// sign-only
PgpSignEncrypt.Builder builder = new PgpSignEncrypt.Builder(
@ -405,7 +405,7 @@ public class OpenPgpService extends RemoteService {
// Find the appropriate subkey to sign with
CachedPublicKeyRing signingRing =
new ProviderHelper(this).getCachedPublicKeyRing(accSettings.getKeyId());
final long sigSubKeyId = signingRing.getSignId();
final long sigSubKeyId = signingRing.getSecretSignId();
String passphrase;
if (data.hasExtra(OpenPgpApi.EXTRA_PASSPHRASE)) {

View File

@ -294,7 +294,7 @@ public class KeychainIntentService extends IntentService implements Progressable
// Find the appropriate subkey to sign with
CachedPublicKeyRing signingRing =
new ProviderHelper(this).getCachedPublicKeyRing(sigMasterKeyId);
long sigSubKeyId = signingRing.getSignId();
long sigSubKeyId = signingRing.getSecretSignId();
// Set signature settings
builder.setSignatureMasterKeyId(sigMasterKeyId)