Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added graceful downgrade from TLS security enabled to disabled if a TLS CLIENT_KEY is not provided #11

Merged
merged 2 commits into from
Nov 3, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 26 additions & 21 deletions nuocd/nuodb_adminquery.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,37 +41,42 @@ def get_admin_conn(pid):
except IOError:
pass

protocol = None
address = None
api_server = env.get('NUOCMD_API_SERVER', os.environ.get('NUOCMD_API_SERVER'))
if api_server is None:
# look in /etc/nuodb/nuoadmin.conf to see if ssl is enabled.
pass

match = re.match('(https?://|)([^:]+)(:[0-9]+|)', api_server)
if match:
protocol = match.group(1)
if len(protocol) == 0:
protocol = 'https://'
adminhost = match.group(2)
port = match.group(3)
if len(port) == 0:
port = ':8888'
api_server = protocol + adminhost + port

if protocol == 'https://':
client_key = env.get('NUOCMD_CLIENT_KEY', os.environ.get('NUOCMD_CLIENT_KEY'))
if client_key:
client_key = ROOT + client_key

# this assumes set to a path. It could be set to True in which case we need
# to return the default path

server_cert = env.get('NUOCMD_VERIFY_SERVER', os.environ.get('NUOCMD_VERIFY_SERVER', False))
if server_cert and server_cert != True:
server_cert = ROOT + server_cert
else:
address = adminhost + port

client_key = env.get('NUOCMD_CLIENT_KEY', os.environ.get('NUOCMD_CLIENT_KEY'))
if client_key:
client_key = ROOT + client_key
# downgrade if protocol is unspecified and client key does not exist
if not protocol and not os.path.exists(client_key):
client_key = None

# this assumes set to a path. It could be set to True in which case we need
# to return the default path

server_cert = env.get('NUOCMD_VERIFY_SERVER', os.environ.get('NUOCMD_VERIFY_SERVER', False))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

server_cert can also be missing. In 4.2 we no longer ship either the server cert or the client cert.

A missing server_cert file is "OK" in python?

Copy link
Collaborator Author

@adriansuarez adriansuarez Nov 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should downgrade if the client key exists. If someone went through the trouble to create nuocmd.pem, but they forgot to create or misplaced (e.g. they called it /etc/nuodb/keys/ca.crt instead of /etc/nuodb/keys/ca.cert) the server certificate, then they would probably be expecting HTTPS to be used.

I'm also being more defensive about downgrading here because I can't tell if the argument was resolved from an environment variable or an explicit argument, (nuocmd only downgrades when the explicit arguments are not specified).

if server_cert and server_cert is not True:
server_cert = ROOT + server_cert

if protocol == 'http://' or (
not protocol and not client_key):
# downgrade to http://
protocol = 'http://'
client_key = None
server_cert = None
else:
protocol = 'https://'

api_server = protocol + address
nuodb_mgmt.disable_ssl_warnings()
admin_conn = nuodb_mgmt.AdminConnection(api_server, client_key=client_key, verify=server_cert)
except Exception as x:
Expand All @@ -89,7 +94,7 @@ def get_admin_conn(pid):
parser.add_option('-a',
'--api-server',
dest='api_server',
default=os.getenv('NUOCMD_API_SERVER', 'https://localhost:8888'),
default=os.getenv('NUOCMD_API_SERVER', 'localhost:8888'),
help='ADMIN url defaults to $NUOCMD_API_SERVER if set')
parser.add_option('-v',
'--verify-server',
Expand Down