Python: Comment everything

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

@@ -1,6 +1,11 @@
 import python
 import TlsLibraryModel
 
+/**
+ * Configuration to track flow from the creation of a context to
+ * that context being used to create a connection.
+ * Flow is broken if the insecure protocol of interest is being restricted.
+ */
 class InsecureContextConfiguration extends DataFlow::Configuration {
   TlsLibrary library;
 
@@ -25,28 +30,38 @@ class InsecureContextConfiguration extends DataFlow::Configuration {
   }
 }
 
+/** Configuration to specifically track the insecure protocol TLS 1.0 */
 class AllowsTLSv1 extends InsecureContextConfiguration {
   AllowsTLSv1() { this = library + "AllowsTLSv1" }
 
   override string flag() { result = "TLSv1" }
 }
 
+/** Configuration to specifically track the insecure protocol TLS 1.1 */
 class AllowsTLSv1_1 extends InsecureContextConfiguration {
   AllowsTLSv1_1() { this = library + "AllowsTLSv1_1" }
 
   override string flag() { result = "TLSv1_1" }
 }
 
+/**
+ * A connection is created from a context allowing an insecure protocol,
+ * and that protocol has not been restricted appropriately.
+ */
 predicate unsafe_connection_creation(DataFlow::Node node, ProtocolVersion insecure_version) {
+  // Connection created from a context allowing TLS 1.0.
   exists(AllowsTLSv1 c | c.hasFlowTo(node)) and
   insecure_version = "TLSv1"
   or
+  // Connection created from a context allowing TLS 1.1.
   exists(AllowsTLSv1_1 c | c.hasFlowTo(node)) and
   insecure_version = "TLSv1_1"
   or
+  // Connection created from a context for an insecure protocol.
   exists(TlsLibrary l | l.insecure_connection_creation(insecure_version) = node)
 }
 
