-
Notifications
You must be signed in to change notification settings - Fork 24
[GT-157] Drop messages from banned dns, and add tests for messages saving to the correct queue #238
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
base: dev
Are you sure you want to change the base?
Changes from 8 commits
5452084
728b8d7
87a9067
ded3121
b3ea5fe
4024cf9
60c8f37
e47b15e
7ef624b
f632de4
6b5ef75
7243862
0cabbe5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
|
@@ -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 = [] | ||
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: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
if f is not None: | ||
f.close() | ||
|
||
return banned_dns_list | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,11 +4,19 @@ | |
import shutil | ||
import tempfile | ||
import unittest | ||
from subprocess import call | ||
from mock import patch | ||
from subprocess import call, Popen, PIPE | ||
import logging | ||
|
||
# For Python 2.7, make sure testfixtures version is < 7.0.0 | ||
# testfixtures version 6.18.5 works fine | ||
# run: pip install testfixtures==6.18.5 | ||
from testfixtures import LogCapture | ||
|
||
from ssm.message_directory import MessageDirectory | ||
from ssm.ssm2 import Ssm2, Ssm2Exception | ||
|
||
logging.basicConfig(level=logging.INFO) | ||
|
||
class TestSsm(unittest.TestCase): | ||
''' | ||
|
@@ -134,6 +142,211 @@ def test_ssm_init_non_dirq(self): | |
# 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. | ||
We then use the log output from ssm2.py to see if the messages | ||
are sent to the queues we expect them to be. | ||
''' | ||
|
||
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') | ||
|
||
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') | ||
|
||
# create a key/cert pair | ||
call(['openssl', 'req', '-x509', '-nodes', '-days', '2', | ||
'-newkey', 'rsa:2048', '-keyout', TEST_KEY_FILE, | ||
'-out', TEST_CERT_FILE, '-subj', | ||
'/C=UK/O=STFC/OU=SC/CN=Test Cert']) | ||
|
||
@patch.object(Ssm2, '_handle_msg') | ||
def test_dns_saved_to_queue(self, mock_handle_msg): | ||
''' | ||
Test that messages sent from different dns are sent | ||
to the correct queue or dropped, depending on their dn. | ||
The logging output is checked to 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 | ||
# These should be sent to the incoming 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 dns that will result in a rejected message | ||
# These should be sent to the rejected queue | ||
rejected_dns = ("/C=UK/O=eScience/OU=CLRC/L=RAL/CN=rejected-1.esc.rl.ac.uk", | ||
"/C=UK/O=eScience/OU=CLRC/L=RAL/CN=rejected-2.esc.rl.ac.uk", | ||
"/C=UK/O=eScience/OU=CLRC/L=RAL/CN=rejected-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
Check failureCode scanning / CodeQL Clear-text storage of sensitive information
This expression stores [sensitive data (certificate)](1) as clear text.
This expression stores [sensitive data (certificate)](2) as clear text.
|
||
|
||
# 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: | ||
# Capture the log output so we can use it in assertions | ||
with LogCapture() as log: | ||
print("Testing dn:", dn) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't generally be printing in tests. We only care about the output if something fails. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
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) | ||
|
||
print(str(log)) | ||
|
||
self.assertIn('Message saved to incoming queue', str(log)) | ||
|
||
print("Test Passed.\n") | ||
|
||
# As there are several different ways messages can be rejected, | ||
# Keep a count to test a different method for each dn | ||
dnCount = 1 | ||
for dn in rejected_dns: | ||
# Capture the log output so we can use it in assertions | ||
with LogCapture() as log: | ||
print("Testing dn:", dn) | ||
|
||
message_rejected = """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 | ||
%%""" | ||
|
||
# Change the reason for method rejection for each dn | ||
if dnCount == 1: | ||
# Pass no message, which will also need an error message | ||
mock_handle_msg.return_value = None, dn, "Empty text passed to _handle_msg" | ||
elif dnCount == 2: | ||
# Pass an error message | ||
mock_handle_msg.return_value = message_rejected, dn, "Signer not in valid DNs list" | ||
else: | ||
# Pass a different error message | ||
mock_handle_msg.return_value = message_rejected, dn, "Failed to verify message" | ||
|
||
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_rejected, empaid) | ||
|
||
print(str(log)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's this and the print below that also need removing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
self.assertIn('Message saved to reject queue', str(log)) | ||
|
||
print("Test Passed.\n") | ||
|
||
dnCount = dnCount + 1 | ||
|
||
# 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: | ||
# Capture the log output so we can use it in assertions | ||
with LogCapture() as log: | ||
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) | ||
|
||
print(str(log)) | ||
|
||
self.assertIn('Message dropped as was sent from a banned dn', str(log)) | ||
|
||
print("Test Passed.\n") | ||
|
||
|
||
TEST_KEY_FILE = '/tmp/test.key' | ||
|
||
TEST_CA_DIR='/tmp' | ||
|
||
TEST_CERT_FILE = '/tmp/test.crt' | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.