diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/CipherAlgorithmInstance.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/CipherAlgorithmInstance.qll index d76265e1c70e..0e41b50300c8 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/CipherAlgorithmInstance.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/CipherAlgorithmInstance.qll @@ -104,11 +104,8 @@ class KnownOpenSSLCipherConstantAlgorithmInstance extends OpenSSLAlgorithmInstan override string getRawAlgorithmName() { result = this.(Literal).getValue().toString() } - override string getKeySizeFixed() { - exists(int keySize | - this.(KnownOpenSSLCipherAlgorithmConstant).getExplicitKeySize() = keySize and - result = keySize.toString() - ) + override int getKeySizeFixed() { + this.(KnownOpenSSLCipherAlgorithmConstant).getExplicitKeySize() = result } override Crypto::KeyOpAlg::Algorithm getAlgorithmType() { diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/EllipticCurveAlgorithmInstance.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/EllipticCurveAlgorithmInstance.qll index d80529dd1c63..574869ca29cd 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/EllipticCurveAlgorithmInstance.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/AlgorithmInstances/EllipticCurveAlgorithmInstance.qll @@ -35,8 +35,11 @@ class KnownOpenSSLEllipticCurveConstantAlgorithmInstance extends OpenSSLAlgorith override string getRawEllipticCurveName() { result = this.(Literal).getValue().toString() } override Crypto::TEllipticCurveType getEllipticCurveType() { - Crypto::ellipticCurveNameToKeySizeAndFamilyMapping(this.(KnownOpenSSLEllipticCurveAlgorithmConstant) - .getNormalizedName(), _, result) + Crypto::ellipticCurveNameToKeySizeAndFamilyMapping(this.getParsedEllipticCurveName(), _, result) + } + + override string getParsedEllipticCurveName() { + result = this.(KnownOpenSSLEllipticCurveAlgorithmConstant).getNormalizedName() } override int getKeySize() { diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/ECKeyGenOperation.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/ECKeyGenOperation.qll new file mode 100644 index 000000000000..9dc723bb5d11 --- /dev/null +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/ECKeyGenOperation.qll @@ -0,0 +1,65 @@ +private import experimental.quantum.Language +private import experimental.quantum.OpenSSL.LibraryDetector +private import experimental.quantum.OpenSSL.CtxFlow as CTXFlow +private import OpenSSLOperationBase +private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers +private import semmle.code.cpp.dataflow.new.DataFlow + +private module AlgGetterToAlgConsumerConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + exists(OpenSSLAlgorithmValueConsumer c | c.getResultNode() = source) + } + + predicate isSink(DataFlow::Node sink) { + exists(ECKeyGenOperation c | c.getAlgorithmArg() = sink.asExpr()) + } +} + +private module AlgGetterToAlgConsumerFlow = DataFlow::Global; + +class ECKeyGenOperation extends OpenSSLOperation, Crypto::KeyGenerationOperationInstance { + ECKeyGenOperation() { + this.(Call).getTarget().getName() = "EC_KEY_generate_key" and + isPossibleOpenSSLFunction(this.(Call).getTarget()) + } + + override Expr getOutputArg() { + result = this.(Call) // return value of call + } + + Expr getAlgorithmArg() { result = this.(Call).getArgument(0) } + + override Expr getInputArg() { + // there is no 'input', in the sense that no data is being manipulated by the operation. + // There is an input of an algorithm, but that is not the intention of the operation input arg. + none() + } + + override Crypto::KeyArtifactType getOutputKeyType() { result = Crypto::TAsymmetricKeyType() } + + override Crypto::ArtifactOutputDataFlowNode getOutputKeyArtifact() { + result = this.getOutputNode() + } + + override Crypto::AlgorithmValueConsumer getAnAlgorithmValueConsumer() { + AlgGetterToAlgConsumerFlow::flow(result.(OpenSSLAlgorithmValueConsumer).getResultNode(), + DataFlow::exprNode(this.getAlgorithmArg())) + } + + override Crypto::ConsumerInputDataFlowNode getKeySizeConsumer() { + none() // no explicit key size, inferred from algorithm + } + + override int getKeySizeFixed() { + none() + // TODO: marked as none as the operation itself has no key size, it + // comes from the algorithm source, but note we could grab the + // algorithm source and get the key size (see below). + // We may need to reconsider what is the best approach here. + // result = + // this.getAnAlgorithmValueConsumer() + // .getAKnownAlgorithmSource() + // .(Crypto::EllipticCurveInstance) + // .getKeySize() + } +} diff --git a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperations.qll b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperations.qll index 819e964878c3..f6ff0dd1f077 100644 --- a/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperations.qll +++ b/cpp/ql/lib/experimental/quantum/OpenSSL/Operations/OpenSSLOperations.qll @@ -1,3 +1,4 @@ import OpenSSLOperationBase import EVPCipherOperation import EVPHashOperation +import ECKeyGenOperation diff --git a/java/ql/lib/experimental/quantum/JCA.qll b/java/ql/lib/experimental/quantum/JCA.qll index 70c65ef581d6..8245abe13c40 100644 --- a/java/ql/lib/experimental/quantum/JCA.qll +++ b/java/ql/lib/experimental/quantum/JCA.qll @@ -353,7 +353,7 @@ module JCAModel { else result instanceof KeyOpAlg::TUnknownKeyOperationAlgorithmType } - override string getKeySizeFixed() { + override int getKeySizeFixed() { none() // TODO: implement to handle variants such as AES-128 } @@ -1104,7 +1104,7 @@ module JCAModel { KeyGeneratorFlowAnalysisImpl::getInitFromUse(this, _, _).getKeySizeArg() = result.asExpr() } - override string getKeySizeFixed() { none() } + override int getKeySizeFixed() { none() } } class KeyGeneratorCipherAlgorithm extends CipherStringLiteralAlgorithmInstance { @@ -1310,7 +1310,7 @@ module JCAModel { result.asExpr() = this.getKeySpecInstantiation().(PBEKeySpecInstantiation).getKeyLengthArg() } - override string getKeySizeFixed() { none() } + override int getKeySizeFixed() { none() } override string getOutputKeySizeFixed() { none() } diff --git a/shared/quantum/codeql/quantum/experimental/Model.qll b/shared/quantum/codeql/quantum/experimental/Model.qll index 8e1e6247484c..5370f72ef47b 100644 --- a/shared/quantum/codeql/quantum/experimental/Model.qll +++ b/shared/quantum/codeql/quantum/experimental/Model.qll @@ -841,7 +841,7 @@ module CryptographyBase Input> { * This will be automatically inferred and applied at the node level. * See `fixedImplicitCipherKeySize`. */ - abstract string getKeySizeFixed(); + abstract int getKeySizeFixed(); /** * Gets a consumer for the key size in bits specified for this algorithm variant. @@ -1044,7 +1044,7 @@ module CryptographyBase Input> { abstract KeyArtifactType getOutputKeyType(); // Defaults or fixed values - string getKeySizeFixed() { none() } + int getKeySizeFixed() { none() } // Consumer input nodes abstract ConsumerInputDataFlowNode getKeySizeConsumer(); @@ -1900,7 +1900,7 @@ module CryptographyBase Input> { or // [ONLY_KNOWN] key = "DefaultKeySize" and - value = kdfInstance.getKeySizeFixed() and + value = kdfInstance.getKeySizeFixed().toString() and location = this.getLocation() or // [ONLY_KNOWN] - TODO: refactor for known unknowns @@ -2259,13 +2259,10 @@ module CryptographyBase Input> { /** * Gets the key size variant of this algorithm in bits, e.g., 128 for "AES-128". */ - string getKeySizeFixed() { + int getKeySizeFixed() { result = instance.asAlg().getKeySizeFixed() or - exists(int size | - KeyOpAlg::fixedImplicitCipherKeySize(instance.asAlg().getAlgorithmType(), size) and - result = size.toString() - ) + KeyOpAlg::fixedImplicitCipherKeySize(instance.asAlg().getAlgorithmType(), result) } /** @@ -2333,7 +2330,7 @@ module CryptographyBase Input> { // [ONLY_KNOWN] key = "KeySize" and ( - value = this.getKeySizeFixed() and + value = this.getKeySizeFixed().toString() and location = this.getLocation() or node_as_property(this.getKeySize(), value, location)