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

[GT-157] Drop messages from banned dns, and add tests for messages saving to the correct queue #238

Open
wants to merge 13 commits into
base: dev
Choose a base branch
from
Open
31 changes: 31 additions & 0 deletions ssm/agents.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,10 @@ def run_receiver(protocol, brokers, project, token, cp, log, dn_file):
dns = get_dns(dn_file, log)
ssm.set_dns(dns)

log.info('Fetching banned DNs.')
banned_dns = get_banned_dns(log)
ssm.set_banned_dns(banned_dns)

except Exception as e:
log.fatal('Failed to initialise SSM: %s', e)
log.info(LOG_BREAK)
Expand Down Expand Up @@ -379,3 +383,30 @@ def get_dns(dn_file, log):

log.debug('%s DNs found.', len(dns))
return dns


def get_banned_dns(log):
"""Retrieve the list of banned dns"""
banned_dns_list = []
tofu-rocketry marked this conversation as resolved.
Show resolved Hide resolved
try:
banned_dns_path = cp.get('auth', 'banned-dns')
tofu-rocketry marked this conversation as resolved.
Show resolved Hide resolved
banned_dns_file = os.path.normpath(os.path.expandvars(banned_dns_path))
except ConfigParser.NoOptionError:
banned_dns_file = None
f = None
try:
f = open(banned_dns_file, 'r')
tofu-rocketry marked this conversation as resolved.
Show resolved Hide resolved
lines = f.readlines()
for line in lines:
if line.isspace() or line.strip().startswith('#'):
continue
elif line.strip().startswith('/'):
banned_dns_list.append(line.strip())
else:
log.warning('DN in banned dns list is not in correct format: %s', line)
finally:
Copy link
Member

Choose a reason for hiding this comment

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

