Merge branch 'dev' into 184-drop-messages-from-banned-dns

.github/workflows/build-pkgs.yml

@@ -21,7 +21,7 @@ jobs:
       run: |
         sudo apt-get update
         sudo apt-get install rpmlint
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4
       with:
         # Get all branches and tags so the latest tag can be found for VERSION
         fetch-depth: 0
@@ -51,7 +51,7 @@ jobs:
       run: rpmlint ${{ steps.rpm.outputs.rpm_dir_path }}
 
     - name: Upload artifact
-      uses: actions/[email protected].2
+      uses: actions/[email protected].3
       with:
         name: Binary and Source RPMs
         path: |

.github/workflows/codeql-analysis.yml

@@ -34,7 +34,7 @@ jobs:
 
     steps:
     - name: Checkout repository
-      uses: actions/checkout@v3
+      uses: actions/checkout@v4
 
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL

.github/workflows/docker.yml

@@ -30,7 +30,7 @@ jobs:
         # Login against a Docker registry
         # https://github.com/docker/login-action
         name: Login to ${{ env.REGISTRY }}
-        uses: docker/login-action@v2
+        uses: docker/login-action@v3
         with:
           registry: ${{ env.REGISTRY }}
           username: ${{ github.actor }}
@@ -40,15 +40,15 @@ jobs:
         # https://github.com/docker/metadata-action
         name: Extract Docker metadata
         id: meta
-        uses: docker/metadata-action@v4
+        uses: docker/metadata-action@v5
         with:
           images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}
 
       -
         # Build and push Docker image
         # https://github.com/docker/build-push-action
         name: Build and push Docker image
-        uses: docker/build-push-action@v4.1.0
+        uses: docker/build-push-action@v5.0.0
         with:
           # Only push containers to the registry on GitHub pushes,
           # not pull requests. GitHub won't let a rogue PR create a container

.github/workflows/unit-test.yml

@@ -4,14 +4,14 @@ on: [push, pull_request]
 
 jobs:
   unit-test:
-    runs-on: ubuntu-18.04
+    runs-on: ubuntu-latest
     strategy:
       fail-fast: false
       matrix:
-        python-version: ['2.x', '3.x']
+        python-version: ['3.x']
     name: Python ${{ matrix.python-version }} test
     steps:
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4
     - name: Set up Python
       uses: actions/setup-python@v4
       with:

.pre-commit-config.yaml

@@ -15,15 +15,22 @@ repos:
   - id: check-added-large-files
   - id: check-merge-conflict
   - id: check-yaml
+  - id: debug-statements
   - id: end-of-file-fixer
   - id: mixed-line-ending
     name: Force line endings to LF
     args: ['--fix=lf']
   - id: trailing-whitespace
 
 - repo: https://github.com/pre-commit/pygrep-hooks
-  rev: v1.9.0
+  rev: v1.10.0
   hooks:
   - id: python-check-mock-methods
   - id: python-no-eval
   - id: python-no-log-warn
+  - id: python-use-type-annotations
+
+# Pre-commit CI config, see https://pre-commit.ci/
+ci:
+  autofix_prs: false
+  autoupdate_schedule: quarterly

CHANGELOG

@@ -1,5 +1,16 @@
 Changelog for ssm
 =================
+* Thu Jun 29 2023 Adrian Coveney <[email protected]> - 3.3.0-1
+ - Added destination queue to the log during startup to aid troubleshooting.
+ - Added check that the config file exists to allow for better error messages.
+ - Changed dependencies to limit python-ldap below 3.4.0 and python-daemon below 2.3.0.
+ - Changed rpmbuild config to use less OS-specific dependencies.
+ - Fixed read timeouts leading to a crash.
+ - Fixed command line arguments to allow a relative file path for the DNs file.
+ - Removed the separate logging config file.
+ - Removed python-daemon as a hard requirement as only needed for receivers.
+ - Refactored a large amount of code for maintainability and security.
+
 * Wed Apr 28 2021 Adrian Coveney <[email protected]> - 3.2.1-1
  - Changed certificate and key comparison to allow both RSA and EC keys.
  - Corrected dependencies to include OpenSSL.

