Skip to content

Quantum: Refactor OpenSSL padding modeling #19908

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ module KnownOpenSslAlgorithmToAlgorithmValueConsumerFlow =
DataFlow::Global<KnownOpenSslAlgorithmToAlgorithmValueConsumerConfig>;

module RsaPaddingAlgorithmToPaddingAlgorithmValueConsumerConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source.asExpr() instanceof OpenSslPaddingLiteral }
predicate isSource(DataFlow::Node source) {
source.asExpr() instanceof OpenSslSpecialPaddingLiteral
}

predicate isSink(DataFlow::Node sink) {
exists(PaddingAlgorithmValueConsumer c | c.getInputNode() = sink)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@ import cpp
private import experimental.quantum.Language
private import KnownAlgorithmConstants
private import Crypto::KeyOpAlg as KeyOpAlg
private import OpenSSLAlgorithmInstanceBase
private import PaddingAlgorithmInstance
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumerBase
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.DirectAlgorithmValueConsumer
private import experimental.quantum.OpenSSL.Operations.OpenSSLOperationBase
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.OpenSSLAlgorithmValueConsumers
private import OpenSSLAlgorithmInstances
private import AlgToAVCFlow
private import BlockAlgorithmInstance

/**
* Given a `KnownOpenSslCipherAlgorithmExpr`, converts this to a cipher family type.
Expand Down Expand Up @@ -97,10 +95,13 @@ class KnownOpenSslCipherConstantAlgorithmInstance extends OpenSslAlgorithmInstan
}

override Crypto::PaddingAlgorithmInstance getPaddingAlgorithm() {
//TODO: the padding is either self, or it flows through getter ctx to a set padding call
// like EVP_PKEY_CTX_set_rsa_padding
result = this
// TODO or trace through getter ctx to set padding
or
exists(OperationStep s |
this.getAvc().(AvcContextCreationStep).flowsToOperationStep(s) and
s.getAlgorithmValueConsumerForInput(PaddingAlgorithmIO()) =
result.(OpenSslAlgorithmInstance).getAvc()
)
}

override string getRawAlgorithmName() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import cpp
private import experimental.quantum.Language
private import OpenSSLAlgorithmInstanceBase
private import experimental.quantum.OpenSSL.Operations.OpenSSLOperationBase
private import experimental.quantum.OpenSSL.AlgorithmInstances.KnownAlgorithmConstants
private import AlgToAVCFlow
private import experimental.quantum.OpenSSL.AlgorithmValueConsumers.DirectAlgorithmValueConsumer
Expand All @@ -18,13 +19,14 @@ private import codeql.quantum.experimental.Standardization::Types::KeyOpAlg as K
* # define RSA_PKCS1_WITH_TLS_PADDING 7
* # define RSA_PKCS1_NO_IMPLICIT_REJECT_PADDING 8
*/
class OpenSslPaddingLiteral extends Literal {
class OpenSslSpecialPaddingLiteral extends Literal {
// TODO: we can be more specific about where the literal is in a larger expression
// to avoid literals that are clealy not representing an algorithm, e.g., array indices.
OpenSslPaddingLiteral() { this.getValue().toInt() in [0, 1, 3, 4, 5, 6, 7, 8] }
OpenSslSpecialPaddingLiteral() { this.getValue().toInt() in [0, 1, 3, 4, 5, 6, 7, 8] }
}

/**
* Holds if `e` has the given `type`.
* Given a `KnownOpenSslPaddingAlgorithmExpr`, converts this to a padding family type.
* Does not bind if there is no mapping (no mapping to 'unknown' or 'other').
*/
Expand All @@ -45,9 +47,6 @@ predicate knownOpenSslConstantToPaddingFamilyType(
)
}

//abstract class OpenSslPaddingAlgorithmInstance extends OpenSslAlgorithmInstance, Crypto::PaddingAlgorithmInstance{}
// TODO: need to alter this to include known padding constants which don't have the
// same mechanics as those with known nids
class KnownOpenSslPaddingConstantAlgorithmInstance extends OpenSslAlgorithmInstance,
Crypto::PaddingAlgorithmInstance instanceof Expr
{
Expand Down Expand Up @@ -79,7 +78,7 @@ class KnownOpenSslPaddingConstantAlgorithmInstance extends OpenSslAlgorithmInsta
isPaddingSpecificConsumer = false
or
// Possibility 3: padding-specific literal
this instanceof OpenSslPaddingLiteral and
this instanceof OpenSslSpecialPaddingLiteral and
exists(DataFlow::Node src, DataFlow::Node sink |
// Sink is an argument to a CipherGetterCall
sink = getterCall.getInputNode() and
Expand Down Expand Up @@ -124,44 +123,6 @@ class KnownOpenSslPaddingConstantAlgorithmInstance extends OpenSslAlgorithmInsta
}
}

// // Values used for EVP_PKEY_CTX_set_rsa_padding, these are
// // not the same as 'typical' constants found in the set of known algorithm constants
// // they do not have an NID
// // TODO: what about setting the padding directly?
// class KnownRSAPaddingConstant extends OpenSslPaddingAlgorithmInstance, Crypto::PaddingAlgorithmInstance instanceof Literal
// {
// KnownRSAPaddingConstant() {
// // from rsa.h in openssl:
// // # define RSA_PKCS1_PADDING 1
// // # define RSA_NO_PADDING 3
// // # define RSA_PKCS1_OAEP_PADDING 4
// // # define RSA_X931_PADDING 5
// // /* EVP_PKEY_ only */
// // # define RSA_PKCS1_PSS_PADDING 6
// // # define RSA_PKCS1_WITH_TLS_PADDING 7
// // /* internal RSA_ only */
// // # define RSA_PKCS1_NO_IMPLICIT_REJECT_PADDING 8
// this instanceof Literal and
// this.getValue().toInt() in [0, 1, 3, 4, 5, 6, 7, 8]
// // TODO: trace to padding-specific consumers
// RsaPaddingAlgorithmToPaddingAlgorithmValueConsumerFlow
// }
// override string getRawPaddingAlgorithmName() { result = this.(Literal).getValue().toString() }
// override Crypto::TPaddingType getPaddingType() {
// if this.(Literal).getValue().toInt() in [1, 6, 7, 8]
// then result = Crypto::PKCS1_v1_5()
// else
// if this.(Literal).getValue().toInt() = 3
// then result = Crypto::NoPadding()
// else
// if this.(Literal).getValue().toInt() = 4
// then result = Crypto::OAEP()
// else
// if this.(Literal).getValue().toInt() = 5
// then result = Crypto::ANSI_X9_23()
// else result = Crypto::OtherPadding()
// }
// }
class OaepPaddingAlgorithmInstance extends Crypto::OaepPaddingAlgorithmInstance,
KnownOpenSslPaddingConstantAlgorithmInstance
{
Expand All @@ -170,10 +131,18 @@ class OaepPaddingAlgorithmInstance extends Crypto::OaepPaddingAlgorithmInstance,
}

override Crypto::HashAlgorithmInstance getOaepEncodingHashAlgorithm() {
none() //TODO
exists(OperationStep s |
this.getAvc().(AvcContextCreationStep).flowsToOperationStep(s) and
s.getAlgorithmValueConsumerForInput(HashAlgorithmOaepIO()) =
result.(OpenSslAlgorithmInstance).getAvc()
)
}

override Crypto::HashAlgorithmInstance getMgf1HashAlgorithm() {
none() //TODO
exists(OperationStep s |
this.getAvc().(AvcContextCreationStep).flowsToOperationStep(s) and
s.getAlgorithmValueConsumerForInput(HashAlgorithmMgf1IO()) =
result.(OpenSslAlgorithmInstance).getAvc()
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,14 @@ class EvpCipherFinalCall extends EvpCipherOperationFinalStep {
*/
class EvpPKeyCipherOperation extends EvpCipherOperationFinalStep {
EvpPKeyCipherOperation() {
this.getTarget().getName() in ["EVP_PKEY_encrypt", "EVP_PKEY_decrypt"]
this.getTarget().getName() in ["EVP_PKEY_encrypt", "EVP_PKEY_decrypt"] and
// TODO: for now ignore this operation entirely if it is setting the cipher text to null
// this needs to be re-evalauted if this scenario sets other values worth tracking
(
exists(this.(Call).getArgument(1).getValue())
implies
this.(Call).getArgument(1).getValue().toInt() != 0
)
}

override DataFlow::Node getInput(IOType type) {
Expand All @@ -228,9 +235,24 @@ class EvpPKeyCipherOperation extends EvpCipherOperationFinalStep {
override DataFlow::Node getOutput(IOType type) {
super.getOutput(type) = result
or
result.asExpr() = this.getArgument(1) and type = CiphertextIO()
result.asExpr() = this.getArgument(1) and
type = CiphertextIO() and
this.getStepType() = FinalStep()
// TODO: could indicate text lengths here, as well
}

override OperationStepType getStepType() {
// When the output buffer is null, the step is not a final step
// it is used to get the buffer size, if 0 consider it an initialization step
// NOTE/TODO: not tracing 0 to the arg, just looking for 0 directly in param
// the assumption is this is the common case, but we may want to make this more
// robust and support a dataflow.
result = FinalStep() and
(exists(super.getArgument(1).getValue()) implies super.getArgument(1).getValue().toInt() != 0)
or
result = InitializerStep() and
super.getArgument(1).getValue().toInt() = 0
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,19 +71,42 @@ class EvpCtxSetEcParamgenCurveNidInitializer extends OperationStep {
* - `EVP_PKEY_CTX_set_ecdh_kdf_md`
*/
class EvpCtxSetHashInitializer extends OperationStep {
boolean isOaep;
boolean isMgf1;

EvpCtxSetHashInitializer() {
this.getTarget().getName() in [
"EVP_PKEY_CTX_set_signature_md", "EVP_PKEY_CTX_set_rsa_mgf1_md_name",
"EVP_PKEY_CTX_set_rsa_mgf1_md", "EVP_PKEY_CTX_set_rsa_oaep_md_name",
"EVP_PKEY_CTX_set_rsa_oaep_md", "EVP_PKEY_CTX_set_dsa_paramgen_md",
"EVP_PKEY_CTX_set_signature_md", "EVP_PKEY_CTX_set_dsa_paramgen_md",
"EVP_PKEY_CTX_set_dh_kdf_md", "EVP_PKEY_CTX_set_ecdh_kdf_md"
]
] and
isOaep = false and
isMgf1 = false
or
this.getTarget().getName() in [
"EVP_PKEY_CTX_set_rsa_mgf1_md_name", "EVP_PKEY_CTX_set_rsa_mgf1_md"
] and
isOaep = false and
isMgf1 = true
or
this.getTarget().getName() in [
"EVP_PKEY_CTX_set_rsa_oaep_md_name",
"EVP_PKEY_CTX_set_rsa_oaep_md"
] and
isOaep = true and
isMgf1 = false
}

override DataFlow::Node getInput(IOType type) {
result.asExpr() = this.getArgument(0) and type = ContextIO()
or
result.asExpr() = this.getArgument(1) and type = HashAlgorithmIO()
result.asExpr() = this.getArgument(1) and
type = HashAlgorithmIO() and
isOaep = false and
isMgf1 = false
or
result.asExpr() = this.getArgument(1) and type = HashAlgorithmOaepIO() and isOaep = true
or
result.asExpr() = this.getArgument(1) and type = HashAlgorithmMgf1IO() and isMgf1 = true
}

override DataFlow::Node getOutput(IOType type) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,11 @@ newtype TIOType =
// For OSSL_PARAM and OSSL_LIB_CTX use of OsslParamIO and OsslLibContextIO
ContextIO() or
DigestIO() or
// For OAEP and MGF1 hashes, there is a special IO type for these hashes
// it is recommended to set the most explicit type known, not both
HashAlgorithmIO() or
HashAlgorithmOaepIO() or
HashAlgorithmMgf1IO() or
IVorNonceIO() or
KeyIO() or
KeyOperationSubtypeIO() or
Expand Down Expand Up @@ -254,18 +258,18 @@ abstract class OperationStep extends Call {
* operation step (dominating operation step, see `getDominatingInitializersToStep`).
*/
Crypto::AlgorithmValueConsumer getPrimaryAlgorithmValueConsumer() {
exists(DataFlow::Node src, DataFlow::Node sink, IOType t, OperationStep avcSucc |
exists(DataFlow::Node src, DataFlow::Node sink, IOType t, OperationStep avcConsumingPred |
(t = PrimaryAlgorithmIO() or t = ContextIO()) and
avcSucc.flowsToOperationStep(this) and
avcConsumingPred.flowsToOperationStep(this) and
src.asExpr() = result and
sink = avcSucc.getInput(t) and
sink = avcConsumingPred.getInput(t) and
AvcToOperationStepFlow::flow(src, sink) and
(
// Case 1: the avcSucc step is a dominating initialization step
// Case 1: the avcConsumingPred step is a dominating initialization step
t = PrimaryAlgorithmIO() and
avcSucc = this.getDominatingInitializersToStep(PrimaryAlgorithmIO())
avcConsumingPred = this.getDominatingInitializersToStep(PrimaryAlgorithmIO())
or
// Case 2: the succ is a context input (any avcSucc is valid)
// Case 2: the pred is a context input
t = ContextIO()
)
)
Expand All @@ -277,6 +281,8 @@ abstract class OperationStep extends Call {
* TODO: generalize to use this for `getPrimaryAlgorithmValueConsumer`
*/
Crypto::AlgorithmValueConsumer getAlgorithmValueConsumerForInput(IOType type) {
result = this and this.setsValue(type)
or
exists(DataFlow::Node src, DataFlow::Node sink |
AvcToOperationStepFlow::flow(src, sink) and
src.asExpr() = result and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,20 @@
| openssl_basic.c:167:9:167:27 | SignOperation | Input | openssl_basic.c:163:35:163:41 | Message |
| openssl_basic.c:167:9:167:27 | SignOperation | Key | openssl_basic.c:160:59:160:62 | Key |
| openssl_basic.c:167:9:167:27 | SignOperation | Output | openssl_basic.c:167:34:167:36 | SignatureOutput |
| openssl_basic.c:235:51:235:55 | KeyOperationAlgorithm | Mode | openssl_basic.c:235:51:235:55 | KeyOperationAlgorithm |
| openssl_basic.c:235:51:235:55 | KeyOperationAlgorithm | Padding | openssl_basic.c:249:51:249:72 | PaddingAlgorithm |
| openssl_basic.c:238:9:238:25 | KeyGeneration | Algorithm | openssl_basic.c:235:51:235:55 | KeyOperationAlgorithm |
| openssl_basic.c:238:9:238:25 | KeyGeneration | Output | openssl_basic.c:238:39:238:43 | Key |
| openssl_basic.c:238:39:238:43 | Key | Algorithm | openssl_basic.c:235:51:235:55 | KeyOperationAlgorithm |
| openssl_basic.c:243:52:243:55 | Key | Source | openssl_basic.c:238:39:238:43 | Key |
| openssl_basic.c:249:51:249:72 | PaddingAlgorithm | MD | openssl_basic.c:250:51:250:60 | HashAlgorithm |
| openssl_basic.c:249:51:249:72 | PaddingAlgorithm | MGF1Hash | openssl_basic.c:251:51:251:60 | HashAlgorithm |
| openssl_basic.c:262:24:262:39 | EncryptOperation | Algorithm | openssl_basic.c:235:51:235:55 | KeyOperationAlgorithm |
| openssl_basic.c:262:24:262:39 | EncryptOperation | Input | openssl_basic.c:263:64:263:70 | Message |
| openssl_basic.c:262:24:262:39 | EncryptOperation | Key | openssl_basic.c:243:52:243:55 | Key |
| openssl_basic.c:262:24:262:39 | EncryptOperation | Nonce | openssl_basic.c:262:24:262:39 | EncryptOperation |
| openssl_basic.c:262:24:262:39 | EncryptOperation | Output | openssl_basic.c:262:54:262:63 | KeyOperationOutput |
| openssl_basic.c:263:64:263:70 | Message | Source | openssl_basic.c:231:27:231:49 | Constant |
| openssl_pkey.c:21:10:21:28 | KeyGeneration | Algorithm | openssl_pkey.c:21:10:21:28 | KeyGeneration |
| openssl_pkey.c:21:10:21:28 | KeyGeneration | Output | openssl_pkey.c:21:30:21:32 | Key |
| openssl_pkey.c:21:10:21:28 | KeyOperationAlgorithm | Mode | openssl_pkey.c:21:10:21:28 | KeyOperationAlgorithm |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,21 @@
| openssl_basic.c:180:42:180:59 | Constant | Description | 0123456789012345 | openssl_basic.c:180:42:180:59 | openssl_basic.c:180:42:180:59 |
| openssl_basic.c:181:49:181:87 | Constant | Description | This is a test message for encryption | openssl_basic.c:181:49:181:87 | openssl_basic.c:181:49:181:87 |
| openssl_basic.c:218:32:218:33 | Constant | Description | 32 | openssl_basic.c:218:32:218:33 | openssl_basic.c:218:32:218:33 |
| openssl_basic.c:231:27:231:49 | Constant | Description | Encrypt me with OAEP! | openssl_basic.c:231:27:231:49 | openssl_basic.c:231:27:231:49 |
| openssl_basic.c:235:51:235:55 | KeyOperationAlgorithm | Name | RSA | openssl_basic.c:235:51:235:55 | openssl_basic.c:235:51:235:55 |
| openssl_basic.c:235:51:235:55 | KeyOperationAlgorithm | RawName | RSA | openssl_basic.c:235:51:235:55 | openssl_basic.c:235:51:235:55 |
| openssl_basic.c:237:54:237:57 | Constant | Description | 2048 | openssl_basic.c:237:54:237:57 | openssl_basic.c:237:54:237:57 |
| openssl_basic.c:238:39:238:43 | Key | KeyType | Asymmetric | openssl_basic.c:238:39:238:43 | openssl_basic.c:238:39:238:43 |
| openssl_basic.c:243:52:243:55 | Key | KeyType | Unknown | openssl_basic.c:243:52:243:55 | openssl_basic.c:243:52:243:55 |
| openssl_basic.c:249:51:249:72 | PaddingAlgorithm | Name | OAEP | openssl_basic.c:249:51:249:72 | openssl_basic.c:249:51:249:72 |
| openssl_basic.c:249:51:249:72 | PaddingAlgorithm | RawName | 4 | openssl_basic.c:249:51:249:72 | openssl_basic.c:249:51:249:72 |
| openssl_basic.c:250:51:250:60 | HashAlgorithm | DigestSize | 256 | openssl_basic.c:250:51:250:60 | openssl_basic.c:250:51:250:60 |
| openssl_basic.c:250:51:250:60 | HashAlgorithm | Name | SHA2 | openssl_basic.c:250:51:250:60 | openssl_basic.c:250:51:250:60 |
| openssl_basic.c:250:51:250:60 | HashAlgorithm | RawName | EVP_sha256 | openssl_basic.c:250:51:250:60 | openssl_basic.c:250:51:250:60 |
| openssl_basic.c:251:51:251:60 | HashAlgorithm | DigestSize | 256 | openssl_basic.c:251:51:251:60 | openssl_basic.c:251:51:251:60 |
| openssl_basic.c:251:51:251:60 | HashAlgorithm | Name | SHA2 | openssl_basic.c:251:51:251:60 | openssl_basic.c:251:51:251:60 |
| openssl_basic.c:251:51:251:60 | HashAlgorithm | RawName | EVP_sha256 | openssl_basic.c:251:51:251:60 | openssl_basic.c:251:51:251:60 |
| openssl_basic.c:262:24:262:39 | EncryptOperation | KeyOperationSubtype | Encrypt | openssl_basic.c:262:24:262:39 | openssl_basic.c:262:24:262:39 |
| openssl_pkey.c:21:10:21:28 | KeyOperationAlgorithm | Name | RSA | openssl_pkey.c:21:10:21:28 | openssl_pkey.c:21:10:21:28 |
| openssl_pkey.c:21:10:21:28 | KeyOperationAlgorithm | RawName | RSA_generate_key_ex | openssl_pkey.c:21:10:21:28 | openssl_pkey.c:21:10:21:28 |
| openssl_pkey.c:21:30:21:32 | Key | KeyType | Asymmetric | openssl_pkey.c:21:30:21:32 | openssl_pkey.c:21:30:21:32 |
Expand Down
12 changes: 12 additions & 0 deletions cpp/ql/test/experimental/library-tests/quantum/nodes.expected
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@
| openssl_basic.c:180:42:180:59 | Constant |
| openssl_basic.c:181:49:181:87 | Constant |
| openssl_basic.c:218:32:218:33 | Constant |
| openssl_basic.c:231:27:231:49 | Constant |
| openssl_basic.c:235:51:235:55 | KeyOperationAlgorithm |
| openssl_basic.c:237:54:237:57 | Constant |
| openssl_basic.c:238:9:238:25 | KeyGeneration |
| openssl_basic.c:238:39:238:43 | Key |
| openssl_basic.c:243:52:243:55 | Key |
| openssl_basic.c:249:51:249:72 | PaddingAlgorithm |
| openssl_basic.c:250:51:250:60 | HashAlgorithm |
| openssl_basic.c:251:51:251:60 | HashAlgorithm |
| openssl_basic.c:262:24:262:39 | EncryptOperation |
| openssl_basic.c:262:54:262:63 | KeyOperationOutput |
| openssl_basic.c:263:64:263:70 | Message |
| openssl_pkey.c:21:10:21:28 | KeyGeneration |
| openssl_pkey.c:21:10:21:28 | KeyOperationAlgorithm |
| openssl_pkey.c:21:30:21:32 | Key |
Expand Down
Loading