[#650] new escaped character processing.

This fixes the approach to escaping characters from the set: [@&;=]
that iRODS historically has had problems accommodating in PAM passwords
in the past.  Presently, for iRODS 4.2 and 4.3 we only need to consider
";" and "=" as problematic characters, due to the conflict with the
use of those characters in the KVP-formatted context parameter when the
AUTH_PLUG_REQ_AN api is used.

Author: d-w-moore

README.md

@@ -559,6 +559,13 @@ variables serving as overrides:
     - Default Value: `False`
     - Environment Variable Override: `PYTHON_IRODSCLIENT_CONFIG__LEGACY_AUTH__PAM__STORE_PASSWORD_TO_ENVIRONMENT`
 
+-   Setting: Force the use of PAM_AUTH_REQUEST_AN API for entering a new PAM password into the catalog.  This API accommodates longer passwords and avoids the step of parsing a semicolon-delimited
+    "context" parameter.
+    - Dotted Name: `legacy_auth.pam.force_use_of_dedicated_pam_api`
+    - Type: `bool`
+    - Default Value: `False`
+    - Environment Variable Override: `PYTHON_IRODSCLIENT_CONFIG__LEGACY_AUTH__PAM__FORCE_USE_OF_DEDICATED_PAM_API`
+
 -   Setting: Default choice of XML parser for all new threads.
     -   Dotted Name: `connections.xml_parser_default`
     -   Type: `str`

irods/client_configuration/__init__.py

@@ -88,11 +88,12 @@ def __init__(self):
 class LegacyAuth(iRODSConfiguration):
     __slots__ = ('pam',)
     class Pam(iRODSConfiguration):
-        __slots__ = ('time_to_live_in_hours', 'password_for_auto_renew', 'store_password_to_environment')
+        __slots__ = ('time_to_live_in_hours', 'password_for_auto_renew', 'store_password_to_environment', 'force_use_of_dedicated_pam_api')
         def __init__(self):
             self.time_to_live_in_hours = 0 # -> We default to the server's TTL preference.
             self.password_for_auto_renew = ''
             self.store_password_to_environment = False
+            self.force_use_of_dedicated_pam_api = False
     def __init__(self):
         self.pam = self.Pam()
 

irods/connection.py

@@ -12,7 +12,6 @@
 from ast import literal_eval as safe_eval
 import re
 
-PAM_PW_ESC_PATTERN = re.compile(r'([@=&;])')
 
 
 from irods.message import (
@@ -471,8 +470,7 @@ def _login_pam(self):
         import irods.client_configuration as cfg
         inline_password = (self.account.authentication_scheme == self.account._original_authentication_scheme)
         time_to_live_in_hours = cfg.legacy_auth.pam.time_to_live_in_hours
-        # For certain characters in the pam password, if they need escaping with '\' then do so.
-        new_pam_password = PAM_PW_ESC_PATTERN.sub(lambda m: '\\'+m.group(1), self.account.password)
+        new_pam_password = self.account.password
         if not inline_password and cfg.legacy_auth.pam.password_for_auto_renew is not None:
             # Login using PAM password from .irodsA
             try:
@@ -487,9 +485,13 @@ def _login_pam(self):
                 # Login succeeded, so we're within the time-to-live and can return without error.
                 return
 
+        # Some characters need to be escaped for the key-value format and parser.
+        KVP_ESCAPED_CHARS = r'\;='
+        kvp_escape = lambda s:''.join((fr'\{c}' if c in KVP_ESCAPED_CHARS else c) for c in s)
+
         # Generate a new PAM password.
         ctx_user = '%s=%s' % (AUTH_USER_KEY, self.account.client_user)
-        ctx_pwd = '%s=%s' % (AUTH_PWD_KEY, new_pam_password)
+        ctx_pwd = '%s=%s' % (AUTH_PWD_KEY, kvp_escape(new_pam_password))
         ctx_ttl = '%s=%s' % (AUTH_TTL_KEY, str(time_to_live_in_hours))
 
         ctx = ";".join([ctx_user, ctx_pwd, ctx_ttl])
@@ -498,10 +500,12 @@ def _login_pam(self):
             if getattr(self,'DISALLOWING_PAM_PLAINTEXT',True):
                 raise PlainTextPAMPasswordError
 
-        # In general authentication API, a ';' and '=' in the password would be misinterpreted due to those
-        # characters' special meaning in the context string parameter.
-        use_dedicated_pam_api = len(ctx) >= MAX_NAME_LEN or \
-                                {';','='}.intersection(set(new_pam_password))
+        # Normally, we use the AUTH_PLUG_REQ_AN api (generalized to handle both PAM and GSI, as evidenced in the gsi_client_auth_request() method.)
+        # However, it has a practical limit to the number of characters in a context_ parameter (defined in packStruct as "str[MAX_NAME_LEN]").
+        # Whereas PAM_AUTH_REQUEST_AN is an older api and defines pamPassword as a "str*" entry, with apparently no length limit.
+
+        use_dedicated_pam_api = cfg.legacy_auth.pam.force_use_of_dedicated_pam_api or (
+                                len(ctx) >= MAX_NAME_LEN )
 
         if use_dedicated_pam_api:
             message_body = PamAuthRequest( pamUser = self.account.client_user,
@@ -510,10 +514,12 @@ def _login_pam(self):
         else:
             message_body = PluginAuthMessage( auth_scheme_ = PAM_AUTH_SCHEME,  context_ = ctx)
 
+        api_name = ('PAM_AUTH_REQUEST_AN' if use_dedicated_pam_api else 'AUTH_PLUG_REQ_AN')
+
         auth_req = iRODSMessage(
             msg_type='RODS_API_REQ',
             msg=message_body,
-            int_info=api_number['PAM_AUTH_REQUEST_AN' if use_dedicated_pam_api else 'AUTH_PLUG_REQ_AN']
+            int_info=api_number[api_name]
         )
 
         self.send(auth_req)
@@ -545,7 +551,7 @@ def _login_pam(self):
                 f.write(obf.encode(auth_out.result_))
                 logger.debug('new PAM pw write succeeded')
 
-        logger.info("PAM authorization validated")
+        logger.info("PAM authorization validated (via %s)", api_name)
 
     def read_file(self, desc, size=-1, buffer=None):
         if size < 0:

irods/test/scripts/test005_test_special_characters_in_passwords.bats

@@ -0,0 +1,66 @@
+#!/usr/bin/env bats
+ 
+# Test creation and use of PAM password authentication info in .irodsA when the password itself contains
+# special characters (those known to have created issues in the past).
+
+. "$BATS_TEST_DIRNAME"/test_support_functions
+PYTHON=python3
+
+# Setup/prerequisites are same as for login_auth_test.
+# Run as ubuntu user with sudo; python_irodsclient must be installed (in either ~/.local or a virtualenv)
+
+ALICES_OLD_PAM_PASSWD="test123"
+ALICES_NEW_PAM_PASSWD="new_&@;=_pass"
+
+setup()
+{
+    setup_pam_login_for_alice "$ALICES_OLD_PAM_PASSWD"
+}
+
+teardown()
+{
+    finalize_pam_login_for_alice
+    test_specific_cleanup
+}
+
+@test create_secrets_file {
+    # Old .irodsA is already created, so we delete it and alter the pam password.
+    sudo chpasswd <<<"alice:$ALICES_NEW_PAM_PASSWD"
+    local logfile i
+    for force_flexible_pam_login_api in False True; do
+        logfile=/tmp/prc_logs.$((++i))
+        sudo su - irods -c 'iadmin rpp alice'
+        rm -f ~/.irods/.irodsA
+        $PYTHON -c "import irods.client_init
+import logging.config
+logging.config.dictConfig(
+{'handlers': {'file': {'class': 'logging.FileHandler',
+                       'filename': '$logfile'}},
+ 'loggers': {'irods.connection': {'handlers': ['file'],
+                                  'level': 'INFO'}},
+ 'version': 1}
+)
+irods.client_configuration.legacy_auth.pam.force_use_of_dedicated_pam_api = $force_flexible_pam_login_api
+irods.client_init.write_pam_credentials_to_secrets_file('$ALICES_NEW_PAM_PASSWD')"
+
+        # Make sure the iinit-like routines created the catalog entry for the PAM password using the algorithm we expected it to call.
+        log_content=$(cat $logfile)
+        declare -A pam_api=([True]=PAM_AUTH_REQUEST_AN
+                           [False]=AUTH_PLUG_REQ_AN)
+        [[ $log_content =~ "PAM authorization validated (via ${pam_api[$force_flexible_pam_login_api]})" ]]
+
+        # Define the core Python to be run, basically a minimal code block ensuring that we can authenticate to iRODS
+        # without an exception being raised.
+
+        local SCRIPT="
+import irods
+import irods.test.helpers as h
+ses = h.make_session()
+ses.collections.get(h.home_collection(ses))
+print ('env_auth_scheme=%s' % ses.pool.account._original_authentication_scheme)
+"
+        OUTPUT=$($PYTHON -c "$SCRIPT")
+        # Assert passing value
+        [[ $OUTPUT = "env_auth_scheme=pam"* ]]
+    done
+}