Fixed GH code scanning issues (#293)

- Pinned remaining actions/checkout by hash
- Pinned dist/sgx Dockerfile base image by hash
- Pinned firmware/test/package Dockerfile base image by hash
- Pinned mware's docker cython-hidapi package by commit hash
- Removed self.expected overwriting from RawCommand test command, updated usage
- Using cls for class variable name in all class methods
- Fixed unused import in certificate.py by explicitly exporting modules, removed exception from linter
- Removed unwanted comma from attestation verification script's header regexp
- Made string concatenation explicit in lists

.github/workflows/run-tests.yml

@@ -94,7 +94,7 @@ jobs:
 
     steps:
       - name: Checkout rsk-powhsm repo
-        uses: actions/checkout@v4
+        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 #v4.2.2
         with:
           path: rsk-powhsm
 
@@ -111,7 +111,7 @@ jobs:
               0x6E regtest
 
       - name: Checkout hsm-integration-test repo
-        uses: actions/checkout@v4
+        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 #v4.2.2
         with:
           repository: rootstock/hsm-integration-test
           ref: 5.3.0.plus

dist/sgx/Dockerfile

@@ -1,4 +1,4 @@
-FROM debian:bookworm-slim
+FROM debian:bookworm-slim@sha256:f70dc8d6a8b6a06824c92471a1a258030836b26b043881358b967bf73de7c5ab
 
 WORKDIR /hsm2
 

docker/mware/Dockerfile

@@ -21,4 +21,5 @@ RUN pip install -r requirements.txt --require-hashes
 RUN rm -f requirements.txt
 
 # Hidapi wrapper with hid_exit support
-RUN pip install --use-pep517 git+https://github.com/rsksmart/[email protected]
+# https://github.com/rsksmart/cython-hidapi/releases/tag/0.10.1.post2 pinned by hash
+RUN pip install --use-pep517 git+https://github.com/rsksmart/cython-hidapi@56d87e287e4da8fc379e3b96573f5ac93e02e613

firmware/test/cases/raw_command.py

@@ -34,52 +34,33 @@ def op_name(cls):
         return "rawCommand"
 
     def __init__(self, spec):
+        super().__init__(spec)
+
         if not is_nonempty_hex_string(spec.get("command")) and spec.get("command") != "":
             raise TestCaseError(f"Invalid raw command: {spec.get("command")}")
         self.command = bytes.fromhex(spec.get("command"))
 
-        # Override default constructor behavior for expectation
-        self.expected = spec.get("expected")
-        spec["expected"] = True
-
-        super().__init__(spec)
-
-        # Normal result expectation
-        if not is_nonempty_hex_string(self.expected):
-            self.expected = None
-        else:
-            self.expected = bytes.fromhex(self.expected)
-
-        # Exception expectation
-        self.exception = spec.get("exception")
-        if self.exception is not None:
-            self.exception_desc = self.exception
-            self.exception = self._parse_int(self.exception)
-
     def run(self, dongle, debug, run_args):
         try:
-            result = dongle.dongle.exchange(self.command)
-            if self.expected is not None and self.expected != result:
-                raise TestCaseError("Unexpected raw command result: "
-                                    f"expected {self.expected.hex()} but got "
-                                    f"{result.hex()}")
-            if self.exception is not None:
-                raise TestCaseError(f"Expected command to raise an {self.exception_desc} "
-                                    f"exception but got 0x{result.hex()} as "
-                                    "result instead")
+            dongle.dongle.exchange(self.command)
+
+            if self.expected is not True:
+                raise TestCaseError(f"Expected error code {self.expected_desc} "
+                                    "but got a successful response")
         except TestCaseError as e:
             raise e
-        except (RuntimeError, CommException) as e:
-            if self.exception is not None:
-                if type(e) == CommException:
-                    error_code = e.sw
-                    error_code_desc = hex(error_code)
-                else:
-                    error_code = None
-                    error_code_desc = "no error"
+        except CommException as e:
+            error_code = e.sw
+            error_code_desc = hex(error_code)
+
+            if self.expected is True:
+                raise TestCaseError("Expected a successful response but got "
+                                    f"error code {error_code_desc}")
+
+            if self.expected != error_code:
+                raise TestCaseError(f"Expected error code {self.expected_desc} "
+                                    f"but got {error_code_desc}")
 
-                if self.exception != error_code:
-                    raise TestCaseError("Expected error code {self.exception_desc} "
-                                        f"but got {error_code_desc}")
-            else:
-                raise TestCaseError(str(e))
+            # All good, expected error code
+        except RuntimeError as e:
+            raise TestCaseError(str(e))

firmware/test/package/Dockerfile

@@ -1 +1 @@
-FROM python:3.12-slim-bookworm
+FROM python:3.12-slim-bookworm@sha256:69ce3aed05675d284bee807e7c45e560e98db21fb1e4c670252b4ee0f2496b6d

firmware/test/resources/900-larger-than-apdu-command-fails.json

@@ -2,6 +2,6 @@
     "name": "Larger than APDU command fails",
     "operation": "rawCommand",
     "command": "804300aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
-    "exception": "0x6982",
+    "expected": "0x6982",
     "runOn": ["tcpsigner", "sgx"]
 }

