From f8b5a820f41c7b0b32bb5dca05543cd77ec83348 Mon Sep 17 00:00:00 2001 From: Rasmus Lerchedahl Petersen Date: Mon, 6 Mar 2023 14:31:17 +0100 Subject: [PATCH 1/6] python: revert change in expected behaviour --- .../InsecureProtocol.expected | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/python/ql/test/query-tests/Security/CWE-327-InsecureProtocol/InsecureProtocol.expected b/python/ql/test/query-tests/Security/CWE-327-InsecureProtocol/InsecureProtocol.expected index d6bc954de82..c08e19e5f1c 100644 --- a/python/ql/test/query-tests/Security/CWE-327-InsecureProtocol/InsecureProtocol.expected +++ b/python/ql/test/query-tests/Security/CWE-327-InsecureProtocol/InsecureProtocol.expected @@ -12,25 +12,28 @@ | InsecureProtocol.py:24:1:24:35 | ControlFlowNode for SSLContext() | Insecure SSL/TLS protocol version SSLv2 specified by $@. | InsecureProtocol.py:24:1:24:35 | ControlFlowNode for SSLContext() | call to SSLContext | | pyOpenSSL_fluent.py:8:27:8:33 | ControlFlowNode for context | Insecure SSL/TLS protocol version SSLv2 allowed by $@. | pyOpenSSL_fluent.py:6:15:6:44 | ControlFlowNode for Attribute() | call to SSL.Context | | pyOpenSSL_fluent.py:8:27:8:33 | ControlFlowNode for context | Insecure SSL/TLS protocol version SSLv3 allowed by $@. | pyOpenSSL_fluent.py:6:15:6:44 | ControlFlowNode for Attribute() | call to SSL.Context | +| pyOpenSSL_fluent.py:8:27:8:33 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1 allowed by $@. | pyOpenSSL_fluent.py:6:15:6:44 | ControlFlowNode for Attribute() | call to SSL.Context | +| pyOpenSSL_fluent.py:8:27:8:33 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | pyOpenSSL_fluent.py:6:15:6:44 | ControlFlowNode for Attribute() | call to SSL.Context | | pyOpenSSL_fluent.py:18:27:18:33 | ControlFlowNode for context | Insecure SSL/TLS protocol version SSLv2 allowed by $@. | pyOpenSSL_fluent.py:15:15:15:44 | ControlFlowNode for Attribute() | call to SSL.Context | | pyOpenSSL_fluent.py:18:27:18:33 | ControlFlowNode for context | Insecure SSL/TLS protocol version SSLv3 allowed by $@. | pyOpenSSL_fluent.py:15:15:15:44 | ControlFlowNode for Attribute() | call to SSL.Context | +| pyOpenSSL_fluent.py:18:27:18:33 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | pyOpenSSL_fluent.py:15:15:15:44 | ControlFlowNode for Attribute() | call to SSL.Context | | ssl_fluent.py:9:14:9:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1 allowed by $@. | ssl_fluent.py:6:15:6:46 | ControlFlowNode for Attribute() | call to ssl.SSLContext | | ssl_fluent.py:9:14:9:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:6:15:6:46 | ControlFlowNode for Attribute() | call to ssl.SSLContext | | ssl_fluent.py:19:14:19:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:15:15:15:46 | ControlFlowNode for Attribute() | call to ssl.SSLContext | | ssl_fluent.py:28:14:28:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:24:15:24:53 | ControlFlowNode for Attribute() | call to ssl.SSLContext | | ssl_fluent.py:37:14:37:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:33:15:33:53 | ControlFlowNode for Attribute() | call to ssl.SSLContext | -| ssl_fluent.py:57:14:57:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version SSLv2 allowed by $@. | ssl_fluent.py:54:15:54:49 | ControlFlowNode for Attribute() | call to ssl.SSLContext | -| ssl_fluent.py:57:14:57:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version SSLv3 allowed by $@. | ssl_fluent.py:54:15:54:49 | ControlFlowNode for Attribute() | call to ssl.SSLContext | +| ssl_fluent.py:57:14:57:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1 allowed by $@. | ssl_fluent.py:54:15:54:49 | ControlFlowNode for Attribute() | call to ssl.SSLContext | +| ssl_fluent.py:57:14:57:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:54:15:54:49 | ControlFlowNode for Attribute() | call to ssl.SSLContext | | ssl_fluent.py:71:14:71:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1 allowed by $@. | ssl_fluent.py:62:12:62:43 | ControlFlowNode for Attribute() | call to ssl.SSLContext | | ssl_fluent.py:71:14:71:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1 allowed by $@. | ssl_fluent.py:101:15:101:46 | ControlFlowNode for Attribute() | call to ssl.SSLContext | | ssl_fluent.py:71:14:71:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:62:12:62:43 | ControlFlowNode for Attribute() | call to ssl.SSLContext | -| ssl_fluent.py:71:14:71:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:65:15:65:46 | ControlFlowNode for Attribute() | call to ssl.SSLContext | | ssl_fluent.py:71:14:71:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:101:15:101:46 | ControlFlowNode for Attribute() | call to ssl.SSLContext | -| ssl_fluent.py:71:14:71:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:115:15:115:46 | ControlFlowNode for Attribute() | call to ssl.SSLContext | +| ssl_fluent.py:71:14:71:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:117:5:117:11 | ControlFlowNode for context | context modification | +| ssl_fluent.py:71:14:71:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:135:5:135:11 | ControlFlowNode for context | context modification | | ssl_fluent.py:77:14:77:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1 allowed by $@. | ssl_fluent.py:62:12:62:43 | ControlFlowNode for Attribute() | call to ssl.SSLContext | | ssl_fluent.py:77:14:77:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:62:12:62:43 | ControlFlowNode for Attribute() | call to ssl.SSLContext | -| ssl_fluent.py:97:14:97:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:65:15:65:46 | ControlFlowNode for Attribute() | call to ssl.SSLContext | -| ssl_fluent.py:146:14:146:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:142:15:142:46 | ControlFlowNode for Attribute() | call to ssl.SSLContext | -| ssl_fluent.py:165:14:165:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version SSLv3 allowed by $@. | ssl_fluent.py:161:15:161:65 | ControlFlowNode for Attribute() | call to ssl.create_default_context | +| ssl_fluent.py:97:14:97:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:95:5:95:11 | ControlFlowNode for context | context modification | +| ssl_fluent.py:146:14:146:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:143:5:143:11 | ControlFlowNode for context | context modification | +| ssl_fluent.py:165:14:165:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version SSLv3 allowed by $@. | ssl_fluent.py:162:5:162:11 | ControlFlowNode for context | context modification | | ssl_fluent.py:165:14:165:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1 allowed by $@. | ssl_fluent.py:161:15:161:65 | ControlFlowNode for Attribute() | call to ssl.create_default_context | | ssl_fluent.py:165:14:165:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:161:15:161:65 | ControlFlowNode for Attribute() | call to ssl.create_default_context | From 8160f742a59297752236183678d9863724fb42c0 Mon Sep 17 00:00:00 2001 From: Rasmus Lerchedahl Petersen Date: Mon, 6 Mar 2023 19:47:53 +0100 Subject: [PATCH 2/6] Python: small clean-up - no need for th 2-suffix - context creations are no longer unrestrictions --- python/ql/src/Security/CWE-327/FluentApiModel.qll | 4 ++-- python/ql/src/Security/CWE-327/PyOpenSSL.qll | 4 +--- python/ql/src/Security/CWE-327/Ssl.qll | 4 ---- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/python/ql/src/Security/CWE-327/FluentApiModel.qll b/python/ql/src/Security/CWE-327/FluentApiModel.qll index 3f479bc1627..2e6f9ffd398 100644 --- a/python/ql/src/Security/CWE-327/FluentApiModel.qll +++ b/python/ql/src/Security/CWE-327/FluentApiModel.qll @@ -20,7 +20,7 @@ import TlsLibraryModel * Since we really want "the last unrestriction, not nullified by a restriction", * we also disallow flow into restrictions. */ -module InsecureContextConfiguration2 implements DataFlow::StateConfigSig { +module InsecureContextConfiguration implements DataFlow::StateConfigSig { private newtype TFlowState = TMkFlowState(TlsLibrary library, int bits) { bits in [0 .. max(any(ProtocolVersion v).getBit()) * 2 - 1] @@ -112,7 +112,7 @@ module InsecureContextConfiguration2 implements DataFlow::StateConfigSig { } } -private module InsecureContextFlow = DataFlow::MakeWithState; +private module InsecureContextFlow = DataFlow::MakeWithState; /** * Holds if `conectionCreation` marks the creation of a connection based on the contex diff --git a/python/ql/src/Security/CWE-327/PyOpenSSL.qll b/python/ql/src/Security/CWE-327/PyOpenSSL.qll index 534f7c474e5..a156323fc2e 100644 --- a/python/ql/src/Security/CWE-327/PyOpenSSL.qll +++ b/python/ql/src/Security/CWE-327/PyOpenSSL.qll @@ -79,7 +79,5 @@ class PyOpenSsl extends TlsLibrary { override ProtocolRestriction protocol_restriction() { result instanceof SetOptionsCall } - override ProtocolUnrestriction protocol_unrestriction() { - result instanceof UnspecificPyOpenSslContextCreation - } + override ProtocolUnrestriction protocol_unrestriction() { none() } } diff --git a/python/ql/src/Security/CWE-327/Ssl.qll b/python/ql/src/Security/CWE-327/Ssl.qll index 0f2ce77acc7..58a4721a3ea 100644 --- a/python/ql/src/Security/CWE-327/Ssl.qll +++ b/python/ql/src/Security/CWE-327/Ssl.qll @@ -217,9 +217,5 @@ class Ssl extends TlsLibrary { result instanceof OptionsAugAndNot or result instanceof ContextSetVersion - or - result instanceof UnspecificSslContextCreation - or - result instanceof UnspecificSslDefaultContextCreation } } From 072df5dbc09185f151634a4c99726f2030525e06 Mon Sep 17 00:00:00 2001 From: Rasmus Lerchedahl Petersen Date: Tue, 7 Mar 2023 14:41:13 +0100 Subject: [PATCH 3/6] python: remove protocol family this concept was due to my confusion between TLS and SSL23, but they are aliases. We might want to bring back the concept if we model DTLS. Also, model what exactly creations allow, bring this back from the unrestrictions they used to be. We accept the changes regarding sources being reported differently. --- .../src/Security/CWE-327/FluentApiModel.qll | 11 +++- python/ql/src/Security/CWE-327/PyOpenSSL.qll | 31 ++++++---- python/ql/src/Security/CWE-327/Ssl.qll | 55 ++++++++--------- .../src/Security/CWE-327/TlsLibraryModel.qll | 60 +++++++++---------- .../InsecureProtocol.expected | 10 ++-- 5 files changed, 85 insertions(+), 82 deletions(-) diff --git a/python/ql/src/Security/CWE-327/FluentApiModel.qll b/python/ql/src/Security/CWE-327/FluentApiModel.qll index 2e6f9ffd398..3d63cab940e 100644 --- a/python/ql/src/Security/CWE-327/FluentApiModel.qll +++ b/python/ql/src/Security/CWE-327/FluentApiModel.qll @@ -61,9 +61,14 @@ module InsecureContextConfiguration implements DataFlow::StateConfigSig { } predicate isSource(DataFlow::Node source, FlowState state) { - exists(ProtocolFamily family | - source = state.getLibrary().unspecific_context_creation(family) and - state.getBits() = family.getBits() + exists(ContextCreation creation | source = creation | + creation = state.getLibrary().unspecific_context_creation() and + state.getBits() = + sum(ProtocolVersion version | + version = creation.getProtocol() and version.isInsecure() + | + version.getBit() + ) ) } diff --git a/python/ql/src/Security/CWE-327/PyOpenSSL.qll b/python/ql/src/Security/CWE-327/PyOpenSSL.qll index a156323fc2e..9b685ecfd1c 100644 --- a/python/ql/src/Security/CWE-327/PyOpenSSL.qll +++ b/python/ql/src/Security/CWE-327/PyOpenSSL.qll @@ -12,14 +12,18 @@ class PyOpenSslContextCreation extends ContextCreation, DataFlow::CallCfgNode { this = API::moduleImport("OpenSSL").getMember("SSL").getMember("Context").getACall() } - override string getProtocol() { + override ProtocolVersion getProtocol() { exists(DataFlow::Node protocolArg, PyOpenSsl pyo | protocolArg in [this.getArg(0), this.getArgByName("method")] | - protocolArg in [ - pyo.specific_version(result).getAValueReachableFromSource(), - pyo.unspecific_version(result).getAValueReachableFromSource() - ] + protocolArg = pyo.specific_version(result).getAValueReachableFromSource() + or + protocolArg = pyo.unspecific_version().getAValueReachableFromSource() and + // PyOpenSSL also allows DTLS + // see https://www.pyopenssl.org/en/stable/api/ssl.html#OpenSSL.SSL.Context + // although they are not mentioned here: + // https://www.pyopenssl.org/en/stable/api/ssl.html#OpenSSL.SSL.TLS_METHOD + result = any(ProtocolVersion pv) ) } } @@ -51,18 +55,21 @@ class SetOptionsCall extends ProtocolRestriction, DataFlow::CallCfgNode { } } -class UnspecificPyOpenSslContextCreation extends PyOpenSslContextCreation, UnspecificContextCreation { - // UnspecificPyOpenSslContextCreation() { library instanceof PyOpenSsl } -} - +// class UnspecificPyOpenSslContextCreation extends PyOpenSslContextCreation, UnspecificContextCreation { +// override ProtocolVersion getProtocol() { result = PyOpenSslContextCreation.super.getProtocol() } +// } class PyOpenSsl extends TlsLibrary { PyOpenSsl() { this = "pyOpenSSL" } override string specific_version_name(ProtocolVersion version) { result = version + "_METHOD" } - override string unspecific_version_name(ProtocolFamily family) { - // `"TLS_METHOD"` is not actually available in pyOpenSSL yet, but should be coming soon.. - result = family + "_METHOD" + override string unspecific_version_name() { + // See + // - https://www.pyopenssl.org/en/23.0.0/api/ssl.html#module-OpenSSL.SSL + // - https://www.openssl.org/docs/manmaster/man3/DTLS_server_method.html#NOTES + result = ["TLS", "SSLv23"] + "_METHOD" + or + result = "TLS_" + ["CLIENT", "SERVER"] + "_METHOD" } override API::Node version_constants() { result = API::moduleImport("OpenSSL").getMember("SSL") } diff --git a/python/ql/src/Security/CWE-327/Ssl.qll b/python/ql/src/Security/CWE-327/Ssl.qll index 58a4721a3ea..c3fd0366436 100644 --- a/python/ql/src/Security/CWE-327/Ssl.qll +++ b/python/ql/src/Security/CWE-327/Ssl.qll @@ -10,20 +10,21 @@ import TlsLibraryModel class SslContextCreation extends ContextCreation, DataFlow::CallCfgNode { SslContextCreation() { this = API::moduleImport("ssl").getMember("SSLContext").getACall() } - override string getProtocol() { + override ProtocolVersion getProtocol() { exists(DataFlow::Node protocolArg, Ssl ssl | protocolArg in [this.getArg(0), this.getArgByName("protocol")] | - protocolArg = - [ - ssl.specific_version(result).getAValueReachableFromSource(), - ssl.unspecific_version(result).getAValueReachableFromSource() - ] + protocolArg = ssl.specific_version(result).getAValueReachableFromSource() + or + protocolArg = ssl.unspecific_version().getAValueReachableFromSource() and + // see https://docs.python.org/3/library/ssl.html#id7 + result in ["TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"] ) or not exists(this.getArg(_)) and not exists(this.getArgByName(_)) and - result = "TLS" + // see https://docs.python.org/3/library/ssl.html#id7 + result in ["TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"] } } @@ -34,7 +35,7 @@ class SslDefaultContextCreation extends ContextCreation { // Allowed insecure versions are "TLSv1" and "TLSv1_1" // see https://docs.python.org/3/library/ssl.html#context-creation - override string getProtocol() { result = "TLS" } + override ProtocolVersion getProtocol() { result in ["TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"] } } /** Gets a reference to an `ssl.Context` instance. */ @@ -161,33 +162,29 @@ class ContextSetVersion extends ProtocolRestriction, ProtocolUnrestriction, Data } } -class UnspecificSslContextCreation extends SslContextCreation, UnspecificContextCreation { - // UnspecificSslContextCreation() { library instanceof Ssl } - // override ProtocolVersion getUnrestriction() { - // result = UnspecificContextCreation.super.getUnrestriction() and - // // These are turned off by default since Python 3.6 - // // see https://docs.python.org/3.6/library/ssl.html#ssl.SSLContext - // not result in ["SSLv2", "SSLv3"] - // } -} - -class UnspecificSslDefaultContextCreation extends SslDefaultContextCreation { - // override DataFlow::Node getContext() { result = this } - // // see https://docs.python.org/3/library/ssl.html#ssl.create_default_context - // override ProtocolVersion getUnrestriction() { - // result in ["TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"] - // } -} - +// class UnspecificSslContextCreation extends SslContextCreation, UnspecificContextCreation { +// // UnspecificSslContextCreation() { library instanceof Ssl } +// override ProtocolVersion getProtocol() { +// result = UnspecificContextCreation.super.getProtocol() and +// // These are turned off by default since Python 3.6 +// // see https://docs.python.org/3.6/library/ssl.html#ssl.SSLContext +// not result in ["SSLv2", "SSLv3"] +// } +// } +// class UnspecificSslDefaultContextCreation extends SslDefaultContextCreation { +// // override DataFlow::Node getContext() { result = this } +// // see https://docs.python.org/3/library/ssl.html#ssl.create_default_context +// override ProtocolVersion getProtocol() { result in ["TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"] } +// } class Ssl extends TlsLibrary { Ssl() { this = "ssl" } override string specific_version_name(ProtocolVersion version) { result = "PROTOCOL_" + version } - override string unspecific_version_name(ProtocolFamily family) { - family = "SSLv23" and result = "PROTOCOL_" + family + override string unspecific_version_name() { + result = "PROTOCOL_SSLv23" or - family = "TLS" and result = "PROTOCOL_" + family + ["", "_CLIENT", "_SERVER"] + result = "PROTOCOL_TLS" + ["", "_CLIENT", "_SERVER"] } override API::Node version_constants() { result = API::moduleImport("ssl") } diff --git a/python/ql/src/Security/CWE-327/TlsLibraryModel.qll b/python/ql/src/Security/CWE-327/TlsLibraryModel.qll index b45fa24a870..1e99755c1a9 100644 --- a/python/ql/src/Security/CWE-327/TlsLibraryModel.qll +++ b/python/ql/src/Security/CWE-327/TlsLibraryModel.qll @@ -37,29 +37,23 @@ class ProtocolVersion extends string { or this = "TLSv1_3" and result = 32 } - - /** Gets the protocol family for this protocol version. */ - ProtocolFamily getFamily() { - result = "SSLv23" and this in ["SSLv2", "SSLv3"] - or - result = "TLS" and this in ["TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"] - } -} - -/** An unspecific protocol version */ -class ProtocolFamily extends string { - ProtocolFamily() { this in ["SSLv23", "TLS"] } - - /** Gets the bit mask for this protocol family. */ - int getBits() { - result = sum(ProtocolVersion version | version.getFamily() = this | version.getBit()) - } } /** The creation of a context. */ abstract class ContextCreation extends DataFlow::Node { - /** Gets the protocol version or family for this context. */ - abstract string getProtocol(); + /** + * Gets the protocol version for this context. + * There can be multiple values if the context was created + * using a non-specific version such as `TLS`. + */ + abstract ProtocolVersion getProtocol(); + + /** + * Holds if the context was created with a specific version + * rather than with a version flexible method, see: + * https://www.openssl.org/docs/manmaster/man3/DTLS_server_method.html#NOTES + */ + predicate specificVersion() { count(this.getProtocol()) = 1 } } /** The creation of a connection from a context. */ @@ -91,13 +85,12 @@ abstract class ProtocolUnrestriction extends DataFlow::Node { * This also serves as unrestricting these protocols. */ abstract class UnspecificContextCreation extends ContextCreation { - // override ProtocolVersion getUnrestriction() { - // // There is only one family, the two names are aliases in OpenSSL. - // // see https://github.com/openssl/openssl/blob/13888e797c5a3193e91d71e5f5a196a2d68d266f/include/openssl/ssl.h.in#L1953-L1955 - // family in ["SSLv23", "TLS"] and - // // see https://docs.python.org/3/library/ssl.html#ssl-contexts - // result in ["SSLv2", "SSLv3", "TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"] - // } + override ProtocolVersion getProtocol() { + // There is only one family, the two names are aliases in OpenSSL. + // see https://github.com/openssl/openssl/blob/13888e797c5a3193e91d71e5f5a196a2d68d266f/include/openssl/ssl.h.in#L1953-L1955 + // see https://docs.python.org/3/library/ssl.html#ssl-contexts + result in ["SSLv2", "SSLv3", "TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"] + } } /** A model of a SSL/TLS library. */ @@ -108,8 +101,8 @@ abstract class TlsLibrary extends string { /** Gets the name of a specific protocol version. */ abstract string specific_version_name(ProtocolVersion version); - /** Gets a name, which is a member of `version_constants`, that can be used to specify the protocol family `family`. */ - abstract string unspecific_version_name(ProtocolFamily family); + /** Gets a name, which is a member of `version_constants`, that can be used to specify the entire protocol family. */ + abstract string unspecific_version_name(); /** Gets an API node representing the module or class holding the version constants. */ abstract API::Node version_constants(); @@ -119,9 +112,9 @@ abstract class TlsLibrary extends string { result = this.version_constants().getMember(this.specific_version_name(version)) } - /** Gets an API node representing the protocol family `family`. */ - API::Node unspecific_version(ProtocolFamily family) { - result = this.version_constants().getMember(this.unspecific_version_name(family)) + /** Gets an API node representing the protocol entire family. */ + API::Node unspecific_version() { + result = this.version_constants().getMember(this.unspecific_version_name()) } /** Gets a creation of a context with a default protocol. */ @@ -133,14 +126,15 @@ abstract class TlsLibrary extends string { /** Gets a creation of a context with a specific protocol version, known to be insecure. */ ContextCreation insecure_context_creation(ProtocolVersion version) { result in [this.specific_context_creation(), this.default_context_creation()] and + result.specificVersion() and result.getProtocol() = version and version.isInsecure() } /** Gets a context that was created using `family`, known to have insecure instances. */ - ContextCreation unspecific_context_creation(ProtocolFamily family) { + ContextCreation unspecific_context_creation() { result in [this.specific_context_creation(), this.default_context_creation()] and - result.getProtocol() = family + not result.specificVersion() } /** Gets a dataflow node representing a connection being created in an insecure manner, not from a context. */ diff --git a/python/ql/test/query-tests/Security/CWE-327-InsecureProtocol/InsecureProtocol.expected b/python/ql/test/query-tests/Security/CWE-327-InsecureProtocol/InsecureProtocol.expected index c08e19e5f1c..3e889d20f57 100644 --- a/python/ql/test/query-tests/Security/CWE-327-InsecureProtocol/InsecureProtocol.expected +++ b/python/ql/test/query-tests/Security/CWE-327-InsecureProtocol/InsecureProtocol.expected @@ -27,13 +27,13 @@ | ssl_fluent.py:71:14:71:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1 allowed by $@. | ssl_fluent.py:62:12:62:43 | ControlFlowNode for Attribute() | call to ssl.SSLContext | | ssl_fluent.py:71:14:71:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1 allowed by $@. | ssl_fluent.py:101:15:101:46 | ControlFlowNode for Attribute() | call to ssl.SSLContext | | ssl_fluent.py:71:14:71:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:62:12:62:43 | ControlFlowNode for Attribute() | call to ssl.SSLContext | +| ssl_fluent.py:71:14:71:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:65:15:65:46 | ControlFlowNode for Attribute() | call to ssl.SSLContext | | ssl_fluent.py:71:14:71:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:101:15:101:46 | ControlFlowNode for Attribute() | call to ssl.SSLContext | -| ssl_fluent.py:71:14:71:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:117:5:117:11 | ControlFlowNode for context | context modification | -| ssl_fluent.py:71:14:71:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:135:5:135:11 | ControlFlowNode for context | context modification | +| ssl_fluent.py:71:14:71:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:115:15:115:46 | ControlFlowNode for Attribute() | call to ssl.SSLContext | | ssl_fluent.py:77:14:77:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1 allowed by $@. | ssl_fluent.py:62:12:62:43 | ControlFlowNode for Attribute() | call to ssl.SSLContext | | ssl_fluent.py:77:14:77:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:62:12:62:43 | ControlFlowNode for Attribute() | call to ssl.SSLContext | -| ssl_fluent.py:97:14:97:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:95:5:95:11 | ControlFlowNode for context | context modification | -| ssl_fluent.py:146:14:146:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:143:5:143:11 | ControlFlowNode for context | context modification | -| ssl_fluent.py:165:14:165:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version SSLv3 allowed by $@. | ssl_fluent.py:162:5:162:11 | ControlFlowNode for context | context modification | +| ssl_fluent.py:97:14:97:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:65:15:65:46 | ControlFlowNode for Attribute() | call to ssl.SSLContext | +| ssl_fluent.py:146:14:146:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:142:15:142:46 | ControlFlowNode for Attribute() | call to ssl.SSLContext | +| ssl_fluent.py:165:14:165:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version SSLv3 allowed by $@. | ssl_fluent.py:161:15:161:65 | ControlFlowNode for Attribute() | call to ssl.create_default_context | | ssl_fluent.py:165:14:165:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1 allowed by $@. | ssl_fluent.py:161:15:161:65 | ControlFlowNode for Attribute() | call to ssl.create_default_context | | ssl_fluent.py:165:14:165:20 | ControlFlowNode for context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | ssl_fluent.py:161:15:161:65 | ControlFlowNode for Attribute() | call to ssl.create_default_context | From cf4eac6fa1e3d4e89454426bc759a7a88b10c064 Mon Sep 17 00:00:00 2001 From: yoff Date: Fri, 24 Mar 2023 13:18:03 +0100 Subject: [PATCH 4/6] Update python/ql/src/Security/CWE-327/PyOpenSSL.qll Co-authored-by: Rasmus Wriedt Larsen --- python/ql/src/Security/CWE-327/PyOpenSSL.qll | 3 --- 1 file changed, 3 deletions(-) diff --git a/python/ql/src/Security/CWE-327/PyOpenSSL.qll b/python/ql/src/Security/CWE-327/PyOpenSSL.qll index 9b685ecfd1c..7e99ebe27d9 100644 --- a/python/ql/src/Security/CWE-327/PyOpenSSL.qll +++ b/python/ql/src/Security/CWE-327/PyOpenSSL.qll @@ -55,9 +55,6 @@ class SetOptionsCall extends ProtocolRestriction, DataFlow::CallCfgNode { } } -// class UnspecificPyOpenSslContextCreation extends PyOpenSslContextCreation, UnspecificContextCreation { -// override ProtocolVersion getProtocol() { result = PyOpenSslContextCreation.super.getProtocol() } -// } class PyOpenSsl extends TlsLibrary { PyOpenSsl() { this = "pyOpenSSL" } From 8ea4878f7a584440dd446c9e58ea025bc903da06 Mon Sep 17 00:00:00 2001 From: Rasmus Lerchedahl Petersen Date: Fri, 24 Mar 2023 13:24:49 +0100 Subject: [PATCH 5/6] python: move comment --- python/ql/src/Security/CWE-327/PyOpenSSL.qll | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/python/ql/src/Security/CWE-327/PyOpenSSL.qll b/python/ql/src/Security/CWE-327/PyOpenSSL.qll index 7e99ebe27d9..e2571195bfa 100644 --- a/python/ql/src/Security/CWE-327/PyOpenSSL.qll +++ b/python/ql/src/Security/CWE-327/PyOpenSSL.qll @@ -19,10 +19,6 @@ class PyOpenSslContextCreation extends ContextCreation, DataFlow::CallCfgNode { protocolArg = pyo.specific_version(result).getAValueReachableFromSource() or protocolArg = pyo.unspecific_version().getAValueReachableFromSource() and - // PyOpenSSL also allows DTLS - // see https://www.pyopenssl.org/en/stable/api/ssl.html#OpenSSL.SSL.Context - // although they are not mentioned here: - // https://www.pyopenssl.org/en/stable/api/ssl.html#OpenSSL.SSL.TLS_METHOD result = any(ProtocolVersion pv) ) } @@ -64,6 +60,11 @@ class PyOpenSsl extends TlsLibrary { // See // - https://www.pyopenssl.org/en/23.0.0/api/ssl.html#module-OpenSSL.SSL // - https://www.openssl.org/docs/manmaster/man3/DTLS_server_method.html#NOTES + // + // PyOpenSSL also allows DTLS + // see https://www.pyopenssl.org/en/stable/api/ssl.html#OpenSSL.SSL.Context + // although they are not mentioned here: + // https://www.pyopenssl.org/en/stable/api/ssl.html#OpenSSL.SSL.TLS_METHOD result = ["TLS", "SSLv23"] + "_METHOD" or result = "TLS_" + ["CLIENT", "SERVER"] + "_METHOD" From 3c407eaa23c899c489025ac3f26444ff5d68bd5a Mon Sep 17 00:00:00 2001 From: Rasmus Lerchedahl Petersen Date: Fri, 24 Mar 2023 13:32:25 +0100 Subject: [PATCH 6/6] python: rewrite comment --- .../ql/src/Security/CWE-327/FluentApiModel.qll | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/python/ql/src/Security/CWE-327/FluentApiModel.qll b/python/ql/src/Security/CWE-327/FluentApiModel.qll index 3d63cab940e..d036020a1ba 100644 --- a/python/ql/src/Security/CWE-327/FluentApiModel.qll +++ b/python/ql/src/Security/CWE-327/FluentApiModel.qll @@ -4,21 +4,16 @@ import TlsLibraryModel /** * Configuration to determine the state of a context being used to create - * a connection. There is one configuration for each pair of `TlsLibrary` and `ProtocolVersion`, - * such that a single configuration only tracks contexts where a specific `ProtocolVersion` is allowed. + * a connection. The configuration uses a flow state to track the `TlsLibrary` + * and the insecure `ProtocolVersion`s that are allowed. * * The state is in terms of whether a specific protocol is allowed. This is * either true or false when the context is created and can then be modified - * later by either restricting or unrestricting the protocol (see the predicates - * `isRestriction` and `isUnrestriction`). + * later by either restricting or unrestricting the protocol (see the predicate + * `isAdditionalFlowStep`). * - * Since we are interested in the final state, we want the flow to start from - * the last unrestriction, so we disallow flow into unrestrictions. We also - * model the creation as an unrestriction of everything it allows, to account - * for the common case where the creation plays the role of "last unrestriction". - * - * Since we really want "the last unrestriction, not nullified by a restriction", - * we also disallow flow into restrictions. + * The state is represented as a bit vector, where each bit corresponds to a + * protocol version. The bit is set if the protocol is allowed. */ module InsecureContextConfiguration implements DataFlow::StateConfigSig { private newtype TFlowState =