Now we have a context manager for the file, we can get rid of the try...finally wrapper (and the `f=None') as the file will close as soon as the handler is exited (whether normally or by exception).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if f is not None:
f.close()

return banned_dns_list

30 changes: 28 additions & 2 deletions ssm/ssm2.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
self._dest = dest

self._valid_dns = []
self._banned_dns = []
self._pidfile = pidfile

# Used to differentiate between STOMP and AMS methods
Expand Down Expand Up @@ -189,6 +190,10 @@
"""Set the list of DNs which are allowed to sign incoming messages."""
self._valid_dns = dn_list

def set_banned_dns(self, banned_dn_list):
"""Set the list of banned dns, so their messages can be dropped."""
self._banned_dns = banned_dn_list

##########################################################################
# Methods called by stomppy
##########################################################################
Expand Down Expand Up @@ -283,8 +288,10 @@
Namely:
- decrypt if necessary
- verify signature
- send an error message if the message wasn't sent from a valid DN
- Return plain-text message, signer's DN and an error/None.
"""

if text is None or text == '':
warning = 'Empty text passed to _handle_msg.'
log.warning(warning)
Expand All @@ -307,10 +314,23 @@
log.error(error)
return None, None, error

if signer not in self._valid_dns:
# If the message has been sent from a banned DN,
# set a specific error message that can be
# checked for later.
if signer in self._banned_dns:
warning = 'Signer is in the banned DNs list'
log.warning(warning)
return None, signer, warning

# Else, if the signer is not in valid DNs list,
# but also not a banned dn,
# set a specific error message
elif signer not in self._valid_dns:
warning = 'Signer not in valid DNs list: %s' % signer
log.warning(warning)
return None, signer, warning

# Else, the message has been sent from a valid DN
else:
log.info('Valid signer: %s', signer)

Expand All @@ -320,9 +340,15 @@
"""Extract message contents and add to the accept or reject queue."""
extracted_msg, signer, err_msg = self._handle_msg(body)
try:
# If the warning states the message was sent from a banned DN,
# don't send the message to the reject queue.
# Instead, drop the message (don't send it to any queue)
if err_msg == "Signer is in the banned DNs list":
log.warning("Message dropped as was sent from a banned dn: %s", signer)
rowan04 marked this conversation as resolved.
Show resolved Hide resolved

# If the message is empty or the error message is not empty
# then reject the message.
if extracted_msg is None or err_msg is not None:
elif extracted_msg is None or err_msg is not None:
if signer is None: # crypto failed
signer = 'Not available.'
elif extracted_msg is not None:
Expand Down
188 changes: 187 additions & 1 deletion test/test_ssm.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,19 @@
import shutil
import tempfile
import unittest
from subprocess import call
import dirq
Fixed Show fixed Hide fixed
import mock
Fixed Show fixed Hide fixed
from mock import patch
from subprocess import call, Popen, PIPE
import logging

from ssm.message_directory import MessageDirectory
from ssm.ssm2 import Ssm2, Ssm2Exception

logging.basicConfig(level=logging.INFO)

schema = {"body": "string", "signer": "string",
"empaid": "string?", "error": "string?"}

class TestSsm(unittest.TestCase):
'''
Expand Down Expand Up @@ -134,6 +142,184 @@
# Assert the outbound queue is of the expected type.
self.assertTrue(isinstance(ssm._outq, MessageDirectory))

class TestMsgToQueue(unittest.TestCase):
'''
Class used for testing how messages are sent to queues based
upon the DN that sent them.
The _handle_msg() function called by _save_msg_to_queue()
(the function we are testing) is being mocked, as it fails
due to client signers and certificates not matching.
It is easier to mock out the failing function instead, as it is
not the function we are testing in this test. To test
_save_msg_to_queue, we are assuming the message's certificate and
signer match.
'''

def setUp(self):
# Create temporary directory for message queues and pidfiles
self.dir_path = tempfile.mkdtemp()

"""Set up a test directory and certificates."""
self._tmp_dir = tempfile.mkdtemp(prefix='ssm')

# Below is not currently being used, and was an alternate method
# I would like to keep it until the test is working
"""
# Some functions require the hardcoded expired certificate and
# key to be files.
key_fd, self._key_path = tempfile.mkstemp(prefix='key',
dir=self._tmp_dir)
os.close(key_fd)
with open(self._key_path, 'w') as key:
key.write(TEST_KEY)

cert_fd, self._expired_cert_path = tempfile.mkstemp(prefix='cert',
dir=self._tmp_dir)
os.close(cert_fd)
with open(self._expired_cert_path, 'w') as cert:
cert.write(TEST_CERT_FILE)

valid_dn_file, self.valid_dn_path = tempfile.mkstemp(
prefix='valid', dir=self._tmp_dir)
os.close(valid_dn_file)
with open(self.valid_dn_path, 'w') as dn:
dn.write('/test/dn')

# Create a new certificate using the hardcoded key.
# The subject has been hardcoded so that the generated
# certificate subject matches the subject of the hardcoded,
# expired, certificate at the bottom of this file.
# 2 days used so that verify_cert_date doesn't think it expires soon.
call(['openssl', 'req', '-x509', '-nodes', '-days', '2', '-new',
'-key', self._key_path, '-out', TEST_CERT_FILE,
'-subj', '/C=UK/O=STFC/OU=SC/CN=Test Cert'])
"""

self._brokers = [('not.a.broker', 123)]
self._capath = '/not/a/path'
self._check_crls = False
self._pidfile = self._tmp_dir + '/pidfile'

self._listen = '/topic/test'
self._dest = '/topic/test'

self._msgdir = tempfile.mkdtemp(prefix='msgq')

@patch.object(Ssm2, '_handle_msg')
def test_banned_dns_not_saved_to_queue(self, mock_handle_msg):
'''
Test that messages sent from banned dns are dropped
and not sent to the accept or reject queue.
The logging output will tell us if the message
was sent to the correct queue, or dropped completely.
Therefore we don't need to create incoming or reject queues
'''

# create a list of fake valid dns that will send the messages
# to make sure these aren't sent to the reject queue
valid_dns = ("/C=UK/O=eScience/OU=CLRC/L=RAL/CN=valid-1.esc.rl.ac.uk",
"/C=UK/O=eScience/OU=CLRC/L=RAL/CN=valid-2.esc.rl.ac.uk",
"/C=UK/O=eScience/OU=CLRC/L=RAL/CN=valid-3.esc.rl.ac.uk")

# create a list of fake banned dns that feature in the dn list
# these should be dropped, and not sent to a queue
banned_dns = ("/C=UK/O=eScience/OU=CLRC/L=RAL/CN=banned-1.esc.rl.ac.uk",
"/C=UK/O=eScience/OU=CLRC/L=RAL/CN=banned-2.esc.rl.ac.uk",
"/C=UK/O=eScience/OU=CLRC/L=RAL/CN=banned-3.esc.rl.ac.uk")

# a custom empaid isn't necessary, and can just be 1
empaid = "1"

# Set up an openssl-style CA directory, containing the
# self-signed certificate as its own CA certificate, but with its
# name as <hash-of-subject-DN>.0.
p1 = Popen(['openssl', 'x509', '-subject_hash', '-noout'],
stdin=PIPE, stdout=PIPE, stderr=PIPE,
universal_newlines=True)

with open(TEST_CERT_FILE, 'r') as test_cert:
cert_string = test_cert.read()

hash_name, _unused_error = p1.communicate(cert_string)

self.ca_certpath = os.path.join(TEST_CA_DIR, hash_name.strip() + '.0')
with open(self.ca_certpath, 'w') as ca_cert:
ca_cert.write(cert_string)
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Show resolved Hide resolved
Dismissed Show dismissed Hide dismissed

# For each dn in the valid dns list,
# Pass it and the message to ssm and use the log output to
# make sure it was dealt with correctly
for dn in valid_dns:
print("Testing dn:", dn)

message_valid = """APEL-summary-job-message: v0.2
Site: RAL-LCG2
Month: 3
Year: 2010
GlobalUserName: """ + dn + """
VO: atlas
VOGroup: /atlas
VORole: Role=production
WallDuration: 234256
CpuDuration: 2345
NumberOfJobs: 100
%%"""

mock_handle_msg.return_value = message_valid, dn, None

ssm = Ssm2(self._brokers, self._msgdir, TEST_CERT_FILE,
TEST_KEY_FILE, dest=self._dest, listen=self._listen,
capath=self.ca_certpath)

ssm._save_msg_to_queue(message_valid, empaid)

# check the valid message hasn't been sent to the reject queue
#self.assertEquals(re_q.count(), 0)

# check the valid message reached the incoming queue
#self.assertEquals(in_q.count(), 1)

print("")

# For each dn in the banned dns list,
# Pass it and the message to ssm and use the log output to
# make sure it was dealt with correctly
for dn in banned_dns:
print("Testing dn:", dn)

message_banned = """APEL-summary-job-message: v0.2
Site: RAL-LCG2
Month: 3
Year: 2010
GlobalUserName: """ + dn + """
VO: atlas
VOGroup: /atlas
VORole: Role=production
WallDuration: 234256
CpuDuration: 2345
NumberOfJobs: 100
%%"""

mock_handle_msg.return_value = message_banned, dn, "Signer is in the banned DNs list"

ssm = Ssm2(self._brokers, self._msgdir, TEST_CERT_FILE,
TEST_KEY_FILE, dest=self._dest, listen=self._listen,
capath=self.ca_certpath)

ssm._save_msg_to_queue(message_banned, empaid)

# check the banned message hasn't been sent to the reject queue
#self.assertEquals(re_q.count(), 0)

# check the banned message hasn't been sent to the incoming queue
#self.assertEquals(in_q.count(), 0)

print("")


TEST_KEY_FILE = '/tmp/test.key'

TEST_CA_DIR='/tmp'

TEST_CERT_FILE = '/tmp/test.crt'

Expand Down
Loading