README.md

@@ -10,20 +10,17 @@ using the [STOMP protocol](http://stomp.github.io/) or via the ARGO Messaging Se
 Messages are signed and may be encrypted during transit.
 Persistent queues should be used to guarantee delivery.
 
-SSM is written in Python. Packages are available for RHEL 6 and 7, and
- Ubuntu Trusty.
+SSM is written in Python. Packages are available for RHEL 7, and Ubuntu Trusty.
 
 For more information about SSM, see the [EGI wiki](https://wiki.egi.eu/wiki/APEL/SSM).
 
 ## Acknowledgements
 
 <span>
-  <img alt="STFC logo" src="https://github.com/GOCDB/gocdb/raw/dev/htdocs/images/UKRI_STF_Council-Logo_Horiz-RGB_crop.png" height="57" />
-  <img alt="EU flag" src="https://github.com/GOCDB/gocdb/raw/dev/htdocs/images/eu_flag_yellow_low_150.png" height="51" />
-  <img alt="EOSC-hub logo" src="https://github.com/GOCDB/gocdb/raw/dev/htdocs/images/eosc-hub-v-web_150.png" height="57" />
+  <img alt="STFC logo" src="https://github.com/GOCDB/gocdb/raw/a3df819/htdocs/images/logos/ukri_stfc.png" height="57" />
 </span>
 
-SSM is provided by [STFC](https://stfc.ukri.org/), a part of [UK Research and Innovation](https://www.ukri.org/), and is co-funded by the [EOSC-hub](https://www.eosc-hub.eu/) project (Horizon 2020) under Grant number 777536. Licensed under the [Apache 2 License](http://www.apache.org/licenses/LICENSE-2.0).
+SSM is provided by [STFC](https://stfc.ukri.org/), a part of [UK Research and Innovation](https://www.ukri.org/). Licensed under the [Apache 2 License](http://www.apache.org/licenses/LICENSE-2.0).
 
 ## Installing the RPM
 
@@ -40,7 +37,7 @@ The Python STOMP library (N.B. versions between 3.1.1 (inclusive) and 5.0.0
 
 The Python AMS library. This is only required if you want to use AMS. See here for details on obtaining an RPM: https://github.com/ARGOeu/argo-ams-library/
 
-The Python ldap library
+The Python ldap library (N.B. versions before 3.4.0 (exclusive) are currently supported)
 * `yum install python-ldap`
 
 Optionally, the Python dirq library (N.B. this is only required if your messages
@@ -211,8 +208,8 @@ add your messages using the `add` method.
  ```
  docker run \
      -d --entrypoint ssmreceive \
-     -v /path/to/downloaded/config/sender.cfg:/etc/apel/sender.cfg \
-     -v /path/to/read/messages:/var/spool/apel/outgoing \
+     -v /path/to/downloaded/config/receiver.cfg:/etc/apel/receiver.cfg \
+     -v /path/to/read/messages:/var/spool/apel/ \
      -v /path/to/dns/file:/etc/apel/dns \
      -v /etc/grid-security:/etc/grid-security \
      -v /path/to/persistently/log:/var/log/apel \

apel-ssm.spec

@@ -4,7 +4,7 @@
 %endif
 
 Name:           apel-ssm
-Version:        3.2.1
+Version:        3.3.0
 %define releasenumber 1
 Release:        %{releasenumber}%{?dist}
 Summary:        Secure stomp messenger
@@ -100,6 +100,17 @@ rm -rf $RPM_BUILD_ROOT
 %doc %_defaultdocdir/%{name}
 
 %changelog
+* Thu Jun 29 2023 Adrian Coveney <[email protected]> - 3.3.0-1
+ - Added destination queue to the log during startup to aid troubleshooting.
+ - Added check that the config file exists to allow for better error messages.
+ - Changed dependencies to limit python-ldap below 3.4.0 and python-daemon below 2.3.0.
+ - Changed rpmbuild config to use less OS-specific dependencies.
+ - Fixed read timeouts leading to a crash.
+ - Fixed command line arguments to allow a relative file path for the DNs file.
+ - Removed the separate logging config file.
+ - Removed python-daemon as a hard requirement as only needed for receivers.
+ - Refactored a large amount of code for maintainability and security.
+
 * Wed Apr 28 2021 Adrian Coveney <[email protected]> - 3.2.1-1
  - Changed certificate and key comparison to allow both RSA and EC keys.
  - Corrected dependencies to include OpenSSL.

requirements.txt

@@ -1,6 +1,7 @@
 # Base requirements for ssm
 
 argo-ams-library
+certifi<2020.4.5.2  # Used by AMS (via requests), 2020.4.5.2 dropped support for Python 2
 stomp.py<5.0.0
 python-daemon<=2.3.0  # 2.3.1 dropped support for Python 2
 python-ldap<3.4.0  # python-ldap-3.4.0 dropped support for Python 2

scripts/ssm-build-deb.sh

@@ -16,7 +16,7 @@
 
 set -eu
 
-TAG=3.2.1-1
+TAG=3.3.0-1
 
 SOURCE_DIR=~/debbuild/source
 BUILD_DIR=~/debbuild/build

scripts/ssm-build-rpm.sh

@@ -10,7 +10,7 @@
 rpmdev-setuptree
 
 RPMDIR=/home/rpmb/rpmbuild
-VERSION=3.2.1-1
+VERSION=3.3.0-1
 SSMDIR=apel-ssm-$VERSION
 
 # Remove old sources and RPMS

ssm/__init__.py

@@ -19,7 +19,7 @@
 import logging
 import sys
 
-__version__ = (3, 2, 1)
+__version__ = (3, 3, 0)
 
 LOG_BREAK = '========================================'
 

ssm/agents.py

@@ -37,7 +37,8 @@
 
 from stomp.exception import NotConnectedException
 try:
-    from argo_ams_library import AmsConnectionException
+    from argo_ams_library import (AmsConnectionException, AmsTimeoutException,
+                                  AmsBalancerException)
 except ImportError:
     # ImportError is raised when Ssm2 initialised if AMS is requested but lib
     # not installed.
@@ -140,10 +141,6 @@ def get_ssm_args(protocol, cp, log):
     elif protocol == Ssm2.AMS_MESSAGING:
         # Then we are setting up an SSM to connect to a AMS.
 
-        # TODO: See if setting use_ssl directly in Ssm2 constructor is ok.
-        # 'use_ssl' isn't checked when using AMS (SSL is always used), but it
-        # is needed for the call to the Ssm2 constructor below.
-        use_ssl = None
         try:
             # We only need a hostname, not a port
             host = cp.get('broker', 'host')
@@ -253,6 +250,9 @@ def run_sender(protocol, brokers, project, token, cp, log):
         print('SSM failed to complete successfully.  See log file for details.')
         log.error('Unexpected exception in SSM: %s', e)
         log.error('Exception type: %s', e.__class__)
+        sender_failed = True
+    else:
+        sender_failed = False
 
     try:
         sender.close_connection()
@@ -262,6 +262,8 @@ def run_sender(protocol, brokers, project, token, cp, log):
 
     log.info('SSM has shut down.')
     log.info(LOG_BREAK)
+    if sender_failed:
+        sys.exit(1)
 
 
 def run_receiver(protocol, brokers, project, token, cp, log, dn_file):
@@ -332,7 +334,9 @@ def run_receiver(protocol, brokers, project, token, cp, log, dn_file):
                     if protocol == Ssm2.STOMP_MESSAGING:
                         ssm.send_ping()
 
-            except (NotConnectedException, AmsConnectionException) as error:
+            except (NotConnectedException, AmsConnectionException,
+                    AmsTimeoutException, AmsBalancerException) as error:
+
                 log.warning('Connection lost.')
                 log.debug(error)
                 ssm.shutdown()
@@ -349,15 +353,24 @@ def run_receiver(protocol, brokers, project, token, cp, log, dn_file):
         log.info('Received the shutdown signal: %s', e)
         ssm.shutdown()
         dc.close()
+        receiver_failed = True
     except Exception as e:
         log.error('Unexpected exception: %s', e)
         log.error('Exception type: %s', e.__class__)
         log.error('The SSM will exit.')
         ssm.shutdown()
         dc.close()
+        receiver_failed = True
+    # Currently won't run the else statement due to the while loop in the reciever
+    # Leaving here in case of future refactoring, but commented out so the unreachable
+    # code isn't flagged by tests
+    # else:
+    #   receiver_failed = False
 
     log.info('Receiving SSM has shut down.')
     log.info(LOG_BREAK)
+    if receiver_failed:
+        sys.exit(1)
 
 
 def get_dns(dn_file, log):

ssm/brokers.py

@@ -43,6 +43,8 @@ class StompBrokerGetter(object):
     def __init__(self, bdii_url):
         """Set up the LDAP connection and strings which are re-used."""
         # Set up the LDAP connection
+        logging.warning('LDAP is deprecated and will be removed in an upcoming version, '
+                        'please set host locally in SSM config.')
         log.debug('Connecting to %s...', bdii_url)
         self._ldap_conn = ldap.initialize(bdii_url)
 
@@ -99,7 +101,7 @@ def _get_broker_details(self, service_type):
         return broker_details
 
     def _broker_in_network(self, broker_id, network):
-        """Check that a GlueServiceUniqueID is part of a specified netowrk."""
+        """Check that a GlueServiceUniqueID is part of a specified network."""
         ldap_filter = '(&(GlueServiceDataKey=cluster)(GlueChunkKey=GlueServiceUniqueID=%s))' \
             % broker_id
         attrs = [self._service_data_value_key]

ssm/ssm2.py

@@ -416,8 +416,12 @@ def _send_msg_ams(self, text, msgid):
             message = AmsMessage(data=to_send,
                                  attributes={'empaid': msgid}).dict()
 
-            argo_response = self._ams.publish(self._dest, message, retry=3)
+            argo_response = self._ams.publish(self._dest, message, retry=3, timeout=10)
             return argo_response['messageIds'][0]
+        else:
+            # We ignore empty messages as there is no point sending them.
+            # (STOMP did require empty messages to keep the connection alive.)
+            return None
 
     def pull_msg_ams(self):
         """Pull 1 message from the AMS and acknowledge it."""
@@ -437,7 +441,8 @@ def pull_msg_ams(self):
 
         for msg_ack_id, msg in self._ams.pull_sub(self._listen,
                                                   messages_to_pull,
-                                                  retry=3):
+                                                  retry=3,
+                                                  timeout=10):
             # Get the AMS message id
             msgid = msg.get_msgid()
             # Get the SSM dirq id
@@ -466,7 +471,7 @@ def pull_msg_ams(self):
         # it can move the offset for the next subscription pull
         # (basically acknowledging pulled messages)
         if ackids:
-            self._ams.ack_sub(self._listen, ackids, retry=3)
+            self._ams.ack_sub(self._listen, ackids, retry=3, timeout=10)
 
     def send_ping(self):
         """Perform connection stay-alive steps.

test/test_crypto.py

@@ -17,9 +17,7 @@
     verify_cert, \
     CryptoException
 
-# Set up logging - is this necessary?
 logging.basicConfig()
-log = logging.getLogger('SSM')
 
 
 class TestEncryptUtils(unittest.TestCase):