Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions test/start-additional-kas/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,13 @@ runs:
LOG_TYPE: ${{ inputs.log-type }}
with:
run: |
# If PQC is requested but key files were not generated, the platform
# version likely does not support PQC key types. Disable PQC to avoid
# referencing missing key files in the config.
if [ "${PQC_ENABLED}" == "true" ] && [ ! -f kas-xwing-private.pem ]; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In POSIX standard [ (test) command, the equality operator is = rather than ==. While bash supports == inside [, some environments or runners might execute this step using sh (which defaults to dash on Ubuntu), leading to a syntax error like [: unexpected operator. Using = ensures maximum compatibility and robustness across different shell environments.

          if [ "${PQC_ENABLED}" = "true" ] && [ ! -f kas-xwing-private.pem ]; then

echo "::warning::PQC enabled but key files not found (platform version may not support PQC). Disabling PQC key configuration for KAS ${KAS_NAME}."
export PQC_ENABLED="false"
fi
Comment on lines +151 to +154

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard all required PQC key files, not only xwing.

This check only tests kas-xwing-private.pem, but the PQC branch writes config entries for x1, h1, and h2 (six PEM files). A partial key set will still produce broken config and can fail startup.

Suggested fix
-          if [ "${PQC_ENABLED}" == "true" ] && [ ! -f kas-xwing-private.pem ]; then
-            echo "::warning::PQC enabled but key files not found (platform version may not support PQC). Disabling PQC key configuration for KAS ${KAS_NAME}."
-            export PQC_ENABLED="false"
-          fi
+          if [ "${PQC_ENABLED}" == "true" ]; then
+            required_pqc_files=(
+              kas-xwing-private.pem kas-xwing-public.pem
+              kas-p256mlkem768-private.pem kas-p256mlkem768-public.pem
+              kas-p384mlkem1024-private.pem kas-p384mlkem1024-public.pem
+            )
+            missing=0
+            for f in "${required_pqc_files[@]}"; do
+              [ -f "$f" ] || missing=1
+            done
+            if [ "$missing" -eq 1 ]; then
+              echo "::warning::PQC enabled but required PQC key files are incomplete. Disabling PQC key configuration for KAS ${KAS_NAME}."
+              export PQC_ENABLED="false"
+            fi
+          fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ "${PQC_ENABLED}" == "true" ] && [ ! -f kas-xwing-private.pem ]; then
echo "::warning::PQC enabled but key files not found (platform version may not support PQC). Disabling PQC key configuration for KAS ${KAS_NAME}."
export PQC_ENABLED="false"
fi
if [ "${PQC_ENABLED}" == "true" ]; then
required_pqc_files=(
kas-xwing-private.pem kas-xwing-public.pem
kas-p256mlkem768-private.pem kas-p256mlkem768-public.pem
kas-p384mlkem1024-private.pem kas-p384mlkem1024-public.pem
)
missing=0
for f in "${required_pqc_files[@]}"; do
[ -f "$f" ] || missing=1
done
if [ "$missing" -eq 1 ]; then
echo "::warning::PQC enabled but required PQC key files are incomplete. Disabling PQC key configuration for KAS ${KAS_NAME}."
export PQC_ENABLED="false"
fi
fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/start-additional-kas/action.yaml` around lines 151 - 154, The current
PQC presence check only verifies kas-xwing-private.pem; update the conditional
that uses PQC_ENABLED and the file-test to require all six expected PEM files
(kas-xwing-private.pem, kas-xwing-public.pem, kas-x1-private.pem,
kas-x1-public.pem, kas-h1-private.pem, kas-h1-public.pem) exist before keeping
PQC_ENABLED="true". Change the if-test (the block referencing PQC_ENABLED and
kas-xwing-private.pem) to verify every listed file, and adjust the echo warning
to list that required key files are missing and then export PQC_ENABLED="false"
when any are absent so partial key sets don’t produce broken config for
KAS_NAME.

yq e '
(.server.port = env(KAS_PORT))
| (.mode = ["kas"])
Expand Down
12 changes: 11 additions & 1 deletion test/start-up-with-containers/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,13 @@ runs:
allowed_algorithms=(ec:secp256r1 rsa:2048)
if echo $PLATFORM_VERSION | awk -F. '{ if ($1 > 0 || ($1 == 0 && $2 > 7) || ($1 == 0 && $2 == 7 && $3 >= 1)) exit 0; else exit 1; }'; then
# For versions 0.7.1 and later, we allow rsa:4096 ec:secp384r1 ec:secp521r1
allowed_algorithms+=(rsa:4096 ec:secp384r1 ec:secp521r1 hpqt:xwing hpqt:secp256r1-mlkem768 hpqt:secp384r1-mlkem1024)
allowed_algorithms+=(rsa:4096 ec:secp384r1 ec:secp521r1)
# Only allow PQC algorithms if the platform keygen produced the key files
if [ -f kas-xwing-private.pem ]; then
allowed_algorithms+=(hpqt:xwing hpqt:secp256r1-mlkem768 hpqt:secp384r1-mlkem1024)
else
echo "::notice::PQC key files not found; hpqt algorithms will not be allowed for extra keys"
fi
fi
keyring='[{"kid":"ec1","alg":"ec:secp256r1"},{"kid":"r1","alg":"rsa:2048"}]'
keys='[{"kid":"e1","alg":"ec:secp256r1","private":"kas-ec-private.pem","cert":"kas-ec-cert.pem"},{"kid":"ec1","alg":"ec:secp256r1","private":"kas-ec-private.pem","cert":"kas-ec-cert.pem"},{"kid":"r1","alg":"rsa:2048","private":"kas-private.pem","cert":"kas-cert.pem"}]'
Expand Down Expand Up @@ -245,6 +251,10 @@ runs:
shell: bash
if: ${{ inputs.pqc-enabled == 'true' }}
run: |
if [ ! -f kas-xwing-private.pem ]; then
echo "::warning::PQC enabled but key files not found (platform version may not support PQC). Skipping PQC key configuration."
exit 0
fi
yq e '
(.services.kas.preview.hybrid_tdf_enabled = true)
| (.services.kas.keyring += [{"kid":"x1","alg":"hpqt:xwing"},{"kid":"h1","alg":"hpqt:secp256r1-mlkem768"},{"kid":"h2","alg":"hpqt:secp384r1-mlkem1024"}])
Expand Down
Loading