Skip to content

Commit dab0abb

Browse files
RasmusWLRasmusWL
authored
Merge pull request github#12428 from yoff/python/rewrite-InsecureContextConfiguration
Python: Clean up insecure context query
2 parents 7e7cd54 + 2121ed7 commit dab0abb

File tree

5 files changed

+92
-100
lines changed

5 files changed

+92
-100
lines changed

‎python/ql/src/Security/CWE-327/FluentApiModel.qll

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,23 +4,18 @@ import TlsLibraryModel
44

55
/**
66
* Configuration to determine the state of a context being used to create
7-
* a connection. There is one configuration for each pair of `TlsLibrary` and `ProtocolVersion`,
8-
* such that a single configuration only tracks contexts where a specific `ProtocolVersion` is allowed.
7+
* a connection. The configuration uses a flow state to track the `TlsLibrary`
8+
* and the insecure `ProtocolVersion`s that are allowed.
99
*
1010
* The state is in terms of whether a specific protocol is allowed. This is
1111
* either true or false when the context is created and can then be modified
12-
* later by either restricting or unrestricting the protocol (see the predicates
13-
* `isRestriction` and `isUnrestriction`).
12+
* later by either restricting or unrestricting the protocol (see the predicate
13+
* `isAdditionalFlowStep`).
1414
*
15-
* Since we are interested in the final state, we want the flow to start from
16-
* the last unrestriction, so we disallow flow into unrestrictions. We also
17-
* model the creation as an unrestriction of everything it allows, to account
18-
* for the common case where the creation plays the role of "last unrestriction".
19-
*
20-
* Since we really want "the last unrestriction, not nullified by a restriction",
21-
* we also disallow flow into restrictions.
15+
* The state is represented as a bit vector, where each bit corresponds to a
16+
* protocol version. The bit is set if the protocol is allowed.
2217
*/
23-
module InsecureContextConfiguration2 implements DataFlow::StateConfigSig {
18+
module InsecureContextConfiguration implements DataFlow::StateConfigSig {
2419
private newtype TFlowState =
2520
TMkFlowState(TlsLibrary library, int bits) {
2621
bits in [0 .. max(any(ProtocolVersion v).getBit()) * 2 - 1]
@@ -61,9 +56,14 @@ module InsecureContextConfiguration2 implements DataFlow::StateConfigSig {
6156
}
6257

6358
predicate isSource(DataFlow::Node source, FlowState state) {
64-
exists(ProtocolFamily family |
65-
source = state.getLibrary().unspecific_context_creation(family) and
66-
state.getBits() = family.getBits()
59+
exists(ContextCreation creation | source = creation |
60+
creation = state.getLibrary().unspecific_context_creation() and
61+
state.getBits() =
62+
sum(ProtocolVersion version |
63+
version = creation.getProtocol() and version.isInsecure()
64+
|
65+
version.getBit()
66+
)
6767
)
6868
}
6969

@@ -112,7 +112,7 @@ module InsecureContextConfiguration2 implements DataFlow::StateConfigSig {
112112
}
113113
}
114114

115-
private module InsecureContextFlow = DataFlow::GlobalWithState<InsecureContextConfiguration2>;
115+
private module InsecureContextFlow = DataFlow::GlobalWithState<InsecureContextConfiguration>;
116116

117117
/**
118118
* Holds if `conectionCreation` marks the creation of a connection based on the contex

‎python/ql/src/Security/CWE-327/PyOpenSSL.qll

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,14 @@ class PyOpenSslContextCreation extends ContextCreation, DataFlow::CallCfgNode {
1212
this = API::moduleImport("OpenSSL").getMember("SSL").getMember("Context").getACall()
1313
}
1414

15-
override string getProtocol() {
15+
override ProtocolVersion getProtocol() {
1616
exists(DataFlow::Node protocolArg, PyOpenSsl pyo |
1717
protocolArg in [this.getArg(0), this.getArgByName("method")]
1818
|
19-
protocolArg in [
20-
pyo.specific_version(result).getAValueReachableFromSource(),
21-
pyo.unspecific_version(result).getAValueReachableFromSource()
22-
]
19+
protocolArg = pyo.specific_version(result).getAValueReachableFromSource()
20+
or
21+
protocolArg = pyo.unspecific_version().getAValueReachableFromSource() and
22+
result = any(ProtocolVersion pv)
2323
)
2424
}
2525
}
@@ -51,19 +51,23 @@ class SetOptionsCall extends ProtocolRestriction, DataFlow::CallCfgNode {
5151
}
5252
}
5353

54-
class UnspecificPyOpenSslContextCreation extends PyOpenSslContextCreation, UnspecificContextCreation
55-
{
56-
// UnspecificPyOpenSslContextCreation() { library instanceof PyOpenSsl }
57-
}
58-
5954
class PyOpenSsl extends TlsLibrary {
6055
PyOpenSsl() { this = "pyOpenSSL" }
6156

6257
override string specific_version_name(ProtocolVersion version) { result = version + "_METHOD" }
6358

64-
override string unspecific_version_name(ProtocolFamily family) {
65-
// `"TLS_METHOD"` is not actually available in pyOpenSSL yet, but should be coming soon..
66-
result = family + "_METHOD"
59+
override string unspecific_version_name() {
60+
// See
61+
// - https://www.pyopenssl.org/en/23.0.0/api/ssl.html#module-OpenSSL.SSL
62+
// - https://www.openssl.org/docs/manmaster/man3/DTLS_server_method.html#NOTES
63+
//
64+
// PyOpenSSL also allows DTLS
65+
// see https://www.pyopenssl.org/en/stable/api/ssl.html#OpenSSL.SSL.Context
66+
// although they are not mentioned here:
67+
// https://www.pyopenssl.org/en/stable/api/ssl.html#OpenSSL.SSL.TLS_METHOD
68+
result = ["TLS", "SSLv23"] + "_METHOD"
69+
or
70+
result = "TLS_" + ["CLIENT", "SERVER"] + "_METHOD"
6771
}
6872

6973
override API::Node version_constants() { result = API::moduleImport("OpenSSL").getMember("SSL") }
@@ -80,7 +84,5 @@ class PyOpenSsl extends TlsLibrary {
8084

8185
override ProtocolRestriction protocol_restriction() { result instanceof SetOptionsCall }
8286

83-
override ProtocolUnrestriction protocol_unrestriction() {
84-
result instanceof UnspecificPyOpenSslContextCreation
85-
}
87+
override ProtocolUnrestriction protocol_unrestriction() { none() }
8688
}

‎python/ql/src/Security/CWE-327/Ssl.qll

Lines changed: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,21 @@ import TlsLibraryModel
1010
class SslContextCreation extends ContextCreation, DataFlow::CallCfgNode {
1111
SslContextCreation() { this = API::moduleImport("ssl").getMember("SSLContext").getACall() }
1212

13-
override string getProtocol() {
13+
override ProtocolVersion getProtocol() {
1414
exists(DataFlow::Node protocolArg, Ssl ssl |
1515
protocolArg in [this.getArg(0), this.getArgByName("protocol")]
1616
|
17-
protocolArg =
18-
[
19-
ssl.specific_version(result).getAValueReachableFromSource(),
20-
ssl.unspecific_version(result).getAValueReachableFromSource()
21-
]
17+
protocolArg = ssl.specific_version(result).getAValueReachableFromSource()
18+
or
19+
protocolArg = ssl.unspecific_version().getAValueReachableFromSource() and
20+
// see https://docs.python.org/3/library/ssl.html#id7
21+
result in ["TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"]
2222
)
2323
or
2424
not exists(this.getArg(_)) and
2525
not exists(this.getArgByName(_)) and
26-
result = "TLS"
26+
// see https://docs.python.org/3/library/ssl.html#id7
27+
result in ["TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"]
2728
}
2829
}
2930

@@ -34,7 +35,7 @@ class SslDefaultContextCreation extends ContextCreation {
3435

3536
// Allowed insecure versions are "TLSv1" and "TLSv1_1"
3637
// see https://docs.python.org/3/library/ssl.html#context-creation
37-
override string getProtocol() { result = "TLS" }
38+
override ProtocolVersion getProtocol() { result in ["TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"] }
3839
}
3940

4041
/** Gets a reference to an `ssl.Context` instance. */
@@ -161,33 +162,29 @@ class ContextSetVersion extends ProtocolRestriction, ProtocolUnrestriction, Data
161162
}
162163
}
163164

164-
class UnspecificSslContextCreation extends SslContextCreation, UnspecificContextCreation {
165-
// UnspecificSslContextCreation() { library instanceof Ssl }
166-
// override ProtocolVersion getUnrestriction() {
167-
// result = UnspecificContextCreation.super.getUnrestriction() and
168-
// // These are turned off by default since Python 3.6
169-
// // see https://docs.python.org/3.6/library/ssl.html#ssl.SSLContext
170-
// not result in ["SSLv2", "SSLv3"]
171-
// }
172-
}
173-
174-
class UnspecificSslDefaultContextCreation extends SslDefaultContextCreation {
175-
// override DataFlow::Node getContext() { result = this }
176-
// // see https://docs.python.org/3/library/ssl.html#ssl.create_default_context
177-
// override ProtocolVersion getUnrestriction() {
178-
// result in ["TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"]
179-
// }
180-
}
181-
165+
// class UnspecificSslContextCreation extends SslContextCreation, UnspecificContextCreation {
166+
// // UnspecificSslContextCreation() { library instanceof Ssl }
167+
// override ProtocolVersion getProtocol() {
168+
// result = UnspecificContextCreation.super.getProtocol() and
169+
// // These are turned off by default since Python 3.6
170+
// // see https://docs.python.org/3.6/library/ssl.html#ssl.SSLContext
171+
// not result in ["SSLv2", "SSLv3"]
172+
// }
173+
// }
174+
// class UnspecificSslDefaultContextCreation extends SslDefaultContextCreation {
175+
// // override DataFlow::Node getContext() { result = this }
176+
// // see https://docs.python.org/3/library/ssl.html#ssl.create_default_context
177+
// override ProtocolVersion getProtocol() { result in ["TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"] }
178+
// }
182179
class Ssl extends TlsLibrary {
183180
Ssl() { this = "ssl" }
184181

185182
override string specific_version_name(ProtocolVersion version) { result = "PROTOCOL_" + version }
186183

187-
override string unspecific_version_name(ProtocolFamily family) {
188-
family = "SSLv23" and result = "PROTOCOL_" + family
184+
override string unspecific_version_name() {
185+
result = "PROTOCOL_SSLv23"
189186
or
190-
family = "TLS" and result = "PROTOCOL_" + family + ["", "_CLIENT", "_SERVER"]
187+
result = "PROTOCOL_TLS" + ["", "_CLIENT", "_SERVER"]
191188
}
192189

193190
override API::Node version_constants() { result = API::moduleImport("ssl") }
@@ -217,9 +214,5 @@ class Ssl extends TlsLibrary {
217214
result instanceof OptionsAugAndNot
218215
or
219216
result instanceof ContextSetVersion
220-
or
221-
result instanceof UnspecificSslContextCreation
222-
or
223-
result instanceof UnspecificSslDefaultContextCreation
224217
}
225218
}

‎python/ql/src/Security/CWE-327/TlsLibraryModel.qll

Lines changed: 27 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -37,29 +37,23 @@ class ProtocolVersion extends string {
3737
or
3838
this = "TLSv1_3" and result = 32
3939
}
40-
41-
/** Gets the protocol family for this protocol version. */
42-
ProtocolFamily getFamily() {
43-
result = "SSLv23" and this in ["SSLv2", "SSLv3"]
44-
or
45-
result = "TLS" and this in ["TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"]
46-
}
47-
}
48-
49-
/** An unspecific protocol version */
50-
class ProtocolFamily extends string {
51-
ProtocolFamily() { this in ["SSLv23", "TLS"] }
52-
53-
/** Gets the bit mask for this protocol family. */
54-
int getBits() {
55-
result = sum(ProtocolVersion version | version.getFamily() = this | version.getBit())
56-
}
5740
}
5841

5942
/** The creation of a context. */
6043
abstract class ContextCreation extends DataFlow::Node {
61-
/** Gets the protocol version or family for this context. */
62-
abstract string getProtocol();
44+
/**
45+
* Gets the protocol version for this context.
46+
* There can be multiple values if the context was created
47+
* using a non-specific version such as `TLS`.
48+
*/
49+
abstract ProtocolVersion getProtocol();
50+
51+
/**
52+
* Holds if the context was created with a specific version
53+
* rather than with a version flexible method, see:
54+
* https://www.openssl.org/docs/manmaster/man3/DTLS_server_method.html#NOTES
55+
*/
56+
predicate specificVersion() { count(this.getProtocol()) = 1 }
6357
}
6458

6559
/** The creation of a connection from a context. */
@@ -91,13 +85,12 @@ abstract class ProtocolUnrestriction extends DataFlow::Node {
9185
* This also serves as unrestricting these protocols.
9286
*/
9387
abstract class UnspecificContextCreation extends ContextCreation {
94-
// override ProtocolVersion getUnrestriction() {
95-
// // There is only one family, the two names are aliases in OpenSSL.
96-
// // see https://github.com/openssl/openssl/blob/13888e797c5a3193e91d71e5f5a196a2d68d266f/include/openssl/ssl.h.in#L1953-L1955
97-
// family in ["SSLv23", "TLS"] and
98-
// // see https://docs.python.org/3/library/ssl.html#ssl-contexts
99-
// result in ["SSLv2", "SSLv3", "TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"]
100-
// }
88+
override ProtocolVersion getProtocol() {
89+
// There is only one family, the two names are aliases in OpenSSL.
90+
// see https://github.com/openssl/openssl/blob/13888e797c5a3193e91d71e5f5a196a2d68d266f/include/openssl/ssl.h.in#L1953-L1955
91+
// see https://docs.python.org/3/library/ssl.html#ssl-contexts
92+
result in ["SSLv2", "SSLv3", "TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"]
93+
}
10194
}
10295

10396
/** A model of a SSL/TLS library. */
@@ -108,8 +101,8 @@ abstract class TlsLibrary extends string {
108101
/** Gets the name of a specific protocol version. */
109102
abstract string specific_version_name(ProtocolVersion version);
110103

111-
/** Gets a name, which is a member of `version_constants`, that can be used to specify the protocol family `family`. */
112-
abstract string unspecific_version_name(ProtocolFamily family);
104+
/** Gets a name, which is a member of `version_constants`, that can be used to specify the entire protocol family. */
105+
abstract string unspecific_version_name();
113106

114107
/** Gets an API node representing the module or class holding the version constants. */
115108
abstract API::Node version_constants();
@@ -119,9 +112,9 @@ abstract class TlsLibrary extends string {
119112
result = this.version_constants().getMember(this.specific_version_name(version))
120113
}
121114

122-
/** Gets an API node representing the protocol family `family`. */
123-
API::Node unspecific_version(ProtocolFamily family) {
124-
result = this.version_constants().getMember(this.unspecific_version_name(family))
115+
/** Gets an API node representing the protocol entire family. */
116+
API::Node unspecific_version() {
117+
result = this.version_constants().getMember(this.unspecific_version_name())
125118
}
126119

127120
/** Gets a creation of a context with a default protocol. */
@@ -133,14 +126,15 @@ abstract class TlsLibrary extends string {
133126
/** Gets a creation of a context with a specific protocol version, known to be insecure. */
134127
ContextCreation insecure_context_creation(ProtocolVersion version) {
135128
result in [this.specific_context_creation(), this.default_context_creation()] and
129+
result.specificVersion() and
136130
result.getProtocol() = version and
137131
version.isInsecure()
138132
}
139133

140134
/** Gets a context that was created using `family`, known to have insecure instances. */
141-
ContextCreation unspecific_context_creation(ProtocolFamily family) {
135+
ContextCreation unspecific_context_creation() {
142136
result in [this.specific_context_creation(), this.default_context_creation()] and
143-
result.getProtocol() = family
137+
not result.specificVersion()
144138
}
145139

146140
/** Gets a dataflow node representing a connection being created in an insecure manner, not from a context. */

‎python/ql/test/query-tests/Security/CWE-327-InsecureProtocol/InsecureProtocol.expected

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,18 @@
1818
| import_use.py:17:14:17:34 | ControlFlowNode for also_insecure_context | Insecure SSL/TLS protocol version TLSv1_1 allowed by $@. | import_def.py:10:25:10:56 | ControlFlowNode for Attribute() | call to ssl.SSLContext |
1919
| 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 |
2020
| 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 |
21+
| 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 |
22+
| 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 |
2123
| 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 |
2224
| 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 |
25+
| 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 |
2326
| 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 |
2427
| 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 |
2528
| 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 |
2629
| 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 |
2730
| 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 |
28-
| 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 |
29-
| 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 |
31+
| 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 |
32+
| 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 |
3033
| 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 |
3134
| 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 |
3235
| 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 |

0 commit comments

Comments
 (0)