firmware/test/resources/901-empty-apdu-command-fails.json

@@ -2,6 +2,6 @@
     "name": "Empty APDU command fails",
     "operation": "rawCommand",
     "command": "",
-    "exception": "0x6982",
+    "expected": "0x6982",
     "runOn": ["tcpsigner", "sgx"]
 }

middleware/admin/attestation_utils.py

@@ -46,8 +46,8 @@ class PowHsmAttestationMessage(CStruct):
     HEADER_REGEX = re.compile(b"^POWHSM:(5.[0-9])::")
 
     @classmethod
-    def is_header(kls, value):
-        return kls.HEADER_REGEX.match(value) is not None
+    def is_header(cls, value):
+        return cls.HEADER_REGEX.match(value) is not None
 
     def __init__(self, value, offset=0, little=True, name="powHSM"):
         self.name = name

middleware/admin/certificate.py

@@ -31,3 +31,10 @@
     1: HSMCertificate,
     2: HSMCertificateV2,
 }
+
+# Exports
+__all__ = (
+    HSMCertificate, HSMCertificateRoot, HSMCertificateElement, HSMCertificateV2,
+    HSMCertificateV2ElementSGXQuote, HSMCertificateV2ElementSGXAttestationKey,
+    HSMCertificateV2ElementX509,
+)

middleware/admin/certificate_v1.py

@@ -150,7 +150,7 @@ class HSMCertificate:
     ELEMENT_FACTORY = HSMCertificateElement
 
     @classmethod
-    def from_jsonfile(kls, path):
+    def from_jsonfile(cls, path):
         try:
             with open(path, "r") as file:
                 certificate_map = json.loads(file.read())
@@ -160,12 +160,12 @@ def from_jsonfile(kls, path):
                     "Certificate file must contain an object as a top level element")
 
             version = certificate_map.get("version")
-            if version not in kls.VERSION_MAPPING:
+            if version not in cls.VERSION_MAPPING:
                 raise ValueError("Invalid or unsupported HSM certificate "
                                  f"version {version} (supported versions are "
-                                 f"{", ".join(kls.VERSION_MAPPING.keys())})")
+                                 f"{", ".join(cls.VERSION_MAPPING.keys())})")
 
-            return kls.VERSION_MAPPING[version](certificate_map)
+            return cls.VERSION_MAPPING[version](certificate_map)
         except (ValueError, json.JSONDecodeError) as e:
             raise ValueError('Unable to read HSM certificate from "%s": %s' %
                              (path, str(e)))

middleware/admin/certificate_v2.py

@@ -39,12 +39,12 @@ def __init__(self):
         raise NotImplementedError("Cannot instantiate a HSMCertificateV2Element")
 
     @classmethod
-    def from_dict(kls, element_map):
-        if element_map.get("type") not in kls.TYPE_MAPPING:
+    def from_dict(cls, element_map):
+        if element_map.get("type") not in cls.TYPE_MAPPING:
             raise ValueError("Invalid or missing element type for "
                              f"element {element_map.get("name")}")
 
-        return kls.TYPE_MAPPING[element_map["type"]](element_map)
+        return cls.TYPE_MAPPING[element_map["type"]](element_map)
 
     def _init_with_map(self, element_map):
         if "name" not in element_map:
@@ -217,16 +217,16 @@ class HSMCertificateV2ElementX509(HSMCertificateV2Element):
     HEADER_END = "-----END CERTIFICATE-----"
 
     @classmethod
