be more mindful of algorithm and usage flag interaction

Fixes #895
This commit is contained in:
Vincent Breitmoser 2014-09-28 14:25:54 +02:00
parent c0abae5cc3
commit 706e60474d
4 changed files with 44 additions and 16 deletions

View File

@ -619,8 +619,7 @@ public class UncachedKeyRing {
boolean needsPrimaryBinding = false;
// If the algorithm is even suitable for signing
if (key.getAlgorithm() != PublicKeyAlgorithmTags.ELGAMAL_ENCRYPT
&& key.getAlgorithm() != PublicKeyAlgorithmTags.RSA_ENCRYPT) {
if (isSigningAlgo(key.getAlgorithm())) {
// If this certificate says it allows signing for the key
if (zert.getHashedSubPackets() != null &&
@ -722,6 +721,24 @@ public class UncachedKeyRing {
continue;
}
// If we have flags, check if the algorithm supports all of them
if (selfCert.getHashedSubPackets() == null
&& selfCert.getHashedSubPackets().hasSubpacket(SignatureSubpacketTags.KEY_FLAGS)) {
int flags = ((KeyFlags) selfCert.getHashedSubPackets().getSubpacket(SignatureSubpacketTags.KEY_FLAGS)).getFlags();
int algo = key.getAlgorithm();
// If this is a signing key, but not a signing algorithm, warn the user
if (!isSigningAlgo(algo) && (flags & PGPKeyFlags.CAN_SIGN) == PGPKeyFlags.CAN_SIGN) {
log.add(LogType.MSG_KC_SUB_ALGO_BAD_SIGN, indent);
}
// If this is an encryption key, but not an encryption algorithm, warn the user
if (!isEncryptionAlgo(algo) && (
(flags & PGPKeyFlags.CAN_ENCRYPT_COMMS) == PGPKeyFlags.CAN_ENCRYPT_COMMS
|| (flags & PGPKeyFlags.CAN_ENCRYPT_STORAGE) == PGPKeyFlags.CAN_ENCRYPT_STORAGE
)) {
log.add(LogType.MSG_KC_SUB_ALGO_BAD_ENCRYPT, indent);
}
}
// re-add certification
modified = PGPPublicKey.addCertification(modified, selfCert);
// add revocation, if any
@ -953,4 +970,23 @@ public class UncachedKeyRing {
}
}
/** Returns true if the algorithm is of a type which is suitable for signing. */
static boolean isSigningAlgo(int algorithm) {
return algorithm == PGPPublicKey.RSA_GENERAL
|| algorithm == PGPPublicKey.RSA_SIGN
|| algorithm == PGPPublicKey.DSA
|| algorithm == PGPPublicKey.ELGAMAL_GENERAL
|| algorithm == PGPPublicKey.ECDSA;
}
/** Returns true if the algorithm is of a type which is suitable for encryption. */
static boolean isEncryptionAlgo(int algorithm) {
return algorithm == PGPPublicKey.RSA_GENERAL
|| algorithm == PGPPublicKey.RSA_ENCRYPT
|| algorithm == PGPPublicKey.ELGAMAL_ENCRYPT
|| algorithm == PGPPublicKey.ELGAMAL_GENERAL
|| algorithm == PGPPublicKey.ECDH;
}
}

View File

@ -18,9 +18,6 @@
package org.sufficientlysecure.keychain.pgp;
import org.spongycastle.asn1.ASN1ObjectIdentifier;
import org.spongycastle.asn1.nist.NISTNamedCurves;
import org.spongycastle.asn1.teletrust.TeleTrusTNamedCurves;
import org.spongycastle.bcpg.ECPublicBCPGKey;
import org.spongycastle.bcpg.SignatureSubpacketTags;
import org.spongycastle.bcpg.sig.KeyFlags;
@ -28,7 +25,6 @@ import org.spongycastle.openpgp.PGPPublicKey;
import org.spongycastle.openpgp.PGPSignature;
import org.spongycastle.openpgp.PGPSignatureSubpacketVector;
import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentVerifierBuilderProvider;
import org.spongycastle.util.Strings;
import org.sufficientlysecure.keychain.Constants;
import org.sufficientlysecure.keychain.util.IterableIterator;
import org.sufficientlysecure.keychain.util.Log;
@ -264,12 +260,6 @@ public class UncachedPublicKey {
return (getKeyUsage() & KeyFlags.CERTIFY_OTHER) != 0;
}
if (mPublicKey.getAlgorithm() == PGPPublicKey.RSA_GENERAL
|| mPublicKey.getAlgorithm() == PGPPublicKey.RSA_SIGN
|| mPublicKey.getAlgorithm() == PGPPublicKey.ECDSA) {
return true;
}
return false;
}
@ -279,9 +269,7 @@ public class UncachedPublicKey {
return (getKeyUsage() & KeyFlags.SIGN_DATA) != 0;
}
if (mPublicKey.getAlgorithm() == PGPPublicKey.RSA_GENERAL
|| mPublicKey.getAlgorithm() == PGPPublicKey.RSA_SIGN
|| mPublicKey.getAlgorithm() == PGPPublicKey.ECDSA) {
if (UncachedKeyRing.isSigningAlgo(mPublicKey.getAlgorithm())) {
return true;
}
@ -295,7 +283,7 @@ public class UncachedPublicKey {
}
// RSA_GENERAL, RSA_ENCRYPT, ELGAMAL_ENCRYPT, ELGAMAL_GENERAL, ECDH
if (mPublicKey.isEncryptionKey()) {
if (UncachedKeyRing.isEncryptionAlgo(mPublicKey.getAlgorithm())) {
return true;
}

View File

@ -341,6 +341,8 @@ public abstract class OperationResult implements Parcelable {
MSG_KC_SUB_REVOKE_BAD (LogLevel.WARN, R.string.msg_kc_sub_revoke_bad),
MSG_KC_SUB_REVOKE_DUP (LogLevel.DEBUG, R.string.msg_kc_sub_revoke_dup),
MSG_KC_SUB_UNKNOWN_ALGO (LogLevel.WARN, R.string.msg_kc_sub_unknown_algo),
MSG_KC_SUB_ALGO_BAD_ENCRYPT (LogLevel.WARN, R.string.msg_kc_sub_algo_bad_encrpyt),
MSG_KC_SUB_ALGO_BAD_SIGN (LogLevel.WARN, R.string.msg_kc_sub_algo_bad_sign),
MSG_KC_SUCCESS_BAD (LogLevel.OK, R.plurals.msg_kc_success_bad),
MSG_KC_SUCCESS_BAD_AND_RED (LogLevel.OK, R.string.msg_kc_success_bad_and_red),
MSG_KC_SUCCESS_REDUNDANT (LogLevel.OK, R.plurals.msg_kc_success_redundant),

View File

@ -684,6 +684,8 @@
<string name="msg_kc_sub_revoke_bad">"Removing bad subkey revocation certificate"</string>
<string name="msg_kc_sub_revoke_dup">"Removing redundant subkey revocation certificate"</string>
<string name="msg_kc_sub_unknown_algo">"Subkey uses an unknown algorithm, not importing…"</string>
<string name="msg_kc_sub_algo_bad_encrpyt">"Subkey has encryption usage flag, but algorithm is not suitable for encryption."</string>
<string name="msg_kc_sub_algo_bad_sign">"Subkey has signing usage flag, but algorithm is not suitable for signing."</string>
<string name="msg_kc_success">"Keyring canonicalization successful, no changes"</string>
<plurals name="msg_kc_success_bad">
<item quantity="one">"Keyring canonicalization successful, removed one erroneous certificate"</item>