Update development dependencies, move null byte fix to python, and ad…

…d redaction to hex dump logging.

Signed-off-by: Leonard Carcaramo <lcarcaramo@ibm.com>

pyproject.toml

@@ -44,13 +44,14 @@
     defusedxml = ">=0.7.1"
 
 [tool.poetry.group.dev.dependencies]
-    isort = ">=5.12.0"
-    pre-commit = ">=3.4.0"
-    black = ">=23.9.1"
-    flake8 = ">=6.1.0"
-    pylint = ">=3.0.0"
-    coverage = ">=7.3.2"
-    wheel = ">=0.41.2"
+    isort = ">=5.13.2"
+    pre-commit = ">=3.6.0"
+    black = ">=24.1.1"
+    flake8 = ">=7.0.0"
+    pylint = ">=3.0.3"
+    coverage = ">=7.4.1"
+    wheel = ">=0.42.0"
+    ebcdic = ">=1.1.1"
 
 [tool.isort]
     profile = "black"

pyracf/__init__.py

@@ -1,4 +1,5 @@
 """Make security admin subclasses available from package root."""
+
 from .access.access_admin import AccessAdmin
 from .common.add_operation_error import AddOperationError
 from .common.alter_operation_error import AlterOperationError

pyracf/common/downstream_fatal_error.py

@@ -1,4 +1,5 @@
 """Exception to use when IRRSMO00 is unable to process a request."""
+
 from typing import Union
 
 

pyracf/common/irrsmo00.c

@@ -14,28 +14,6 @@ typedef struct
     char running_userid[8];
 } running_userid_t;
 
-// This function changes any null character not preceded by '>' to a space character.
-// This is a workaround for an issue where profile data embedded in response xml
-// returned by IRROSMO00 sometimes includes null characters instead of properly
-// encoded text, which causes the returned xml to be truncated.
-void null_byte_fix(char *str, unsigned int length)
-{
-    for (int i = 1; i < length; i++)
-    {
-        if (str[i] == 0)
-        {
-            if (str[i - 1] == 0x6E)     // 0xfE is a '>' character in IBM-1047
-            {
-                return;
-            }
-            else
-            {
-                str[i] = 0x40;          // 0x40 is a space character in IBM-1047
-            }
-        }
-    }
-}
-
 static PyObject *call_irrsmo00(PyObject *self, PyObject *args, PyObject *kwargs)
 {
     const char *request_xml;
@@ -107,8 +85,6 @@ static PyObject *call_irrsmo00(PyObject *self, PyObject *args, PyObject *kwargs)
         response_buffer_size,
         response_buffer);
 
-    null_byte_fix(response_buffer, response_buffer_size);
-
     // https://docs.python.org/3/c-api/arg.html#c.Py_BuildValue
     //
     // According to the Python 3 C API documentation:
@@ -132,15 +108,18 @@ static PyObject *call_irrsmo00(PyObject *self, PyObject *args, PyObject *kwargs)
     // to do any memory mangement here. 'response_buffer' will simply
     // just be popped off the stack when this function returns.
     //
-    // Also, according to the Python3 C documentation 'y#' should be 
-    // just giving us a copy copy of exactly what is in the buffer,
+    // Also, according to the Python3 C API documentation, 'y#' should
+    // be just giving us a copy copy of exactly what is in the buffer,
     // without attempting to do any transformations to the data.
     // The following GeesForGeeks article futher confirms that we are
     // going to get a bytes object that is completely unmanipulated.
     // https://www.geeksforgeeks.org/c-strings-conversion-to-python/
     //
     // In this case, all post processing of the data is handled on
     // the Python side.
