Python: support unrestrictions

Also pyOpenSSL allows SSL 2 and SSL 3 on `SSLv23`

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

@@ -8,40 +8,44 @@ import TlsLibraryModel
  */
 class InsecureContextConfiguration extends DataFlow::Configuration {
   TlsLibrary library;
+  ProtocolVersion tracked_version;
 
-  InsecureContextConfiguration() { this = library + ["AllowsTLSv1", "AllowsTLSv1_1"] }
+  InsecureContextConfiguration() {
+    this = library + "Allows" + tracked_version and
+    tracked_version.isInsecure()
+  }
+
+  ProtocolVersion getTrackedVersion() { result = tracked_version }
 
   override predicate isSource(DataFlow::Node source) {
-    source = library.unspecific_context_creation()
+    // source = library.unspecific_context_creation()
+    exists(ProtocolUnrestriction pu |
+      pu = library.protocol_unrestriction() and
+      pu.getUnrestriction() = tracked_version
+    |
+      source = pu.getContext()
+    )
   }
 
   override predicate isSink(DataFlow::Node sink) {
     sink = library.connection_creation().getContext()
   }
 
-  abstract string flag();
-
   override predicate isBarrierOut(DataFlow::Node node) {
     exists(ProtocolRestriction r |
       r = library.protocol_restriction() and
       node = r.getContext() and
-      r.getRestriction() = flag()
+      r.getRestriction() = tracked_version
     )
   }
-}
 
-/** 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" }
+  override predicate isBarrierIn(DataFlow::Node node) {
+    exists(ProtocolUnrestriction r |
+      r = library.protocol_unrestriction() and
+      node = r.getContext() and
+      r.getUnrestriction() = tracked_version
+    )
+  }
 }
 
 /**
@@ -49,22 +53,22 @@ class AllowsTLSv1_1 extends InsecureContextConfiguration {
  * and that protocol has not been restricted appropriately.
  */
 predicate unsafe_connection_creation(
-  DataFlow::Node node, ProtocolVersion insecure_version, CallNode call
+  DataFlow::Node creation, ProtocolVersion insecure_version, DataFlow::Node source, boolean specific
 ) {
-  // Connection created from a context allowing TLS 1.0.
-  exists(AllowsTLSv1 c, ContextCreation cc | c.hasFlow(cc, node) | cc.getNode() = call) and
-  insecure_version = "TLSv1"
+  // Connection created from a context allowing `insecure_version`.
+  exists(InsecureContextConfiguration c, ProtocolUnrestriction cc | c.hasFlow(cc, creation) |
+    insecure_version = c.getTrackedVersion() and
+    source = cc and
+    specific = false
+  )
   or
-  // Connection created from a context allowing TLS 1.1.
-  exists(AllowsTLSv1_1 c, ContextCreation cc | c.hasFlow(cc, node) | cc.getNode() = call) and
-  insecure_version = "TLSv1_1"
-  or
-  // Connection created from a context for an insecure protocol.
+  // Connection created from a context specifying `insecure_version`.
   exists(TlsLibrary l, DataFlow::CfgNode cc |
     cc = l.insecure_connection_creation(insecure_version)
   |
-    cc = node and
-    cc.getNode() = call
+    creation = cc and
+    source = cc and
+    specific = true
   )
 }
 

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

@@ -18,11 +18,25 @@ string callName(AstNode call) {
   exists(Attribute a | a = call | result = callName(a.getObject()) + "." + a.getName())
 }
 
-from DataFlow::Node node, string insecure_version, CallNode call
-where
-  unsafe_connection_creation(node, insecure_version, call)
+string sourceName(DataFlow::Node source) {
+  result = "call to " + callName(source.asCfgNode().(CallNode).getFunction().getNode())
   or
-  unsafe_context_creation(node, insecure_version, call)
-select node, "Insecure SSL/TLS protocol version " + insecure_version + " specified in $@ ", call,
-  "call to " + callName(call.getFunction().getNode())
-//+ " specified in call to " + method_name + "."
+  not source.asCfgNode() instanceof CallNode and
+  not source instanceof ContextCreation and
+  result = "context modification"
+}
+
+string verb(boolean specific) {
+  specific = true and result = "specified"
+  or
+  specific = false and result = "allowed"
+}
+
+from DataFlow::Node creation, string insecure_version, DataFlow::Node source, boolean specific
+where
+  unsafe_connection_creation(creation, insecure_version, source, specific)
+  or
+  unsafe_context_creation(creation, insecure_version, source.asCfgNode()) and specific = true
+select creation,
+  "Insecure SSL/TLS protocol version " + insecure_version + " " + verb(specific) + " by $@ ",
+  source, sourceName(source)

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