+/** A connection is created insecurely without reference to a context. */
 predicate unsafe_context_creation(DataFlow::Node node, string insecure_version) {
   exists(TlsLibrary l, ContextCreation cc | cc = l.insecure_context_creation(insecure_version) |
     cc = node

python/ql/src/Security/CWE-327/InsecureProtocol.ql

@@ -12,22 +12,9 @@
 import python
 import FluentApiModel
 
-// string foo(ProtocolRestriction r) { result = r.getRestriction() }
-// The idea is to track flow from the creation of an insecure context to a use
-// such as `wrap_socket`. There should be a data-flow path for each insecure version
-// and each path should have a version specific sanitizer. This will allow fluent api
-// style code to block the paths one by one.
 from DataFlow::Node node, string insecure_version
 where
   unsafe_connection_creation(node, insecure_version)
   or
   unsafe_context_creation(node, insecure_version)
 select node, "Insecure SSL/TLS protocol version " + insecure_version //+ " specified in call to " + method_name + "."
-// from CallNode call, string method_name, string insecure_version
-// where
-//   unsafe_ssl_wrap_socket_call(call, method_name, insecure_version, _)
-//   or
-//   unsafe_pyOpenSSL_Context_call(call, insecure_version) and method_name = "pyOpenSSL.SSL.Context"
-// select call,
-//   "Insecure SSL/TLS protocol version " + insecure_version + " specified in call to " + method_name +
-//     "."

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

@@ -59,7 +59,7 @@ class PyOpenSSL extends TlsLibrary {
 
   override API::Node version_constants() { result = API::moduleImport("OpenSSL").getMember("SSL") }
 
-  override DataFlow::CfgNode default_context_creation() { none() }
+  override ContextCreation default_context_creation() { none() }
 
   override ContextCreation specific_context_creation() {
     result instanceof PyOpenSSLContextCreation

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

@@ -13,6 +13,16 @@ class SSLContextCreation extends ContextCreation {
   }
 }
 
+class SSLDefaultContextCreation extends ContextCreation {
+  SSLDefaultContextCreation() {
+    this = API::moduleImport("ssl").getMember("create_default_context").getACall()
+  }
+
+  // Allowed insecure versions are "TLSv1" and "TLSv1_1"
+  // see https://docs.python.org/3/library/ssl.html#context-creation
+  override DataFlow::CfgNode getProtocol() { none() }
+}
+
 class WrapSocketCall extends ConnectionCreation {
   override CallNode node;
 
@@ -67,7 +77,6 @@ class Ssl extends TlsLibrary {
   override string specific_insecure_version_name(ProtocolVersion version) {
     version in ["SSLv2", "SSLv3", "TLSv1", "TLSv1_1"] and
     result = "PROTOCOL_" + version
-    // result in ["PROTOCOL_SSLv2", "PROTOCOL_SSLv3", "PROTOCOL_TLSv1", "PROTOCOL_TLSv1_1"]
   }
 
   override string unspecific_version_name() {
@@ -83,10 +92,8 @@ class Ssl extends TlsLibrary {
 
   override API::Node version_constants() { result = API::moduleImport("ssl") }
 
-  override DataFlow::CfgNode default_context_creation() {
-    result = API::moduleImport("ssl").getMember("create_default_context").getACall() //and
-    // see https://docs.python.org/3/library/ssl.html#context-creation
-    // version in ["TLSv1", "TLSv1_1"]
+  override ContextCreation default_context_creation() {
+    result instanceof SSLDefaultContextCreation
   }
 
   override ContextCreation specific_context_creation() { result instanceof SSLContextCreation }

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

@@ -21,17 +21,24 @@ class ProtocolVersion extends string {
   }
 }
 
+/** The creation of a context. */
 abstract class ContextCreation extends DataFlow::CfgNode {
+  /** Gets the requested protocol if any. */
   abstract DataFlow::CfgNode getProtocol();
 }
 
+/** The creation of a connection from a context. */
 abstract class ConnectionCreation extends DataFlow::CfgNode {
+  /** Gets the context used to create the connection. */
   abstract DataFlow::CfgNode getContext();
 }
 
+/** A context is being restricted on which protocols it can accepts. */
 abstract class ProtocolRestriction extends DataFlow::CfgNode {
+  /** Gets the context being restricted. */
   abstract DataFlow::CfgNode getContext();
 
+  /** Gets the protocol version being disallowed. */
   abstract ProtocolVersion getRestriction();
 }
 
@@ -41,28 +48,35 @@ abstract class TlsLibrary extends string {
   /** The name of a specific protocol version, known to be insecure. */
   abstract string specific_insecure_version_name(ProtocolVersion version);
 
-  /** The name of an unspecific protocol version, say TLS, known to have insecure insatnces. */
+  /** The name of an unspecific protocol version, say TLS, known to have insecure instances. */
   abstract string unspecific_version_name();
 
+  /** The module or class holding the version constants. */
   abstract API::Node version_constants();
 
+  /** A dataflow node representing a specific protocol version, known to be insecure. */
   DataFlow::Node insecure_version(ProtocolVersion version) {
     result = version_constants().getMember(specific_insecure_version_name(version)).getAUse()
   }
 
+  /** A dataflow node representing an unspecific protocol version, say TLS, known to have insecure instances. */
   DataFlow::Node unspecific_version() {
     result = version_constants().getMember(unspecific_version_name()).getAUse()
   }
 
-  abstract DataFlow::CfgNode default_context_creation();
+  /** The creation of a context with a deafult protocol. */
+  abstract ContextCreation default_context_creation();
 
+  /** The creation of a context with a specific protocol. */
   abstract ContextCreation specific_context_creation();
 
+  /** The creation of a context with a specific protocol version, known to be insecure. */
   ContextCreation insecure_context_creation(ProtocolVersion version) {
     result = specific_context_creation() and
     result.getProtocol() = insecure_version(version)
   }
 
+  /** The creation of a context with an unspecific protocol version, say TLS, known to have insecure instances. */
   DataFlow::CfgNode unspecific_context_creation() {
     result = default_context_creation()
     or
@@ -70,11 +84,12 @@ abstract class TlsLibrary extends string {
     result.(ContextCreation).getProtocol() = unspecific_version()
   }
 
-  /** A connection is created in an outright insecure manner. */
+  /** A connection is created in an insecure manner, not from a context. */
   abstract DataFlow::CfgNode insecure_connection_creation(ProtocolVersion version);
 
   /** A connection is created from a context. */
   abstract ConnectionCreation connection_creation();
 
+  /** A context is being restricted on which protocols it can accepts. */
   abstract ProtocolRestriction protocol_restriction();
 }