+    //
+    // Also note that when two or more return values are provided,
+    // Py_BuildValue() will return a Tuple.
 
     return Py_BuildValue(
         "y#BBB", 

pyracf/common/irrsmo00.py

@@ -1,6 +1,7 @@
 """Interface to irrsmo00.dll."""
+
 import platform
-from typing import Union
+from typing import Tuple, Union
 
 try:
     from cpyracf import call_irrsmo00
@@ -19,13 +20,31 @@ class IRRSMO00:
     def __init__(self, response_buffer_size=16384) -> None:
         # Initialize size of the response buffer (16 kilobytes by default)
         self.__response_buffer_size = response_buffer_size
-        self.__raw_binary_response = b""
+        self.__raw_response = b""
+
+    def get_raw_response(self) -> bytes:
+        """Get the current preserved raw response from IRRSMO00."""
+        return self.__raw_response
 
-    def get_raw_binary_response(self) -> bytes:
-        return self.__raw_binary_response
+    def clear_raw_response(self) -> None:
+        """Clear the current preserved raw response from IRRSMO00."""
+        self.__raw_response = b""
 
-    def clear_raw_binary_response(self) -> None:
-        self.__raw_binary_response = b""
+    def __null_byte_fix(response: bytes) -> bytes:
+        """
+        This function replaces all null bytes that exist before the
+        last occurance the '>' (0x6E in IBM-1047) character in the
+        response ' ' (0x40 in IBM-1047) characters.
+        This is a workaround for an issue where profile data embedded
+        in response xml returned by IRROSMO00 sometimes includes null
+        bytes instead of properly encoded text, which causes the
+        returned xml to be truncated.
+        """
+        last_greater_than = response.rfind(b"\x6e")
+        for i in range(last_greater_than):
+            if response[i] == 0:
+                response[i] = b"\x40"
+        return response
 
     def call_racf(
         self,
@@ -36,36 +55,43 @@ def call_racf(
         """Make request to call_irrsmo00 in the cpyracf Python extension."""
         irrsmo00_options = 15 if precheck else 13
         running_userid = b""
-        running_userid_length = 0
         if run_as_userid:
             running_userid = run_as_userid.encode("cp1047")
-            running_userid_length = len(run_as_userid)
-        response = call_irrsmo00(
-            request_xml=request_xml,
-            request_xml_length=len(request_xml),
-            response_buffer_size=self.__response_buffer_size,
-            irrsmo00_options=irrsmo00_options,
-            running_userid=running_userid,
-            running_userid_length=running_userid_length,
+        response = self.__call_irrsmo00_wrapper(
+            request_xml, irrsmo00_options, running_userid
         )
         # Preserve raw binary respone just in case we need to create a dump.
         # If the decoded response cannot be parsed with the XML parser,
         # a dump may need to be taken to aid in problem determination.
-        self.__raw_binary_response = response[0]
+        self.__raw_response = response[0]
+        # Replace any null bytes in the XML response with spaces.
+        response_xml = self.__null_byte_fix(response[0])
         # 'irrsmo00.c' returns a raw unmodified bytes object containing a copy
         # of the exact contents of the response xml buffer that the IRRSMO00
         # callable service populates.
         #
         # The first occurance of a null byte '0x00' is the end of the IBM-1047
         # encoded XML response and all of the trailing null bytes should be removed
         # from the XML response to ensure that downstream XML parsing is successful.
-        response_length = len(response[0])
-        null_terminator_index = response[0].find(b"\x00")
+        response_length = len(response_xml)
+        null_terminator_index = response_xml.find(b"\x00")
         if null_terminator_index != -1:
             response_length = null_terminator_index
         # If 'response_length' is 0, this indicates that the IRRSMO00 callable
         # service was unable to process the request. in this case, would should
         # only return the return and reasons codes for error handling downstream.
         if response_length == 0:
             return list(response[1:4])
-        return response[0][:response_length].decode("cp1047")
+        return response_xml[:response_length].decode("cp1047")
+
+    def __call_irrsmo00_wrapper(
+        self, request_xml: bytes, irrsmo00_options: int, running_userid: bytes
+    ) -> Tuple[bytes, int, int, int]:
+        return call_irrsmo00(
+            request_xml=request_xml,
+            request_xml_length=len(request_xml),
+            response_buffer_size=self.__response_buffer_size,
+            irrsmo00_options=irrsmo00_options,
+            running_userid=running_userid,
+            running_userid_length=len(running_userid),
+        )

pyracf/common/logger.py

@@ -4,7 +4,6 @@
 import inspect
 import json
 import os
-import platform
 import re
 import struct
 from datetime import datetime
@@ -149,17 +148,20 @@ def __redact_request_dictionary(
 
     def __redact_string(
         self,
-        input_string: str,
+        input_string: Union[str, bytes],
         start_ind: int,
-        end_pattern: str,
+        end_pattern: Union[str, bytes],
     ):
         """
         Redacts characters in a string between a starting index and ending pattern.
         Replaces the identified characters with '********' regardless of the original length.
         """
+        asterisks = "********"
+        if isinstance(input_string, bytes):
+            asterisks = asterisks.encode("cp1047")
         pre_keyword = input_string[:start_ind]
         post_keyword = end_pattern.join(input_string[start_ind:].split(end_pattern)[1:])
-        return pre_keyword + "********" + end_pattern + post_keyword
+        return pre_keyword + asterisks + end_pattern + post_keyword
 
     def redact_request_xml(
         self,
@@ -196,7 +198,7 @@ def redact_request_xml(
 
     def redact_result_xml(
         self,
-        security_response: Union[str, List[int]],
+        security_response: Union[str, bytes, List[int]],
         secret_traits: dict,
     ) -> str:
         """
@@ -209,11 +211,18 @@ def redact_result_xml(
             return security_response
         for xml_key in secret_traits.values():
             racf_key = xml_key.split(":")[1] if ":" in xml_key else xml_key
-            match = re.search(rf"{racf_key.upper()} +\(", security_response)
+            end_pattern = ")"
+            if isinstance(security_response, bytes):
+                match = re.search(
+                    rf"{racf_key.upper()} +\(", security_response.decode("cp1047")
+                )
+                end_pattern = end_pattern.encode("cp1047")
+            else:
+                match = re.search(rf"{racf_key.upper()} +\(", security_response)
             if not match:
                 continue
             security_response = self.__redact_string(
-                security_response, match.end(), ")"
+                security_response, match.end(), end_pattern
             )
         return security_response
 
@@ -346,7 +355,7 @@ def __indent_xml(self, minified_xml: str) -> str:
             indented_xml += f"{'  ' * indent_level}{current_line}\n"
         return indented_xml[:-2]
 
-    def binary_dump(self, raw_binary_response: bytes) -> str:
+    def raw_dump(self, raw_binary_response: bytes) -> str:
         """Dump raw binary response returned by IRRSMO00 to a dump file."""
 
         def opener(path: str, flags: int) -> int:
@@ -383,7 +392,9 @@ def get_timestamp(self) -> str:
         """
         return datetime.now().strftime("%Y%m%d-%H%M%S")
 
-    def log_formatted_hex_dump(self, raw_binary_response: bytes) -> None:
+    def log_formatted_hex_dump(
+        self, raw_binary_response: bytes, secret_traits: dict
+    ) -> None:
         """
         Log the raw binary response returned by IRRSMO00 as a formatted hex dump.
         """
@@ -392,14 +403,14 @@ def log_formatted_hex_dump(self, raw_binary_response: bytes) -> None:
         interpreted_row = ""
         i = 0
         hex_dump = ""
-        encoding = "cp1047"
-        if platform.system() != "OS/390" and encoding == "cp1047":
-            # If not running on z/OS, EBCDIC is most likely not supported.
-            # Force iso-8859-1 if running tests on Linux, Mac, Windows, etc...
-            encoding = "iso-8859-1"
+        # Redact secrets from raw result because we are logging it to the console.
+        raw_binary_response = self.redact_result_xml(
+            raw_binary_response,
+            secret_traits,
+        )
         for byte in raw_binary_response:
             color_function = self.__green
-            char = struct.pack("B", byte).decode(encoding)
+            char = struct.pack("B", byte).decode("cp1047")
             # Non-displayable characters should be interpreted as '.'.
             match len(repr(char)):
                 # All non-displayable characters should be red.

pyracf/common/security_admin.py

@@ -136,11 +136,15 @@ def __replace_valid_segment_traits(self, new_valid_segment_traits: dict) -> None
     # ============================================================================
     # Dump Mode
     # ============================================================================
-    def __binary_dump(self) -> None:
-        raw_binary_response = self.__irrsmo00.get_raw_binary_response()
-        self.__logger.binary_dump(raw_binary_response)
+    def __raw_dump(self) -> None:
+        raw_binary_response = self.__irrsmo00.get_raw_response()
+        self.__logger.raw_dump(raw_binary_response)
         if self.__debug:
-            self.__logger.log_formatted_hex_dump(raw_binary_response)
+            # Note, since the hex dump is logged to the console,
+            # secrets will be redacted.
+            self.__logger.log_formatted_hex_dump(
+                raw_binary_response, self.__secret_traits
+            )
 
     # ============================================================================
     # Secrets Redaction
@@ -236,16 +240,16 @@ def _make_request(
         except xml.etree.ElementTree.ParseError as e:
             # If the result XML cannot be parsed, a dump of the raw binary
             # response will be created to aid in problem determination.
-            self.__binary_dump()
-            self.__irrsmo00.clear_raw_binary_response()
+            self.__raw_dump()
+            self.__irrsmo00.clear_raw_response()
             # After creating the dump, re-raise the exception.
             raise e
         if self.__dump_mode:
             # If in dump mode a dump of the raw binary response
             # will be created even if the raw security result was
             # able to be processed successfully
-            self.__binary_dump()
-        self.__irrsmo00.clear_raw_binary_response()
+            self.__raw_dump()
+        self.__irrsmo00.clear_raw_response()
         if self.__debug:
             # No need to redact anything here since the result dictionary
             # already has secrets redacted when it is built.
@@ -628,12 +632,12 @@ def __format_user_list_data(
         profile[current_segment]["users"][user_index]["access"] = self._cast_from_str(
             user_fields[1]
         )
-        profile[current_segment]["users"][user_index][
-            "accessCount"
-        ] = self._cast_from_str(user_fields[2])
-        profile[current_segment]["users"][user_index][
-            "universalAccess"
-        ] = self._cast_from_str(user_fields[3])
+        profile[current_segment]["users"][user_index]["accessCount"] = (
+            self._cast_from_str(user_fields[2])
+        )
+        profile[current_segment]["users"][user_index]["universalAccess"] = (
+            self._cast_from_str(user_fields[3])
+        )
 
         self.__add_key_value_pairs_to_segment(
             current_segment,

pyracf/setropts/setropts_admin.py

@@ -712,10 +712,10 @@ def __add_generic_subfield(
                             subdictionary["enabled"] = self._cast_from_str(
                                 value_tokens[0]
                             )
-                            subdictionary[
-                                "options"
-                            ] = self.__process_generic_subsubfield_options(
-                                value_tokens[1]
+                            subdictionary["options"] = (
+                                self.__process_generic_subsubfield_options(
+                                    value_tokens[1]
+                                )
                             )
                         else:
                             subdictionary["enabled"] = self._cast_from_str(value_raw)