Ssl2/3 should not be used for secure VNC access

So, it looks like what we need to do here is pass ssl_version=ssl.PROTOCOL_TLSv1_2 [1] to wrap_socket [2] to fix this.

[1] https://docs.python.org/2/library/ssl.html#ssl.PROTOCOL_TLSv1_2
[2] https://docs.python.org/2/library/ssl.html#ssl.wrap_socket

Fix proposed to branch: master
Review: https://review.openstack.org/589992

Change abandoned by melanie witt (<email address hidden>) on branch: master
Review: https://review.openstack.org/589992
Reason: openssl should already use systemwide settings for the ssl_version underneath, so there is no need in hard-coding a version here. In fact, it would cause systemwide settings not to be honored.

I'm going to close out this bug based on input from Daniel Berrange from the patch review:

"IMHO hardcoding a specific TLS version is pretty undesirable. There is active work to enable TLS 1.3 in all crypto libraries in the very near future, so we really want choice of version to be configurable, to avoid having to make potentially bogus assumptions about which specific versions are desired.

In Fedora there is a systemwide crypto policy which controls what versions of TLS openssl uses in all apps. IIUC, the original code should honour that global policy, so if the admin turned off TLS 1.0 / 1.1 in global policy Nova would already be doing the right thing. By explicitly setting a version here, it overrides the system global defaults. IOW if those defaults requested 1.3, this proposed change will in fact cause a regression."

Sorry, I didn't mean to suggest we should abandon the change/bug, as not all distros have crypto policy support systemwide.

Rather, that we should

1. make sure the out of the box behaviour is to honour openssl defaults
2. provide a nova.conf setting for the protocol version, which allows an ordered list of versions to be set by the admin. eg might set something like vnc_tls_protocol = [ "tls1.3", "tls1.2"]

Thanks for the clarification.

With the config option list approach, I'm not sure how we would try the sequence of listed options to settle on. Does wrap_socket raise if a ssl_version is not supported? If so, then we would try calling wrap_socket with each ssl_version, trying the next version if the previous one fails, and falling through to the default after exhausting the options.

Oh right, I see there's two different "wrap_socket" methods, one at the module level (https://docs.python.org/2/library/ssl.html#ssl.wrap_socket) and one one the new SSLContext object (new in 2.7.9 / 3.2.0 https://docs.python.org/2/library/ssl.html#ssl.SSLContext.wrap_socket). Only the latter appears to provide a way to allow use of multiple versions, and because OpenSSL is so awesome, you can't list the protocols you want, rather you have to list the ones you don't want :-(

So on python < 2.7.9, we would have to raise an error if the admin had requested multiple versions in nova.conf. On Python >= 2.7.9 we would have to see what they requested, and tell openssl to disable the inverse set.

Reviewed: https://review.opendev.org/679502
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=08bdcdb5b6866c2b6bf084344cca4dd07b960133
Submitter: Zuul
Branch: master

commit 08bdcdb5b6866c2b6bf084344cca4dd07b960133
Author: Nathan Kinder <email address hidden>
Date: Fri Aug 30 12:24:03 2019 -0700

    Allow TLS ciphers/protocols to be configurable for console proxies

    The console proxies (VNC, SPICE, etc) currently don't allow the
    allowed TLS ciphers and protocol versions to be configurable. This
    results in the defaults being used from the underlying system,
    which may not be secure enough for many deployments. This patch
    allows for the ciphers and minimum SSL/TLS protocol version for
    each console proxy to be configured in nova's config.

    We utilize websockify underneath our console proxies, which added
    support for allowed ciphers and the SSL/TLS version to be
    configurable as of version 0.9.0. This change updates the lower
    constraint for this dependency.

    Closes-Bug: #1842149
    Related-Bug: #1771773
    Change-Id: I23ac1cc79482d0fabb359486a4b934463854cae5

Related fix proposed to branch: stable/train
Review: https://review.opendev.org/746798

Related fix proposed to branch: stable/stein
Review: https://review.opendev.org/746800

Change abandoned by Douglas Mendizábal (<email address hidden>) on branch: stable/stein
Review: https://review.opendev.org/746800

Change abandoned by Douglas Mendizábal (<email address hidden>) on branch: stable/train
Review: https://review.opendev.org/746798