Python: restrict attention to ss.wrap_socket

python/change-notes/2021-02-23-port-insecure-default-protocol.md

@@ -1,2 +1,2 @@
 lgtm,codescanning
-* Changed the query that detects insecure SSL/TLS protocol creation from default values (`py/insecure-default-protocol`) to use the new API graphs. Python 3.4 introduced new ways to specify protocol version (through modifying the `options` field), which means that using the default value _can_ be secure (and if the usage is not secure, this should be reported by the `py/insecure-protocol` query). This query now reports results only in versions of Python prior to 3.4.
+* Changed the query that detects insecure SSL/TLS protocol creation from default values (`py/insecure-default-protocol`) to use the new API graphs. Python 3.2 introduced so-called contexts to create connections from. These come with new ways to specify protocol version (through modifying the `options` field), which means that using the default value _can_ be secure (and if the usage is not secure, this should be reported by the `py/insecure-protocol` query). This query now reports results only for the deprecated way of creating connections without a context.

python/ql/src/Security/CWE-327/InsecureDefaultProtocol.qhelp

@@ -4,7 +4,7 @@
 <qhelp>
      <overview>
           <p>
-          In version of Python before 3.4, the <code>ssl</code> library defaults
+          The <code>ssl</code> library defaults
           to an insecure version of SSL/TLS when no specific protocol version is
           specified. This may leave the connection vulnerable to attack.
           </p>
@@ -37,6 +37,13 @@
             instead, for instance by specifying
             <code>ssl_version=PROTOCOL_TLSv1_2</code> as a keyword argument.
           </p>
+          <p>
+            The latter example can also be made secure by modifying the created
+            context before it is used to create a connection and so will not be
+            flagged by this query. However, if a connection is created before
+            the context has been secured (e.g. by setting the value of <code>minimum_version</code>),
+            then the code should be flagged by the query <code>py/insecure-protocol</code>.
+          </p>
           <p>
             Note that <code>ssl.wrap_socket</code> has been deprecated in
             Python 3.7. The recommended alternatives are:

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

@@ -11,14 +11,14 @@
  */
 
 // This query is based on the premise that default constructors are always a security concern.
-// This has become untrue since Python 3.4 where flags such as `OP_NO_TLSv1_1` were introduced
-// on the `options` field to modify contexts created with default values. These flags are now,
-// since OpenSSL1.1.0, themselves deprecated in favor of the `minimum_version` field (and its
-// counterpart `maximum_version`).
+// This has become untrue since Python 3.2 where the `SSLContext` was introduced. Such contexts
+// are designed to later be modified by flags such as `OP_NO_TLSv1_1`, and so the default values
+// are not necessarity problematic.
 //
 // Detecting that a connection is created with a context that has not been suitably modified is
 // handled by the data-flow query py/insecure-protocol, while the present query is restricted
-// to alerting on default constructors when the Python version is earlier than 3.4.
+// to alerting on the one deprecated default constructor whch does not refer to a contex, namely
+// `ssl.wrap_socket`.
 import python
 import semmle.python.ApiGraphs
 
@@ -26,17 +26,6 @@ CallNode unsafe_call(string method_name) {
   result = API::moduleImport("ssl").getMember("wrap_socket").getACall().asCfgNode() and
   not exists(result.getArgByName("ssl_version")) and
   method_name = "deprecated method ssl.wrap_socket"
-  or
-  result = API::moduleImport("ssl").getMember("SSLContext").getACall().asCfgNode() and
-  not exists(result.getArgByName("protocol")) and
-  not exists(result.getArg(0)) and
-  method_name = "ssl.SSLContext" and
-  // in version 3.4, flags were introduced to modify contexts created with default values
-  (
-    major_version() = 2
-    or
-    major_version() = 3 and minor_version() < 4
-  )
 }
 
 from CallNode call, string method_name

python/ql/test/query-tests/Security/CWE-327-py2/InsecureDefaultProtocol.expected

@@ -1,2 +1 @@
-| InsecureProtocol.py:9:1:9:17 | ControlFlowNode for Attribute() | Call to deprecated method ssl.wrap_socket does not specify a protocol, which may result in an insecure default being used. |
-| InsecureProtocol.py:10:11:10:22 | ControlFlowNode for SSLContext() | Call to ssl.SSLContext does not specify a protocol, which may result in an insecure default being used. |
+| InsecureProtocol.py:7:1:7:17 | ControlFlowNode for Attribute() | Call to deprecated method ssl.wrap_socket does not specify a protocol, which may result in an insecure default being used. |

python/ql/test/query-tests/Security/CWE-327-py2/InsecureProtocol.py

@@ -1,10 +1,7 @@
 import ssl
-from ssl import SSLContext
 
 # secure versions
 ssl.wrap_socket(ssl_version=ssl.PROTOCOL_TLSv1_2)
-SSLContext(protocol=ssl.PROTOCOL_TLSv1_2)
 
 # possibly insecure default
 ssl.wrap_socket()
-context = SSLContext()