diff --git a/java/ql/lib/experimental/Quantum/JCA.qll b/java/ql/lib/experimental/Quantum/JCA.qll index 9892b445312a..59aadd3652b3 100644 --- a/java/ql/lib/experimental/Quantum/JCA.qll +++ b/java/ql/lib/experimental/Quantum/JCA.qll @@ -532,4 +532,209 @@ module JCAModel { this.getMethod().getParameterType(2).hasName("AlgorithmParameterSpec") } } + + /** + * Key Material Concept + * any class that implements `java.security.spec.KeySpec` + */ + class KeyMaterialObject extends Class { + KeyMaterialObject() { + exists(RefType t | + this.extendsOrImplements*(t) and + t.hasQualifiedName("java.security.spec", "KeySpec") + ) + } + } + + /** + * KeyMaterial + * ie some plain material that gets used to generate a Key + */ + class KeyMaterialInstantiation extends Crypto::KeyMaterialInstance instanceof ClassInstanceExpr { + KeyMaterialInstantiation() { + this.(ClassInstanceExpr).getConstructedType() instanceof KeyMaterialObject + } + + override DataFlow::Node asOutputData() { result.asExpr() = this } + + override DataFlow::Node getInput() { result.asExpr() = this.(ClassInstanceExpr).getArgument(0) } + } + + abstract class JCAKeyGenerationCall extends Call { } + + private class JCAKeyGenerationCallWithMaterialArg extends JCAKeyGenerationCall { + JCAKeyGenerationCallWithMaterialArg() { + exists(string s | s in ["generatePrivate", "generatePublic", "translateKey"] | + this.getCallee().hasQualifiedName("java.security", "KeyFactory", s) + ) + } + + DataFlow::Node getInputData() { result.asExpr() = this.getArgument(0) } + } + + private class JCAKeyGenerationCallWithoutMaterialArg extends JCAKeyGenerationCall { + JCAKeyGenerationCallWithoutMaterialArg() { + this.getCallee().hasQualifiedName("javax.crypto", "KeyGenerator", "generateKey") + } + + DataFlow::Node getInputData() { result.asExpr() = this.getArgument(0) } + } + + abstract class KeyGenerators extends Class { + Method getInstance() { result = this.getAMethod() and result.hasName("getInstance") } + + abstract Method keySizeSetter(); + } + + class KeyGenerator extends KeyGenerators { + KeyGenerator() { this.hasQualifiedName("javax.crypto", "KeyGenerator") } + + override Method keySizeSetter() { result = this.getAMethod() and result.hasName("init") } + } + + class KeyFactory extends KeyGenerators { + KeyFactory() { this.hasQualifiedName("java.security", "KeyFactory") } + + //todo this requires flow from a keyspec + override Method keySizeSetter() { none() } + } + + class KeyPairGenerator extends KeyGenerators { + KeyPairGenerator() { this.hasQualifiedName("java.security", "KeyPairGenerator") } + + override Method keySizeSetter() { result = this.getAMethod() and result.hasName("initialize") } + } + + /** + * Data-flow configuration modelling flow from a string literal to a `KeyGeneratorsGetInstanceCall` argument. + */ + private module KeyGenAlgorithmStringToFetchFlow implements DataFlow::ConfigSig { + //TODO narrow this type based on JCA standard names doc for whats possible here, like is done for the cipher algs + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof StringLiteral } + + predicate isSink(DataFlow::Node sink) { + exists(KeyGenerators gen, Call c | + c.getCallee() = gen.getInstance() and + c.getArgument(0) = sink.asExpr() + ) + } + } + + module KeyGenAlgorithmStringToFetchFlowIns = DataFlow::Global; + + /** + * The key generation algorithm argument to a `KeyGeneratorsGetInstanceCall `. + * + * For example, in `KeyGenerator.getInstance(algorithm)`, this class represents `algorithm`. + * + * the algorithm is both what the resulting key is meant to be used for but also informs how it + * was created + */ + class KeyDerivationAlgorithmArg extends Crypto::KeyDerivationAlgorithmInstance instanceof Expr { + Call getInstanceCall; + + KeyDerivationAlgorithmArg() { + exists(KeyGenerators gen | + getInstanceCall.getCallee() = gen.getInstance() and this = getInstanceCall.getArgument(0) + ) + } + + /** + * Returns the `StringLiteral` from which this argument is derived, if known. + */ + StringLiteral getOrigin() { + KeyGenAlgorithmStringToFetchFlowIns::flow(DataFlow::exprNode(result), + DataFlow::exprNode(this.(Expr).getAChildExpr*())) + } + + Call getCall() { result = getInstanceCall } + } + + /** + * A data-flow configuration to track flow from a integer value to + * the keysize argument of the various `init` methods of the KeyGeneratorsGetInstanceCall types + */ + private module IntToKeyGeneratorInitConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof IntegerLiteral } + + predicate isSink(DataFlow::Node sink) { + exists(KeyGenerators c, Call call | + call.getCallee() = c.keySizeSetter() and call.getArgument(0) = sink.asExpr() + ) + } + } + + module IntToKeyGeneratorInitConfigFlow = DataFlow::Global; + + /** + * Data-flow configuration modelling flow from a keygenerator type getinstance call + * to generation step + */ + module KeyDerivationOperationInstanceConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { + exists(Call getInstanceCall, KeyGenerators kg | + kg.getInstance() = getInstanceCall.getCallee() and + src.asExpr() = getInstanceCall + ) + } + + predicate isSink(DataFlow::Node sink) { + exists(JCAKeyGenerationCall call | sink.asExpr() = call.getQualifier()) + } + + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(Call getInstanceCall, KeyGenerators kg, Call keySizeSetterCall | + kg.getInstance() = getInstanceCall.getCallee() and + node1.asExpr() = getInstanceCall.getQualifier() and + kg.keySizeSetter() = keySizeSetterCall.getCallee() and + node2.asExpr() = keySizeSetterCall.getQualifier() + ) + } + } + + module KeyDerivationOperationInstanceConfigFlow = + DataFlow::Global; + + class KeyDerivationOperationInstance extends Crypto::KeyDerivationOperationInstance instanceof Call + { + Crypto::KeyDerivationAlgorithmInstance algorithm; + JCAKeyGenerationCall generateCall; + + KeyDerivationOperationInstance() { + exists(KeyDerivationAlgorithmArg arg, DataFlow::Node source, DataFlow::Node sink | + this = generateCall and + generateCall.getQualifier() = sink.asExpr() and + KeyDerivationOperationInstanceConfigFlow::flow(source, sink) and + algorithm = arg and + source.asExpr() = arg.getCall() + ) + } + + //TODO fill these out using various flows defined above + override Crypto::KeyDerivationAlgorithmInstance getAlgorithm() { none() } + + override Crypto::KeyMaterialInstance getInputKeyMaterial() { none() } + + override Crypto::KeyArtifactInstance getOutputKey() { none() } + } + + /** + * A Key Object + */ + class KeyInstantiation extends Crypto::KeyArtifactInstance instanceof Expr { + KeyInstantiation() { + exists(RefType t, Variable v | + this = v.getInitializer() and + v.getType().(RefType).extendsOrImplements*(t) and + t.hasQualifiedName("java.security", "Key") + ) + } + + override DataFlow::Node asOutputData() { result.asExpr() = this } + + //TODO fix by using the KeyDerivationOperationInstance type in this type + override DataFlow::Node getInput() { result.asExpr() = this.(ClassInstanceExpr).getArgument(0) } + + override DataFlow::Node getKeySize() { none() } + } } diff --git a/java/ql/src/experimental/Quantum/ConstantPassword.ql b/java/ql/src/experimental/Quantum/ConstantPassword.ql new file mode 100644 index 000000000000..4d895ac8c0f8 --- /dev/null +++ b/java/ql/src/experimental/Quantum/ConstantPassword.ql @@ -0,0 +1,38 @@ +/** + * @name Constant password + * @description Using constant passwords is not secure, because potential attackers can easily recover them from the source code. + * @kind problem + * @problem.severity error + * @security-severity 6.8 + * @precision high + * @id java/constant-password-new-model + * @tags security + * external/cwe/cwe-259 + */ + +//this query is a replica of the concept in: https://github.com/github/codeql/blob/main/swift/ql/src/queries/Security/CWE-259/ConstantPassword.ql +//but uses the **NEW MODELLING** +import experimental.Quantum.Language +import semmle.code.java.dataflow.TaintTracking + +/** + * A constant password is created through either a byte array or string literals. + */ +class ConstantPasswordSource extends Expr { + ConstantPasswordSource() { + this instanceof CharacterLiteral or + this instanceof StringLiteral + } +} + +module ConstantToKeyDerivationFlow implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof ConstantPasswordSource } + + predicate isSink(DataFlow::Node sink) { any(Crypto::KeyMaterialInstance i).getInput() = sink } +} + +module ConstantToKeyDerivationFlowInit = TaintTracking::Global; + +from DataFlow::Node source, DataFlow::Node sink +where ConstantToKeyDerivationFlowInit::flow(source, sink) +select sink, "Constant password $@ is used.", source, source.toString() diff --git a/shared/cryptography/codeql/cryptography/Model.qll b/shared/cryptography/codeql/cryptography/Model.qll index e5b91e198eaf..15ae51632306 100644 --- a/shared/cryptography/codeql/cryptography/Model.qll +++ b/shared/cryptography/codeql/cryptography/Model.qll @@ -82,7 +82,13 @@ module CryptographyBase Input> { abstract class HashAlgorithmInstance extends LocatableElement { } - abstract class KeyDerivationOperationInstance extends LocatableElement { } + abstract class KeyDerivationOperationInstance extends LocatableElement { + abstract KeyDerivationAlgorithmInstance getAlgorithm(); + + abstract KeyMaterialInstance getInputKeyMaterial(); + + abstract KeyArtifactInstance getOutputKey(); + } abstract class KeyDerivationAlgorithmInstance extends LocatableElement { } @@ -118,7 +124,11 @@ module CryptographyBase Input> { abstract class DigestArtifactInstance extends ArtifactLocatableElement { } - abstract class KeyArtifactInstance extends ArtifactLocatableElement { } + abstract class KeyMaterialInstance extends ArtifactLocatableElement { } + + abstract class KeyArtifactInstance extends ArtifactLocatableElement { + abstract DataFlowNode getKeySize(); + } abstract class NonceArtifactInstance extends ArtifactLocatableElement { } @@ -130,6 +140,7 @@ module CryptographyBase Input> { // Artifacts (data that is not an operation or algorithm, e.g., a key) TDigest(DigestArtifactInstance e) or TKey(KeyArtifactInstance e) or + TKeyMaterial(KeyMaterialInstance e) or TNonce(NonceArtifactInstance e) or TRandomNumberGeneration(RandomNumberGenerationInstance e) or // Operations (e.g., hashing, encryption) @@ -407,16 +418,6 @@ module CryptographyBase Input> { } } - /** - * An operation that derives one or more keys from an input value. - */ - abstract class KeyDerivationOperation extends Operation, TKeyDerivationOperation { - final override Location getLocation() { - exists(LocatableElement le | this = TKeyDerivationOperation(le) and result = le.getLocation()) - } - //override string getOperationType() { result = "KeyDerivationOperation" } - } - /** * An algorithm that derives one or more keys from an input value. * @@ -425,6 +426,8 @@ module CryptographyBase Input> { * For known algorithms, use the specialized classes, e.g., `HKDF` and `PKCS12KDF`. */ abstract class KeyDerivationAlgorithm extends Algorithm, TKeyDerivationAlgorithm { + final LocatableElement getInstance() { this = TKeyDerivationAlgorithm(result) } + final override Location getLocation() { exists(LocatableElement le | this = TKeyDerivationAlgorithm(le) and result = le.getLocation()) } @@ -1000,4 +1003,62 @@ module CryptographyBase Input> { abstract class KEMAlgorithm extends TKeyEncapsulationAlgorithm, Algorithm { final override string getAlgorithmType() { result = "KeyEncapsulationAlgorithm" } } + + /** + * A Key Material Object + */ + private class KeyMaterialImpl extends Artifact, TKeyMaterial { + KeyMaterialInstance instance; + + KeyMaterialImpl() { this = TKeyMaterial(instance) } + + final override string getInternalType() { result = "KeyMaterial" } + + override Location getLocation() { result = instance.getLocation() } + + override DataFlowNode asOutputData() { result = instance.asOutputData() } + + override DataFlowNode getInputData() { result = instance.getInput() } + } + + final class KeyMaterial = KeyMaterialImpl; + + /** + * A Key Object + */ + private class KeyArtifactImpl extends Artifact, TKey { + KeyArtifactInstance instance; + + KeyArtifactImpl() { this = TKey(instance) } + + final override string getInternalType() { result = "KeyMaterial" } + + override Location getLocation() { result = instance.getLocation() } + + override DataFlowNode asOutputData() { result = instance.asOutputData() } + + override DataFlowNode getInputData() { result = instance.getInput() } + } + + final class KeyArtifact = KeyArtifactImpl; + + /** + * A Key Derivation operation + * todo: decide if derivation or generation is the right term here for the most general concept + */ + private class KeyDerivationOperationImpl extends Operation, TKeyDerivationOperation { + KeyDerivationOperationInstance instance; + + KeyDerivationOperationImpl() { this = TKeyDerivationOperation(instance) } + + final override string getInternalType() { result = "KeyMaterial" } + + override Location getLocation() { result = instance.getLocation() } + + override KeyDerivationAlgorithm getAlgorithm() { + result.getInstance() = instance.getAlgorithm() + } + } + + final class KeyDerivationOperation = KeyDerivationOperationImpl; }