From 0fa05d47e32f1c0297d54ecdf8331394ddb1454f Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Wed, 9 Nov 2022 09:28:30 -0500 Subject: [PATCH 1/9] add shared key sizes --- config/identical-files.json | 4 ++++ .../code/java/security/InsufficientKeySize.qll | 7 ++++--- .../java/security/internal/EncryptionKeySizes.qll | 15 +++++++++++++++ python/ql/lib/semmle/python/Concepts.qll | 7 ++++--- .../security/internal/EncryptionKeySizes.qll | 15 +++++++++++++++ 5 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 java/ql/lib/semmle/code/java/security/internal/EncryptionKeySizes.qll create mode 100644 python/ql/lib/semmle/python/security/internal/EncryptionKeySizes.qll diff --git a/config/identical-files.json b/config/identical-files.json index 0a5d037e226..26d1598ec53 100644 --- a/config/identical-files.json +++ b/config/identical-files.json @@ -580,5 +580,9 @@ "IncompleteMultiCharacterSanitization JS/Ruby": [ "javascript/ql/lib/semmle/javascript/security/IncompleteMultiCharacterSanitizationQuery.qll", "ruby/ql/lib/codeql/ruby/security/IncompleteMultiCharacterSanitizationQuery.qll" + ], + "EncryptionKeySizes Python/Java": [ + "python/ql/lib/semmle/python/security/internal/EncryptionKeySizes.qll", + "java/ql/lib/semmle/code/java/security/internal/EncryptionKeySizes.qll" ] } diff --git a/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll b/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll index e09bffca8e1..eff82641a95 100644 --- a/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll +++ b/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll @@ -2,6 +2,7 @@ private import semmle.code.java.security.Encryption private import semmle.code.java.dataflow.DataFlow +private import semmle.code.java.security.EncryptionKeySizes /** A source for an insufficient key size. */ abstract class InsufficientKeySizeSource extends DataFlow::Node { @@ -42,7 +43,7 @@ private module Asymmetric { } /** Returns the minimum recommended key size for RSA, DSA, and DH algorithms. */ - private int getMinKeySize() { result = 2048 } + private int getMinKeySize() { result = minSecureKeySizeAsymmetricNonEc() } /** An instance of an RSA, DSA, or DH algorithm specification. */ private class Spec extends ClassInstanceExpr { @@ -87,7 +88,7 @@ private module Asymmetric { } /** Returns the minimum recommended key size for elliptic curve (EC) algorithms. */ - private int getMinKeySize() { result = 256 } + private int getMinKeySize() { result = minSecureKeySizeAsymmetricEc() } /** Returns the key size from an EC algorithm's curve name string */ bindingset[algorithm] @@ -168,7 +169,7 @@ private module Symmetric { } /** Returns the minimum recommended key size for AES algorithms. */ - private int getMinKeySize() { result = 128 } + private int getMinKeySize() { result = minSecureKeySizeSymmetric() } /** A call to the `init` method declared in `javax.crypto.KeyGenerator`. */ private class KeyGenInit extends MethodAccess { diff --git a/java/ql/lib/semmle/code/java/security/internal/EncryptionKeySizes.qll b/java/ql/lib/semmle/code/java/security/internal/EncryptionKeySizes.qll new file mode 100644 index 00000000000..d3675f07523 --- /dev/null +++ b/java/ql/lib/semmle/code/java/security/internal/EncryptionKeySizes.qll @@ -0,0 +1,15 @@ +/** + * INTERNAL: Do not use. + * + * Provides predicates for recommended encryption key sizes. + * Such that we can share this logic across our CodeQL analysis of different languages. + */ + +/** Returns the minimum recommended key size for asymmetric algorithms (RSA, DSA, and DH). */ +int minSecureKeySizeAsymmetricNonEc() { result = 2048 } + +/** Returns the minimum recommended key size for elliptic curve (EC) algorithms. */ +int minSecureKeySizeAsymmetricEc() { result = 256 } + +/** Returns the minimum recommended key size for symmetric algorithmms (AES). */ +int minSecureKeySizeSymmetric() { result = 128 } diff --git a/python/ql/lib/semmle/python/Concepts.qll b/python/ql/lib/semmle/python/Concepts.qll index 861e4e7ca81..6864b63d3dd 100644 --- a/python/ql/lib/semmle/python/Concepts.qll +++ b/python/ql/lib/semmle/python/Concepts.qll @@ -9,6 +9,7 @@ private import semmle.python.dataflow.new.DataFlow private import semmle.python.dataflow.new.RemoteFlowSources private import semmle.python.dataflow.new.TaintTracking private import semmle.python.Frameworks +private import semmle.python.security.internal.EncryptionKeySizes /** * A data-flow node that executes an operating system command, @@ -1141,21 +1142,21 @@ module Cryptography { abstract class RsaRange extends Range { final override string getName() { result = "RSA" } - final override int minimumSecureKeySize() { result = 2048 } + final override int minimumSecureKeySize() { result = minSecureKeySizeAsymmetricNonEc() } } /** A data-flow node that generates a new DSA key-pair. */ abstract class DsaRange extends Range { final override string getName() { result = "DSA" } - final override int minimumSecureKeySize() { result = 2048 } + final override int minimumSecureKeySize() { result = minSecureKeySizeAsymmetricNonEc() } } /** A data-flow node that generates a new ECC key-pair. */ abstract class EccRange extends Range { final override string getName() { result = "ECC" } - final override int minimumSecureKeySize() { result = 224 } + final override int minimumSecureKeySize() { result = minSecureKeySizeAsymmetricEc() } } } } diff --git a/python/ql/lib/semmle/python/security/internal/EncryptionKeySizes.qll b/python/ql/lib/semmle/python/security/internal/EncryptionKeySizes.qll new file mode 100644 index 00000000000..d3675f07523 --- /dev/null +++ b/python/ql/lib/semmle/python/security/internal/EncryptionKeySizes.qll @@ -0,0 +1,15 @@ +/** + * INTERNAL: Do not use. + * + * Provides predicates for recommended encryption key sizes. + * Such that we can share this logic across our CodeQL analysis of different languages. + */ + +/** Returns the minimum recommended key size for asymmetric algorithms (RSA, DSA, and DH). */ +int minSecureKeySizeAsymmetricNonEc() { result = 2048 } + +/** Returns the minimum recommended key size for elliptic curve (EC) algorithms. */ +int minSecureKeySizeAsymmetricEc() { result = 256 } + +/** Returns the minimum recommended key size for symmetric algorithmms (AES). */ +int minSecureKeySizeSymmetric() { result = 128 } From 4d99cd1b7a6263d31043fe59763aebbd151e54dd Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Wed, 9 Nov 2022 18:14:46 -0500 Subject: [PATCH 2/9] update EC key size in help file --- python/ql/src/Security/CWE-326/WeakCryptoKey.qhelp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/python/ql/src/Security/CWE-326/WeakCryptoKey.qhelp b/python/ql/src/Security/CWE-326/WeakCryptoKey.qhelp index 8c6b578d12d..2bc138f86bb 100644 --- a/python/ql/src/Security/CWE-326/WeakCryptoKey.qhelp +++ b/python/ql/src/Security/CWE-326/WeakCryptoKey.qhelp @@ -11,13 +11,13 @@ As computational power increases, the ability to break ciphers grows and keys ne

The three main asymmetric key algorithms currently in use are Rivest–Shamir–Adleman (RSA) cryptography, Digital Signature Algorithm (DSA), and Elliptic-curve cryptography (ECC). With current technology, key sizes of 2048 bits for RSA and DSA, -or 224 bits for ECC, are regarded as unbreakable. +or 256 bits for ECC, are regarded as unbreakable.

-Increase the key size to the recommended amount or larger. For RSA or DSA this is at least 2048 bits, for ECC this is at least 224 bits. +Increase the key size to the recommended amount or larger. For RSA or DSA this is at least 2048 bits, for ECC this is at least 256 bits.

@@ -45,4 +45,3 @@ Recommendation for Transitioning the Use of Cryptographic Algorithms and Key Len - From c4dac31895082e9a7484f09f41863fbe6e7f7710 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Wed, 9 Nov 2022 18:51:35 -0500 Subject: [PATCH 3/9] fix typo in import statement --- java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll b/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll index eff82641a95..139faa6a01e 100644 --- a/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll +++ b/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll @@ -2,7 +2,7 @@ private import semmle.code.java.security.Encryption private import semmle.code.java.dataflow.DataFlow -private import semmle.code.java.security.EncryptionKeySizes +private import semmle.code.java.security.internal.EncryptionKeySizes /** A source for an insufficient key size. */ abstract class InsufficientKeySizeSource extends DataFlow::Node { From 25f0a13e156ebe132518c7ecedd12b815a5caeb9 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Wed, 9 Nov 2022 18:51:56 -0500 Subject: [PATCH 4/9] update python test cases --- .../Security/CWE-326-WeakCryptoKey/WeakCryptoKey.expected | 4 ++-- .../query-tests/Security/CWE-326-WeakCryptoKey/weak_crypto.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/python/ql/test/query-tests/Security/CWE-326-WeakCryptoKey/WeakCryptoKey.expected b/python/ql/test/query-tests/Security/CWE-326-WeakCryptoKey/WeakCryptoKey.expected index dc50ddd7873..ea1f924045b 100644 --- a/python/ql/test/query-tests/Security/CWE-326-WeakCryptoKey/WeakCryptoKey.expected +++ b/python/ql/test/query-tests/Security/CWE-326-WeakCryptoKey/WeakCryptoKey.expected @@ -1,8 +1,8 @@ | weak_crypto.py:68:1:68:21 | ControlFlowNode for dsa_gen_key() | Creation of an DSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:16:12:16:15 | ControlFlowNode for IntegerLiteral | 1024 | -| weak_crypto.py:69:1:69:19 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 224 and considered breakable. | weak_crypto.py:22:11:22:22 | ControlFlowNode for Attribute | 163 | +| weak_crypto.py:69:1:69:19 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 256 and considered breakable. | weak_crypto.py:22:11:22:22 | ControlFlowNode for Attribute | 224 | | weak_crypto.py:70:1:70:28 | ControlFlowNode for rsa_gen_key() | Creation of an RSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:12:12:12:15 | ControlFlowNode for IntegerLiteral | 1024 | | weak_crypto.py:72:1:72:30 | ControlFlowNode for dsa_gen_key() | Creation of an DSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:16:12:16:15 | ControlFlowNode for IntegerLiteral | 1024 | -| weak_crypto.py:73:1:73:25 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 224 and considered breakable. | weak_crypto.py:22:11:22:22 | ControlFlowNode for Attribute | 163 | +| weak_crypto.py:73:1:73:25 | ControlFlowNode for ec_gen_key() | Creation of an ECC key uses $@ bits, which is below 256 and considered breakable. | weak_crypto.py:22:11:22:22 | ControlFlowNode for Attribute | 224 | | weak_crypto.py:74:1:74:37 | ControlFlowNode for rsa_gen_key() | Creation of an RSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:12:12:12:15 | ControlFlowNode for IntegerLiteral | 1024 | | weak_crypto.py:76:1:76:22 | ControlFlowNode for Attribute() | Creation of an DSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:16:12:16:15 | ControlFlowNode for IntegerLiteral | 1024 | | weak_crypto.py:77:1:77:22 | ControlFlowNode for Attribute() | Creation of an RSA key uses $@ bits, which is below 2048 and considered breakable. | weak_crypto.py:12:12:12:15 | ControlFlowNode for IntegerLiteral | 1024 | diff --git a/python/ql/test/query-tests/Security/CWE-326-WeakCryptoKey/weak_crypto.py b/python/ql/test/query-tests/Security/CWE-326-WeakCryptoKey/weak_crypto.py index de533254cfe..5ec929c7d09 100644 --- a/python/ql/test/query-tests/Security/CWE-326-WeakCryptoKey/weak_crypto.py +++ b/python/ql/test/query-tests/Security/CWE-326-WeakCryptoKey/weak_crypto.py @@ -19,8 +19,8 @@ DSA_STRONG = 3076 BIG = 10000 -EC_WEAK = ec.SECT163K1() # has key size of 163 -EC_OK = ec.SECP224R1() +EC_WEAK = ec.SECP224R1() +EC_OK = ec.SECP256R1() EC_STRONG = ec.SECP384R1() EC_BIG = ec.SECT571R1() From 1f4bd009934d67ec3ed89e9b9cbc22e968b97cb1 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Wed, 16 Nov 2022 23:32:07 -0500 Subject: [PATCH 5/9] split rsa/dsa/dh --- .../java/security/InsufficientKeySize.qll | 142 ++++++++++++++---- .../security/internal/EncryptionKeySizes.qll | 18 ++- python/ql/lib/semmle/python/Concepts.qll | 6 +- .../security/internal/EncryptionKeySizes.qll | 18 ++- 4 files changed, 138 insertions(+), 46 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll b/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll index 139faa6a01e..1f293bb0590 100644 --- a/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll +++ b/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll @@ -20,41 +20,121 @@ abstract class InsufficientKeySizeSink extends DataFlow::Node { private module Asymmetric { /** Provides models for non-elliptic-curve asymmetric cryptography. */ private module NonEllipticCurve { - /** A source for an insufficient key size used in RSA, DSA, and DH algorithms. */ - private class Source extends InsufficientKeySizeSource { - Source() { this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize() } + private module Rsa { + /** A source for an insufficient key size used in an RSA algorithm. */ + private class Source extends InsufficientKeySizeSource { + Source() { this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize() } - override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() } - } - - /** A sink for an insufficient key size used in RSA, DSA, and DH algorithms. */ - private class Sink extends InsufficientKeySizeSink { - Sink() { - exists(KeyPairGenInit kpgInit, KeyPairGen kpg | - kpg.getAlgoName().matches(["RSA", "DSA", "DH"]) and - DataFlow::localExprFlow(kpg, kpgInit.getQualifier()) and - this.asExpr() = kpgInit.getKeySizeArg() - ) - or - exists(Spec spec | this.asExpr() = spec.getKeySizeArg()) + override predicate hasState(DataFlow::FlowState state) { + state = getMinKeySize().toString() + } } - override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() } - } + /** A sink for an insufficient key size used in an RSA algorithm. */ + private class Sink extends InsufficientKeySizeSink { + Sink() { + exists(KeyPairGenInit kpgInit, KeyPairGen kpg | + kpg.getAlgoName() = "RSA" and + DataFlow::localExprFlow(kpg, kpgInit.getQualifier()) and + this.asExpr() = kpgInit.getKeySizeArg() + ) + or + exists(Spec spec | this.asExpr() = spec.getKeySizeArg()) + } - /** Returns the minimum recommended key size for RSA, DSA, and DH algorithms. */ - private int getMinKeySize() { result = minSecureKeySizeAsymmetricNonEc() } - - /** An instance of an RSA, DSA, or DH algorithm specification. */ - private class Spec extends ClassInstanceExpr { - Spec() { - this.getConstructedType() instanceof RsaKeyGenParameterSpec or - this.getConstructedType() instanceof DsaGenParameterSpec or - this.getConstructedType() instanceof DhGenParameterSpec + override predicate hasState(DataFlow::FlowState state) { + state = getMinKeySize().toString() + } } - /** Gets the `keysize` argument of this instance. */ - Argument getKeySizeArg() { result = this.getArgument(0) } + /** Returns the minimum recommended key size for an RSA algorithm. */ + private int getMinKeySize() { result = minSecureKeySizeRsa() } + + /** An instance of an RSA algorithm specification. */ + private class Spec extends ClassInstanceExpr { + Spec() { this.getConstructedType() instanceof RsaKeyGenParameterSpec } + + /** Gets the `keysize` argument of this instance. */ + Argument getKeySizeArg() { result = this.getArgument(0) } + } + } + + private module Dsa { + /** A source for an insufficient key size used a DSA algorithm. */ + private class Source extends InsufficientKeySizeSource { + Source() { this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize() } + + override predicate hasState(DataFlow::FlowState state) { + state = getMinKeySize().toString() + } + } + + /** A sink for an insufficient key size used in a DSA algorithm. */ + private class Sink extends InsufficientKeySizeSink { + Sink() { + exists(KeyPairGenInit kpgInit, KeyPairGen kpg | + kpg.getAlgoName() = "DSA" and + DataFlow::localExprFlow(kpg, kpgInit.getQualifier()) and + this.asExpr() = kpgInit.getKeySizeArg() + ) + or + exists(Spec spec | this.asExpr() = spec.getKeySizeArg()) + } + + override predicate hasState(DataFlow::FlowState state) { + state = getMinKeySize().toString() + } + } + + /** Returns the minimum recommended key size for a DSA algorithm. */ + private int getMinKeySize() { result = minSecureKeySizeDsa() } + + /** An instance of a DSA algorithm specification. */ + private class Spec extends ClassInstanceExpr { + Spec() { this.getConstructedType() instanceof DsaGenParameterSpec } + + /** Gets the `keysize` argument of this instance. */ + Argument getKeySizeArg() { result = this.getArgument(0) } + } + } + + private module Dh { + /** A source for an insufficient key size used in a DH algorithm. */ + private class Source extends InsufficientKeySizeSource { + Source() { this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize() } + + override predicate hasState(DataFlow::FlowState state) { + state = getMinKeySize().toString() + } + } + + /** A sink for an insufficient key size used in a DH algorithm. */ + private class Sink extends InsufficientKeySizeSink { + Sink() { + exists(KeyPairGenInit kpgInit, KeyPairGen kpg | + kpg.getAlgoName() = "DH" and + DataFlow::localExprFlow(kpg, kpgInit.getQualifier()) and + this.asExpr() = kpgInit.getKeySizeArg() + ) + or + exists(Spec spec | this.asExpr() = spec.getKeySizeArg()) + } + + override predicate hasState(DataFlow::FlowState state) { + state = getMinKeySize().toString() + } + } + + /** Returns the minimum recommended key size for a DH algorithm. */ + private int getMinKeySize() { result = minSecureKeySizeDh() } + + /** An instance of an RSA, DSA, or DH algorithm specification. */ + private class Spec extends ClassInstanceExpr { + Spec() { this.getConstructedType() instanceof DhGenParameterSpec } + + /** Gets the `keysize` argument of this instance. */ + Argument getKeySizeArg() { result = this.getArgument(0) } + } } } @@ -88,7 +168,7 @@ private module Asymmetric { } /** Returns the minimum recommended key size for elliptic curve (EC) algorithms. */ - private int getMinKeySize() { result = minSecureKeySizeAsymmetricEc() } + private int getMinKeySize() { result = minSecureKeySizeEcc() } /** Returns the key size from an EC algorithm's curve name string */ bindingset[algorithm] @@ -169,7 +249,7 @@ private module Symmetric { } /** Returns the minimum recommended key size for AES algorithms. */ - private int getMinKeySize() { result = minSecureKeySizeSymmetric() } + private int getMinKeySize() { result = minSecureKeySizeAes() } /** A call to the `init` method declared in `javax.crypto.KeyGenerator`. */ private class KeyGenInit extends MethodAccess { diff --git a/java/ql/lib/semmle/code/java/security/internal/EncryptionKeySizes.qll b/java/ql/lib/semmle/code/java/security/internal/EncryptionKeySizes.qll index d3675f07523..46df3a3ca7b 100644 --- a/java/ql/lib/semmle/code/java/security/internal/EncryptionKeySizes.qll +++ b/java/ql/lib/semmle/code/java/security/internal/EncryptionKeySizes.qll @@ -5,11 +5,17 @@ * Such that we can share this logic across our CodeQL analysis of different languages. */ -/** Returns the minimum recommended key size for asymmetric algorithms (RSA, DSA, and DH). */ -int minSecureKeySizeAsymmetricNonEc() { result = 2048 } +/** Returns the minimum recommended key size for RSA. */ +int minSecureKeySizeRsa() { result = 2048 } -/** Returns the minimum recommended key size for elliptic curve (EC) algorithms. */ -int minSecureKeySizeAsymmetricEc() { result = 256 } +/** Returns the minimum recommended key size for DSA. */ +int minSecureKeySizeDsa() { result = 2048 } -/** Returns the minimum recommended key size for symmetric algorithmms (AES). */ -int minSecureKeySizeSymmetric() { result = 128 } +/** Returns the minimum recommended key size for DH. */ +int minSecureKeySizeDh() { result = 2048 } + +/** Returns the minimum recommended key size for elliptic curve cryptography. */ +int minSecureKeySizeEcc() { result = 256 } + +/** Returns the minimum recommended key size for AES. */ +int minSecureKeySizeAes() { result = 128 } diff --git a/python/ql/lib/semmle/python/Concepts.qll b/python/ql/lib/semmle/python/Concepts.qll index 6864b63d3dd..b4d2a64cb45 100644 --- a/python/ql/lib/semmle/python/Concepts.qll +++ b/python/ql/lib/semmle/python/Concepts.qll @@ -1142,21 +1142,21 @@ module Cryptography { abstract class RsaRange extends Range { final override string getName() { result = "RSA" } - final override int minimumSecureKeySize() { result = minSecureKeySizeAsymmetricNonEc() } + final override int minimumSecureKeySize() { result = minSecureKeySizeRsa() } } /** A data-flow node that generates a new DSA key-pair. */ abstract class DsaRange extends Range { final override string getName() { result = "DSA" } - final override int minimumSecureKeySize() { result = minSecureKeySizeAsymmetricNonEc() } + final override int minimumSecureKeySize() { result = minSecureKeySizeDsa() } } /** A data-flow node that generates a new ECC key-pair. */ abstract class EccRange extends Range { final override string getName() { result = "ECC" } - final override int minimumSecureKeySize() { result = minSecureKeySizeAsymmetricEc() } + final override int minimumSecureKeySize() { result = minSecureKeySizeEcc() } } } } diff --git a/python/ql/lib/semmle/python/security/internal/EncryptionKeySizes.qll b/python/ql/lib/semmle/python/security/internal/EncryptionKeySizes.qll index d3675f07523..46df3a3ca7b 100644 --- a/python/ql/lib/semmle/python/security/internal/EncryptionKeySizes.qll +++ b/python/ql/lib/semmle/python/security/internal/EncryptionKeySizes.qll @@ -5,11 +5,17 @@ * Such that we can share this logic across our CodeQL analysis of different languages. */ -/** Returns the minimum recommended key size for asymmetric algorithms (RSA, DSA, and DH). */ -int minSecureKeySizeAsymmetricNonEc() { result = 2048 } +/** Returns the minimum recommended key size for RSA. */ +int minSecureKeySizeRsa() { result = 2048 } -/** Returns the minimum recommended key size for elliptic curve (EC) algorithms. */ -int minSecureKeySizeAsymmetricEc() { result = 256 } +/** Returns the minimum recommended key size for DSA. */ +int minSecureKeySizeDsa() { result = 2048 } -/** Returns the minimum recommended key size for symmetric algorithmms (AES). */ -int minSecureKeySizeSymmetric() { result = 128 } +/** Returns the minimum recommended key size for DH. */ +int minSecureKeySizeDh() { result = 2048 } + +/** Returns the minimum recommended key size for elliptic curve cryptography. */ +int minSecureKeySizeEcc() { result = 256 } + +/** Returns the minimum recommended key size for AES. */ +int minSecureKeySizeAes() { result = 128 } From f7ae4e894f48c21516931b0773545a16cc351727 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Mon, 28 Nov 2022 10:47:32 -0500 Subject: [PATCH 6/9] apply rasmus' approach --- .../java/security/InsufficientKeySize.qll | 148 ++++++------------ 1 file changed, 48 insertions(+), 100 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll b/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll index 1f293bb0590..3c811db1f34 100644 --- a/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll +++ b/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll @@ -20,121 +20,69 @@ abstract class InsufficientKeySizeSink extends DataFlow::Node { private module Asymmetric { /** Provides models for non-elliptic-curve asymmetric cryptography. */ private module NonEllipticCurve { - private module Rsa { - /** A source for an insufficient key size used in an RSA algorithm. */ - private class Source extends InsufficientKeySizeSource { - Source() { this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize() } + /** A source for an insufficient key size used in an RSA, DSA, and DH algorithms. */ + private class Source extends InsufficientKeySizeSource { + string algoName; - override predicate hasState(DataFlow::FlowState state) { - state = getMinKeySize().toString() - } - } + Source() { this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize(algoName) } - /** A sink for an insufficient key size used in an RSA algorithm. */ - private class Sink extends InsufficientKeySizeSink { - Sink() { - exists(KeyPairGenInit kpgInit, KeyPairGen kpg | - kpg.getAlgoName() = "RSA" and - DataFlow::localExprFlow(kpg, kpgInit.getQualifier()) and - this.asExpr() = kpgInit.getKeySizeArg() - ) - or - exists(Spec spec | this.asExpr() = spec.getKeySizeArg()) - } - - override predicate hasState(DataFlow::FlowState state) { - state = getMinKeySize().toString() - } - } - - /** Returns the minimum recommended key size for an RSA algorithm. */ - private int getMinKeySize() { result = minSecureKeySizeRsa() } - - /** An instance of an RSA algorithm specification. */ - private class Spec extends ClassInstanceExpr { - Spec() { this.getConstructedType() instanceof RsaKeyGenParameterSpec } - - /** Gets the `keysize` argument of this instance. */ - Argument getKeySizeArg() { result = this.getArgument(0) } + override predicate hasState(DataFlow::FlowState state) { + state = getMinKeySize(algoName).toString() } } - private module Dsa { - /** A source for an insufficient key size used a DSA algorithm. */ - private class Source extends InsufficientKeySizeSource { - Source() { this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize() } + /** A sink for an insufficient key size used in an RSA, DSA, and DH algorithms. */ + private class Sink extends InsufficientKeySizeSink { + string algoName; - override predicate hasState(DataFlow::FlowState state) { - state = getMinKeySize().toString() - } + Sink() { + exists(KeyPairGenInit kpgInit, KeyPairGen kpg | + algoName in ["RSA", "DSA", "DH"] and + kpg.getAlgoName().matches(algoName) and + DataFlow::localExprFlow(kpg, kpgInit.getQualifier()) and + this.asExpr() = kpgInit.getKeySizeArg() + ) + or + exists(Spec spec | this.asExpr() = spec.getKeySizeArg() and algoName = spec.getAlgoName()) } - /** A sink for an insufficient key size used in a DSA algorithm. */ - private class Sink extends InsufficientKeySizeSink { - Sink() { - exists(KeyPairGenInit kpgInit, KeyPairGen kpg | - kpg.getAlgoName() = "DSA" and - DataFlow::localExprFlow(kpg, kpgInit.getQualifier()) and - this.asExpr() = kpgInit.getKeySizeArg() - ) - or - exists(Spec spec | this.asExpr() = spec.getKeySizeArg()) - } - - override predicate hasState(DataFlow::FlowState state) { - state = getMinKeySize().toString() - } - } - - /** Returns the minimum recommended key size for a DSA algorithm. */ - private int getMinKeySize() { result = minSecureKeySizeDsa() } - - /** An instance of a DSA algorithm specification. */ - private class Spec extends ClassInstanceExpr { - Spec() { this.getConstructedType() instanceof DsaGenParameterSpec } - - /** Gets the `keysize` argument of this instance. */ - Argument getKeySizeArg() { result = this.getArgument(0) } + override predicate hasState(DataFlow::FlowState state) { + state = getMinKeySize(algoName).toString() } } - private module Dh { - /** A source for an insufficient key size used in a DH algorithm. */ - private class Source extends InsufficientKeySizeSource { - Source() { this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize() } + /** Returns the minimum recommended key size for RSA, DSA, and DH algorithms. */ + private int getMinKeySize(string algoName) { + algoName = "RSA" and + result = minSecureKeySizeRsa() + or + algoName = "DSA" and + result = minSecureKeySizeDsa() + or + algoName = "DH" and + result = minSecureKeySizeDh() + } - override predicate hasState(DataFlow::FlowState state) { - state = getMinKeySize().toString() - } + /** An instance of an RSA, DSA, or DH algorithm specification. */ + private class Spec extends ClassInstanceExpr { + string algoName; + + Spec() { + this.getConstructedType() instanceof RsaKeyGenParameterSpec and + algoName = "RSA" + or + this.getConstructedType() instanceof DsaGenParameterSpec and + algoName = "DSA" + or + this.getConstructedType() instanceof DhGenParameterSpec and + algoName = "DH" } - /** A sink for an insufficient key size used in a DH algorithm. */ - private class Sink extends InsufficientKeySizeSink { - Sink() { - exists(KeyPairGenInit kpgInit, KeyPairGen kpg | - kpg.getAlgoName() = "DH" and - DataFlow::localExprFlow(kpg, kpgInit.getQualifier()) and - this.asExpr() = kpgInit.getKeySizeArg() - ) - or - exists(Spec spec | this.asExpr() = spec.getKeySizeArg()) - } + /** Gets the `keysize` argument of this instance. */ + Argument getKeySizeArg() { result = this.getArgument(0) } - override predicate hasState(DataFlow::FlowState state) { - state = getMinKeySize().toString() - } - } - - /** Returns the minimum recommended key size for a DH algorithm. */ - private int getMinKeySize() { result = minSecureKeySizeDh() } - - /** An instance of an RSA, DSA, or DH algorithm specification. */ - private class Spec extends ClassInstanceExpr { - Spec() { this.getConstructedType() instanceof DhGenParameterSpec } - - /** Gets the `keysize` argument of this instance. */ - Argument getKeySizeArg() { result = this.getArgument(0) } - } + /** Gets the algorithm name of this spec. */ + string getAlgoName() { result = algoName } } } From 548ff47f03e4441c3754d97531a1b642935fe129 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Mon, 28 Nov 2022 11:18:29 -0500 Subject: [PATCH 7/9] fix typo in QLDoc --- java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll b/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll index 3c811db1f34..e559437307f 100644 --- a/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll +++ b/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll @@ -20,7 +20,7 @@ abstract class InsufficientKeySizeSink extends DataFlow::Node { private module Asymmetric { /** Provides models for non-elliptic-curve asymmetric cryptography. */ private module NonEllipticCurve { - /** A source for an insufficient key size used in an RSA, DSA, and DH algorithms. */ + /** A source for an insufficient key size used in RSA, DSA, and DH algorithms. */ private class Source extends InsufficientKeySizeSource { string algoName; @@ -31,7 +31,7 @@ private module Asymmetric { } } - /** A sink for an insufficient key size used in an RSA, DSA, and DH algorithms. */ + /** A sink for an insufficient key size used in RSA, DSA, and DH algorithms. */ private class Sink extends InsufficientKeySizeSink { string algoName; From 315ceb57e9d983dcaf0fa95952d3a367b1dff2ed Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Tue, 29 Nov 2022 17:28:08 +0100 Subject: [PATCH 8/9] Python: Add change-note --- .../ql/src/change-notes/2022-11-29-ecc-min-secure-keysize.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 python/ql/src/change-notes/2022-11-29-ecc-min-secure-keysize.md diff --git a/python/ql/src/change-notes/2022-11-29-ecc-min-secure-keysize.md b/python/ql/src/change-notes/2022-11-29-ecc-min-secure-keysize.md new file mode 100644 index 00000000000..2fc0532d547 --- /dev/null +++ b/python/ql/src/change-notes/2022-11-29-ecc-min-secure-keysize.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Bumped the minimum keysize we consider secure for elliptic curve cryptography from 224 to 256 bits, following current best practices. This might effect results from the _Use of weak cryptographic key_ (`py/weak-crypto-key`) query. From f54480b7c87304fc3af9f8b645aacc6ebb99e8a7 Mon Sep 17 00:00:00 2001 From: Jami <57204504+jcogs33@users.noreply.github.com> Date: Wed, 30 Nov 2022 16:14:50 -0500 Subject: [PATCH 9/9] change matches to equality Co-authored-by: Rasmus Wriedt Larsen --- java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll b/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll index e559437307f..b156b594a70 100644 --- a/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll +++ b/java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll @@ -38,7 +38,7 @@ private module Asymmetric { Sink() { exists(KeyPairGenInit kpgInit, KeyPairGen kpg | algoName in ["RSA", "DSA", "DH"] and - kpg.getAlgoName().matches(algoName) and + kpg.getAlgoName() = algoName and DataFlow::localExprFlow(kpg, kpgInit.getQualifier()) and this.asExpr() = kpgInit.getKeySizeArg() )