-    def from_pemfile(kls, pem_path, name, signed_by):
-        return kls.from_pem(Path(pem_path).read_text(), name, signed_by)
+    def from_pemfile(cls, pem_path, name, signed_by):
+        return cls.from_pem(Path(pem_path).read_text(), name, signed_by)
 
     @classmethod
-    def from_pem(kls, pem_str, name, signed_by):
-        return kls({
+    def from_pem(cls, pem_str, name, signed_by):
+        return cls({
             "name": name,
             "message": re.sub(r"[\s\n\r]+", " ", pem_str)
-                         .replace(kls.HEADER_END, "")
-                         .replace(kls.HEADER_BEGIN, "")
+                         .replace(cls.HEADER_END, "")
+                         .replace(cls.HEADER_BEGIN, "")
                          .strip().encode(),
             "signed_by": signed_by,
         })

middleware/admin/verify_ledger_attestation.py

@@ -28,8 +28,8 @@
 from .certificate import HSMCertificate, HSMCertificateRoot
 
 
-UI_MESSAGE_HEADER_REGEX = re.compile(b"^HSM:UI:([2,3,4,5].[0-9])")
-SIGNER_LEGACY_MESSAGE_HEADER_REGEX = re.compile(b"^HSM:SIGNER:([2,3,4,5].[0-9])")
+UI_MESSAGE_HEADER_REGEX = re.compile(b"^HSM:UI:([2345].[0-9])")
+SIGNER_LEGACY_MESSAGE_HEADER_REGEX = re.compile(b"^HSM:SIGNER:([2345].[0-9])")
 UI_DERIVATION_PATH = "m/44'/0'/0'/0/0"
 UD_VALUE_LENGTH = 32
 PUBLIC_KEYS_HASH_LENGTH = 32

middleware/comm/platform.py

@@ -42,24 +42,24 @@ class Platform:
     _options = None
 
     @classmethod
-    def set(klass, plf, options={}):
-        if plf not in klass.VALID_PLATFORMS:
+    def set(cls, plf, options={}):
+        if plf not in cls.VALID_PLATFORMS:
             raise RuntimeError("Invalid platform given")
-        klass._platform = plf
-        klass._options = options
+        cls._platform = plf
+        cls._options = options
 
     @classmethod
-    def is_ledger(klass):
-        return klass._platform == Platform.LEDGER
+    def is_ledger(cls):
+        return cls._platform == Platform.LEDGER
 
     @classmethod
-    def is_sgx(klass):
-        return klass._platform == Platform.SGX
+    def is_sgx(cls):
+        return cls._platform == Platform.SGX
 
     @classmethod
-    def options(klass, key):
-        return klass._options[key]
+    def options(cls, key):
+        return cls._options[key]
 
     @classmethod
-    def message(klass, key):
-        return klass.MESSAGES[klass._platform][key]
+    def message(cls, key):
+        return cls.MESSAGES[cls._platform][key]

middleware/tests/admin/test_verify_ledger_attestation.py

@@ -179,14 +179,14 @@ def test_verify_attestation(self,
             ["Signer verified with public keys:"] + self.expected_pubkeys_output + [
                 f"Hash: {self.pubkeys_hash.hex()}",
                 "",
-                "Installed Signer hash: ffffffffffffffffffffffffffffffffffffffffffffffff"
-                "ffffffffffffffff",
+                "Installed Signer hash: ffffffffffffffffffffffffffffffffffffffffffff" +
+                "ffffffffffffffffffff",
                 "Installed Signer version: 5.4",
                 "Platform: plf",
-                "UD value: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
-                "aaa",
-                "Best block: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
-                "bbbbb",
+                "UD value: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +
+                "aaaaaaa",
+                "Best block: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" +
+                "bbbbbbbbb",
                 "Last transaction signed: cccccccccccccccc",
                 "Timestamp: 171",
             ],

setup.cfg

@@ -24,7 +24,6 @@ indent-size = 4
 # Ignore unused imports in __init__.py files
 per-file-ignores = 
     __init__.py:F401,
-    middleware/admin/certificate.py:F401,
     middleware/tests/sgx/test_envelope.py:E122,
     middleware/tests/admin/test_certificate_v2_resources.py:E501,
     middleware/tests/admin/test_attestation_utils_resources.py:E501,