@@ -26,6 +26,8 @@ class ConnectionCall extends ConnectionCreation {
   }
 }
 
+// This cannot be used to unrestrict,
+// see https://www.pyopenssl.org/en/stable/api/ssl.html#OpenSSL.SSL.Context.set_options
 class SetOptionsCall extends ProtocolRestriction {
   override CallNode node;
 
@@ -42,6 +44,10 @@ class SetOptionsCall extends ProtocolRestriction {
   }
 }
 
+class UnspecificPyOpenSSLContextCreation extends PyOpenSSLContextCreation, UnspecificContextCreation {
+  UnspecificPyOpenSSLContextCreation() { library = "pyOpenSSL" }
+}
+
 class PyOpenSSL extends TlsLibrary {
   PyOpenSSL() { this = "pyOpenSSL" }
 
@@ -50,11 +56,9 @@ class PyOpenSSL extends TlsLibrary {
     result = version + "_METHOD"
   }
 
-  override string unspecific_version_name() {
-    result in [
-        "TLS_METHOD", // This is not actually available in pyOpenSSL yet
-        "SSLv23_METHOD" // This is what can negotiate TLS 1.3 (indeed, I know, I did test that..)
-      ]
+  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 API::Node version_constants() { result = API::moduleImport("OpenSSL").getMember("SSL") }
@@ -70,4 +74,8 @@ class PyOpenSSL extends TlsLibrary {
   override ConnectionCreation connection_creation() { result instanceof ConnectionCall }
 
   override ProtocolRestriction protocol_restriction() { result instanceof SetOptionsCall }
+
+  override ProtocolUnrestriction protocol_unrestriction() {
+    result instanceof UnspecificPyOpenSSLContextCreation
+  }
 }

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

@@ -56,6 +56,31 @@ class OptionsAugOr extends ProtocolRestriction {
   override ProtocolVersion getRestriction() { result = restriction }
 }
 
+class OptionsAugAndNot extends ProtocolUnrestriction {
+  ProtocolVersion restriction;
+
+  OptionsAugAndNot() {
+    exists(AugAssign aa, AttrNode attr, Expr flag, UnaryExpr notFlag |
+      aa.getOperation().getOp() instanceof BitAnd and
+      aa.getTarget() = attr.getNode() and
+      attr.getName() = "options" and
+      attr.getObject() = node and
+      notFlag.getOp() instanceof Invert and
+      notFlag.getOperand() = flag and
+      flag = API::moduleImport("ssl").getMember("OP_NO_" + restriction).getAUse().asExpr() and
+      (
+        aa.getValue() = notFlag
+        or
+        impliesValue(aa.getValue(), notFlag, true, true)
+      )
+    )
+  }
+
+  override DataFlow::CfgNode getContext() { result = this }
+
+  override ProtocolVersion getUnrestriction() { result = restriction }
+}
+
 /** Whether `part` evaluates to `partIsTrue` if `whole` evaluates to `wholeIsTrue`. */
 predicate impliesValue(BinaryExpr whole, Expr part, boolean partIsTrue, boolean wholeIsTrue) {
   whole.getOp() instanceof BitAnd and
@@ -75,8 +100,8 @@ predicate impliesValue(BinaryExpr whole, Expr part, boolean partIsTrue, boolean
   )
 }
 
-class ContextSetVersion extends ProtocolRestriction {
-  string restriction;
+class ContextSetVersion extends ProtocolRestriction, ProtocolUnrestriction {
+  ProtocolVersion restriction;
 
   ContextSetVersion() {
     exists(Attributes::AttrWrite aw |
@@ -90,6 +115,21 @@ class ContextSetVersion extends ProtocolRestriction {
   override DataFlow::CfgNode getContext() { result = this }
 
   override ProtocolVersion getRestriction() { result.lessThan(restriction) }
+
+  override ProtocolVersion getUnrestriction() {
+    restriction = result or restriction.lessThan(result)
+  }
+}
+
+class UnspecificSSLContextCreation extends SSLContextCreation, UnspecificContextCreation {
+  UnspecificSSLContextCreation() { library = "ssl" }
+
+  override ProtocolVersion getUnrestriction() {
+    result = UnspecificContextCreation.super.getUnrestriction() and
+    // These are turned off by default
+    // see https://docs.python.org/3/library/ssl.html#ssl-contexts
+    not result in ["SSLv2", "SSLv3"]
+  }
 }
 
 class Ssl extends TlsLibrary {
@@ -100,16 +140,7 @@ class Ssl extends TlsLibrary {
     result = "PROTOCOL_" + version
   }
 
-  override string unspecific_version_name() {
-    result =
-      "PROTOCOL_" +
-        [
-          "TLS",
-          // This can negotiate a TLS 1.3 connection (!)
-          // see https://docs.python.org/3/library/ssl.html#ssl-contexts
-          "SSLv23"
-        ]
-  }
+  override string unspecific_version_name(ProtocolFamily family) { result = "PROTOCOL_" + family }
 
   override API::Node version_constants() { result = API::moduleImport("ssl") }
 
@@ -132,4 +163,12 @@ class Ssl extends TlsLibrary {
     or
     result instanceof ContextSetVersion
   }
+
+  override ProtocolUnrestriction protocol_unrestriction() {
+    result instanceof OptionsAugAndNot
+    or
+    result instanceof ContextSetVersion
+    or
+    result instanceof UnspecificSSLContextCreation
+  }
 }

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

@@ -19,6 +19,13 @@ class ProtocolVersion extends string {
     or
     this = ["TLSv1", "TLSv1_1", "TLSv1_2"] and version = "TLSv1_3"
   }
+
+  predicate isInsecure() { this in ["SSLv2", "SSLv3", "TLSv1", "TLSv1_1"] }
+}
+
+/** An unspecific protocol version */
+class ProtocolFamily extends string {
+  ProtocolFamily() { this in ["SSLv23", "TLS"] }
 }
 
 /** The creation of a context. */
@@ -42,6 +49,34 @@ abstract class ProtocolRestriction extends DataFlow::CfgNode {
   abstract ProtocolVersion getRestriction();
 }
 
+/** A context is being relaxed on which protocols it can accepts. */
+abstract class ProtocolUnrestriction extends DataFlow::CfgNode {
+  /** Gets the context being relaxed. */
+  abstract DataFlow::CfgNode getContext();
+
+  /** Gets the protocol version being allowed. */
+  abstract ProtocolVersion getUnrestriction();
+}
+
+abstract class UnspecificContextCreation extends ContextCreation, ProtocolUnrestriction {
+  TlsLibrary library;
+  ProtocolFamily family;
+
+  UnspecificContextCreation() { this.getProtocol() = library.unspecific_version(family) }
+
+  override DataFlow::CfgNode getContext() { result = this }
+
+  override ProtocolVersion getUnrestriction() {
+    family = "TLS" and
+    result in ["TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"]
+    or
+    // This can negotiate a TLS 1.3 connection (!)
+    // see https://docs.python.org/3/library/ssl.html#ssl-contexts
+    family = "SSLv23" and
+    result in ["SSLv2", "SSLv3", "TLSv1", "TLSv1_1", "TLSv1_2", "TLSv1_3"]
+  }
+}
+
 abstract class TlsLibrary extends string {
   TlsLibrary() { this in ["ssl", "pyOpenSSL"] }
 
@@ -49,7 +84,7 @@ abstract class TlsLibrary extends string {
   abstract string specific_insecure_version_name(ProtocolVersion version);
 
   /** The name of an unspecific protocol version, say TLS, known to have insecure instances. */
-  abstract string unspecific_version_name();
+  abstract string unspecific_version_name(ProtocolFamily family);
 
   /** The module or class holding the version constants. */
   abstract API::Node version_constants();
@@ -60,8 +95,8 @@ abstract class TlsLibrary extends string {
   }
 
   /** 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()
+  DataFlow::Node unspecific_version(ProtocolFamily family) {
+    result = version_constants().getMember(unspecific_version_name(family)).getAUse()
   }
 
   /** The creation of a context with a deafult protocol. */
@@ -77,11 +112,11 @@ abstract class TlsLibrary extends string {
   }
 
   /** The creation of a context with an unspecific protocol version, say TLS, known to have insecure instances. */
-  DataFlow::CfgNode unspecific_context_creation() {
+  DataFlow::CfgNode unspecific_context_creation(ProtocolFamily family) {
     result = default_context_creation()
     or
     result = specific_context_creation() and
-    result.(ContextCreation).getProtocol() = unspecific_version()
+    result.(ContextCreation).getProtocol() = unspecific_version(family)
   }
 
   /** A connection is created in an insecure manner, not from a context. */
@@ -92,4 +127,7 @@ abstract class TlsLibrary extends string {
 
   /** A context is being restricted on which protocols it can accepts. */
   abstract ProtocolRestriction protocol_restriction();
+
+  /** A context is being relaxed on which protocols it can accepts. */
+  abstract ProtocolUnrestriction protocol_unrestriction();
 }