fix: detect PQC keys via PKCS#8 OID + parity test gaps (#97)

pki-client-output's SanEntry is a serde-tagged enum, so `pki cert show -f
json` emits `{"Dns":"x"}` in the SAN array. The openssl parity script was
asserting on the flat "DNS:x" form produced by the Display impl, so every
SAN assertion diverged. Normalize via jq (`to_entries | key+:+value`,
uppercasing Dns/Ip to DNS/IP).

Subject DN assertions broke under openssl 3.5+ which drops spaces around
`=`. Normalize both sides with `sed 's/ = /=/g'` before matching.

EKU assertions used openssl's short names (serverAuth/clientAuth); pki
emits RFC 5280 long-form labels ("TLS Web Server Authentication"). Assert
on the long form — that's the canonical text, not the openssl shorthand.

Issue #97: ML-DSA/SLH-DSA private keys were misreported as "EC P-256,
256 bits, Strong" because spork-core emitted raw seeds and the detector
fell through to size-based heuristics. Fix at the source:

  * vendor/spork-core/src/algo/pqc_pkcs8.rs — RFC 5958 wrap/unwrap helpers.
  * mldsa_impl + slhdsa_impl now export PKCS#8 PrivateKeyInfo with the
    correct AlgorithmIdentifier OID; `from_pkcs8_der` transparently
    extracts the raw seed (and still accepts legacy raw-seed blobs for
    backward compat with keys on disk).
  * crates/pki-client/src/compat.rs — PKCS#8 OID detection runs first,
    routing PQC OIDs to `KeyAlgorithm::MlDsa` / `SlhDsa`. Also wires
    build_with_key through spork-core's PQC AlgorithmIds so `pki csr
    create` no longer errors on PQC keys.

Plumbing:
  * Box the ML-DSA-65/87 round-trip test keys — debug builds overflowed
    the test stack holding three ExpandedSigningKeys simultaneously.
  * crates/pki-client/tests/key_show_algorithms.rs — regression guard:
    generate each algorithm, assert detector output, specifically catch
    any PQC→EcP256 misidentification.
  * .github/workflows/interop.yml — openssl-parity job + broader PR
    path triggers so parity regressions block merges.

Co-Authored-By: Claude <noreply@anthropic.com>

Author: rayketcham

.github/workflows/interop.yml

@@ -7,11 +7,13 @@ on:
   pull_request:
     branches: [main]
     paths:
-      - 'crates/pki-client/src/commands/probe.rs'
+      - 'crates/pki-client/**'
+      - 'crates/pki-client-output/**'
       - 'crates/pki-probe/**'
       - 'vendor/spork-core/**'
       - 'tests/interop/**'
       - '**/Cargo.toml'
+      - '.github/workflows/interop.yml'
 
 env:
   CARGO_TERM_COLOR: always
@@ -100,3 +102,28 @@ jobs:
         env:
           PKI_BIN: ./pki
         run: bash tests/interop/cert_roundtrip.sh
+
+  openssl-parity:
+    name: OpenSSL Parity
+    needs: [build]
+    runs-on: [self-hosted, Linux]
+    timeout-minutes: 10
+    steps:
+      - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # v6.0.2
+        with:
+          persist-credentials: false
+      - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c  # v8
+        with:
+          name: pki-binary
+      - name: Prepare binary
+        run: chmod +x pki
+      - name: Install openssl + jq
+        run: |
+          if ! command -v jq >/dev/null; then
+            sudo apt-get update
+            sudo apt-get install -y jq openssl
+          fi
+      - name: Run openssl ↔ pki parity tests
+        env:
+          PKI_BIN: ./pki
+        run: bash tests/interop/openssl_parity.sh

crates/pki-client/src/compat.rs

@@ -264,6 +264,116 @@ fn is_pkcs8_private_key(data: &[u8]) -> bool {
     body[3] == 0x30
 }
 
+/// Decode a DER OID's content bytes into a dotted string (e.g. "1.2.840.113549.1.1.1").
+/// Input is the OID *content* — not including the 0x06 tag or length.
+fn decode_oid_bytes(oid: &[u8]) -> Option<String> {
+    let first = *oid.first()?;
+    let mut parts = Vec::with_capacity(8);
+    parts.push(format!("{}", first / 40));
+    parts.push(format!("{}", first % 40));
+    let mut acc: u64 = 0;
+    for &b in &oid[1..] {
+        acc = acc.checked_shl(7)?.checked_add((b & 0x7F) as u64)?;
+        if b & 0x80 == 0 {
+            parts.push(acc.to_string());
+            acc = 0;
+        }
+    }
+    Some(parts.join("."))
+}
+
+/// Extract the AlgorithmIdentifier OID (dotted form) from a PKCS#8 PrivateKeyInfo.
+fn extract_pkcs8_oid(der: &[u8]) -> Option<String> {
+    if !is_pkcs8_private_key(der) {
+        return None;
+    }
+    // Skip outer SEQUENCE tag + length → body of PrivateKeyInfo
+    let body = skip_der_tag_and_length(der)?;
+    // Skip version INTEGER(0): 02 01 00 (3 bytes)
+    if body.len() < 4 {
+        return None;
+    }
+    let after_version = &body[3..];
+    // Expect AlgorithmIdentifier SEQUENCE (tag 0x30)
+    if *after_version.first()? != 0x30 {
+        return None;
+    }
+    let alg_body = skip_der_tag_and_length(after_version)?;
+    // First field is the OID
+    if *alg_body.first()? != 0x06 {
+        return None;
+    }
+    let oid_len = *alg_body.get(1)? as usize;
+    if oid_len == 0 || oid_len >= 0x80 {
+        return None;
+    }
+    let oid_content = alg_body.get(2..2 + oid_len)?;
+    decode_oid_bytes(oid_content)
+}
+
+/// Decode PEM → DER bytes. Returns None on parse errors or non-PEM input.
+fn pem_to_der(pem_str: &str) -> Option<Vec<u8>> {
+    pem::parse(pem_str).ok().map(|p| p.contents().to_vec())
+}
+
+/// Identify a PKCS#8 private key's algorithm by parsing its AlgorithmIdentifier OID.
+/// Returns the `KeyAlgorithm` variant plus a reasonable bit-count hint for display.
+/// Accepts either PEM or raw DER input as text.
+fn detect_pkcs8_algorithm(pem_or_der: &str) -> Option<(KeyAlgorithm, u32)> {
+    let der = pem_to_der(pem_or_der)?;
+    let oid = extract_pkcs8_oid(&der)?;
+    match oid.as_str() {
+        // Classical
+        "1.2.840.113549.1.1.1" => {
+            // rsaEncryption — size must be inferred from the inner RSAPrivateKey;
+            // fall back to PEM size heuristic here since we only need a hint.
+            let bits = rsa_bits_from_pem_len(pem_or_der.len());
+            Some((KeyAlgorithm::Rsa(bits), bits))
+        }
+        "1.2.840.10045.2.1" => {
+            // ecPublicKey — curve is a nested OID in the AlgorithmIdentifier parameters.
+            // Check the PEM text for the two curves we support.
+            if pem_or_der.contains("BgUrgQQAIg") || pem_or_der.contains("GBSuBBAAi") {
+                Some((KeyAlgorithm::EcP384, 384))
+            } else {
+                Some((KeyAlgorithm::EcP256, 256))
+            }
+        }
+        "1.3.101.112" => Some((KeyAlgorithm::Ed25519, 256)),
+        "1.3.101.113" => Some((KeyAlgorithm::Ed448, 456)),
+
+        // PQC — ML-DSA (FIPS 204). `bits` reports the claimed classical-equivalent
+        // security level so the generic "Size: N bits" UI stays meaningful.
+        #[cfg(feature = "pqc")]
+        "2.16.840.1.101.3.4.3.17" => Some((KeyAlgorithm::MlDsa(44), 128)),
+        #[cfg(feature = "pqc")]
+        "2.16.840.1.101.3.4.3.18" => Some((KeyAlgorithm::MlDsa(65), 192)),
+        #[cfg(feature = "pqc")]
+        "2.16.840.1.101.3.4.3.19" => Some((KeyAlgorithm::MlDsa(87), 256)),
+
+        // PQC — SLH-DSA (FIPS 205)
+        #[cfg(feature = "pqc")]
+        "2.16.840.1.101.3.4.3.20" => Some((KeyAlgorithm::SlhDsa("SHA2-128s".to_string()), 128)),
+        #[cfg(feature = "pqc")]
+        "2.16.840.1.101.3.4.3.22" => Some((KeyAlgorithm::SlhDsa("SHA2-192s".to_string()), 192)),
+        #[cfg(feature = "pqc")]
+        "2.16.840.1.101.3.4.3.24" => Some((KeyAlgorithm::SlhDsa("SHA2-256s".to_string()), 256)),
+
+        _ => None,
+    }
+}
+
+/// Heuristic: infer RSA key size from PKCS#8 PEM character length.
+fn rsa_bits_from_pem_len(len: usize) -> u32 {
+    if len > 3000 {
+        4096
+    } else if len > 2200 {
+        3072
+    } else {
+        2048
+    }
+}
+
 /// Skip the DER tag and length bytes, returning the body.
 fn skip_der_tag_and_length(data: &[u8]) -> Option<&[u8]> {
     if data.len() < 2 {
@@ -1724,11 +1834,22 @@ impl CsrBuilder {
                 ));
             }
             #[cfg(feature = "pqc")]
-            KeyAlgorithm::MlDsa(_) | KeyAlgorithm::SlhDsa(_) => {
-                return Err(anyhow!(
-                    "ML-DSA/SLH-DSA not yet supported for CSR generation."
-                ));
+            KeyAlgorithm::MlDsa(44) => spork_core::AlgorithmId::MlDsa44,
+            #[cfg(feature = "pqc")]
+            KeyAlgorithm::MlDsa(65) => spork_core::AlgorithmId::MlDsa65,
+            #[cfg(feature = "pqc")]
+            KeyAlgorithm::MlDsa(87) => spork_core::AlgorithmId::MlDsa87,
+            #[cfg(feature = "pqc")]
+            KeyAlgorithm::MlDsa(level) => {
+                return Err(anyhow!("Unsupported ML-DSA level: {level}"));
             }
+            #[cfg(feature = "pqc")]
+            KeyAlgorithm::SlhDsa(ref variant) => match variant.as_str() {
+                "SHA2-128s" => spork_core::AlgorithmId::SlhDsaSha2_128s,
+                "SHA2-192s" => spork_core::AlgorithmId::SlhDsaSha2_192s,
+                "SHA2-256s" => spork_core::AlgorithmId::SlhDsaSha2_256s,
+                other => return Err(anyhow!("Unsupported SLH-DSA variant: {other}")),
+            },
         };
 
         // Load key pair from PEM
@@ -1943,6 +2064,29 @@ pub fn load_private_key(path: &Path) -> Result<PrivateKey> {
     // Check if encrypted
     let encrypted = data.contains("ENCRYPTED");
 
+    // First, try the authoritative path: decode PKCS#8 DER and match the
+    // AlgorithmIdentifier OID. This is the only way to reliably distinguish
+    // ML-DSA / SLH-DSA (FIPS 204/205) from classical keys, since PQC PKCS#8
+    // containers can be smaller than classical RSA PEMs and otherwise land
+    // in the size-based RSA/EC heuristics below. Issue #97.
+    if !encrypted {
+        if let Some((algo, bits_hint)) = detect_pkcs8_algorithm(&data) {
+            let curve = match algo {
+                KeyAlgorithm::EcP256 => Some("P-256".to_string()),
+                KeyAlgorithm::EcP384 => Some("P-384".to_string()),
+                _ => None,
+            };
+            return Ok(PrivateKey {
+                algorithm: algo,
+                key_size: Some(bits_hint),
+                curve,
+                pem: data,
+                bits: bits_hint,
+                encrypted,
+            });
+        }
+    }
+
     // Detect key type from PEM header and content
     // For PKCS#8 keys, we need to check the base64-encoded OID markers:
     // - P-256: contains "BggqhkjOPQMBBw" (OID 1.2.840.10045.3.1.7 = secp256r1)

crates/pki-client/tests/key_show_algorithms.rs

@@ -0,0 +1,107 @@
+//! Algorithm-detection regression tests for `pki key show`.
+//!
+//! Guards against issue #97, where ML-DSA / SLH-DSA keys were misreported as
+//! "EC P-256" because the detector fell through to size-based heuristics when
+//! the PKCS#8 OID was an unrecognised PQC identifier.
+
+use std::path::PathBuf;
+use std::process::Command;
+use tempfile::TempDir;
+
+fn pki_binary() -> PathBuf {
+    let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+    path.pop();
+    path.pop();
+    path.push("target");
+    path.push("debug");
+    path.push("pki");
+    path
+}
+
+fn binary_exists() -> bool {
+    pki_binary().exists()
+}
+
+fn gen_and_show(alg: &str) -> Option<(String, String)> {
+    if !binary_exists() {
+        return None;
+    }
+    let tmp = TempDir::new().ok()?;
+    let key_path = tmp.path().join(format!("{alg}.key"));
+    let keygen = Command::new(pki_binary())
+        .args(["key", "gen", alg, "-o"])
+        .arg(&key_path)
+        .output()
+        .ok()?;
+    if !keygen.status.success() {
+        let stderr = String::from_utf8_lossy(&keygen.stderr);
+        if stderr.contains("Unknown algorithm") || stderr.contains("not supported") {
+            return None; // pqc feature off → skip
+        }
+        panic!("keygen {alg} failed: {stderr}");
+    }
+    let show = Command::new(pki_binary())
+        .args(["key", "show", "-f", "json"])
+        .arg(&key_path)
+        .output()
+        .ok()?;
+    assert!(
+        show.status.success(),
+        "key show failed for {alg}: {}",
+        String::from_utf8_lossy(&show.stderr)
+    );
+    Some((
+        String::from_utf8_lossy(&show.stdout).into_owned(),
+        String::from_utf8_lossy(&show.stderr).into_owned(),
+    ))
+}
+
+fn assert_algo(alg: &str, expected_substrings: &[&str]) {
+    let Some((stdout, _)) = gen_and_show(alg) else {
+        return;
+    };
+    for needle in expected_substrings {
+        assert!(
+            stdout.contains(needle),
+            "`pki key show` output for {alg} missing `{needle}`:\n{stdout}"
+        );
+    }
+    // Catch the #97 regression directly: PQC keys must NOT be reported as EcP256.
+    if !alg.starts_with("ec-") && !alg.starts_with("rsa") && !alg.starts_with("ed") {
+        assert!(
+            !stdout.contains("\"EcP256\""),
+            "PQC key {alg} misidentified as EcP256 (regression of #97):\n{stdout}"
+        );
+    }
+}
+
+#[test]
+fn key_show_detects_ec_p256() {
+    assert_algo("ec-p256", &["\"EcP256\"", "\"P-256\""]);
+}
+
+#[test]
+fn key_show_detects_rsa() {
+    // Default RSA size is 4096 (2048 is refused under the secure-defaults policy).
+    assert_algo("rsa", &["\"Rsa\"", "4096"]);
+}
+
+#[test]
+fn key_show_detects_mldsa44() {
+    assert_algo("ml-dsa-44", &["MlDsa"]);
+}
+
+#[test]
+fn key_show_detects_mldsa65() {
+    assert_algo("ml-dsa-65", &["MlDsa"]);
+}
+
+#[test]
+fn key_show_detects_mldsa87() {
+    assert_algo("ml-dsa-87", &["MlDsa"]);
+}
+
+#[test]
+fn key_show_detects_slhdsa_128s() {
+    assert_algo("slh-dsa-128s", &["SlhDsa"]);
+}

tests/interop/openssl_parity.sh

@@ -145,16 +145,23 @@ assert_eq "cert/signature_algorithm_name" "sha256WithRSAEncryption" "$PKI_SIGALG
 # SAN: openssl decodes 4 entries (2 DNS, 1 IP, 1 email)
 PKI_SAN_COUNT=$(echo "$PKI_CERT_JSON" | jq -r '.san | length')
 assert_eq "cert/SAN count" "4" "$PKI_SAN_COUNT"
-PKI_SAN_ALL=$(echo "$PKI_CERT_JSON" | jq -r '.san[]' | tr '\n' ',' | sed 's/,$//')
+# SAN is serialized as a tagged enum: [{"Dns":"x"}, {"Ip":"y"}, …]. Flatten to
+# "TAG:value" strings (Dns→DNS, Ip→IP) so assertions stay algorithm-readable.
+PKI_SAN_ALL=$(echo "$PKI_CERT_JSON" | jq -r '
+    .san[] | to_entries[]
+    | ((.key | ascii_upcase | sub("EMAIL"; "email")) + ":" + .value)
+' | tr '\n' ',' | sed 's/,$//')
 assert_contains "cert/SAN DNS:ee.parity.test" "DNS:ee.parity.test" "$PKI_SAN_ALL"
 assert_contains "cert/SAN DNS:alt.parity.test" "DNS:alt.parity.test" "$PKI_SAN_ALL"
 assert_contains "cert/SAN IP:10.0.0.1" "IP:10.0.0.1" "$PKI_SAN_ALL"
 assert_contains "cert/SAN email:admin@parity.test" "admin@parity.test" "$PKI_SAN_ALL"
 
-# EKU
+# EKU — pki emits RFC 5280 long-form labels ("TLS Web Server Authentication");
+# openssl's short names are a display quirk, not a canonical format. Assert on
+# the long form here.
 PKI_EKU=$(echo "$PKI_CERT_JSON" | jq -r '.extended_key_usage[]' | tr '\n' ',' | sed 's/,$//')
-assert_contains "cert/EKU serverAuth" "serverAuth" "$PKI_EKU"
-assert_contains "cert/EKU clientAuth" "clientAuth" "$PKI_EKU"
+assert_contains "cert/EKU serverAuth" "TLS Web Server Authentication" "$PKI_EKU"
+assert_contains "cert/EKU clientAuth" "TLS Web Client Authentication" "$PKI_EKU"
 
 echo ""
 
@@ -177,7 +184,10 @@ assert_eq "csr/key_size" "2048" "$PKI_CSR_KEYSIZE"
 PKI_CSR_SAN_COUNT=$(echo "$PKI_CSR_JSON" | jq -r '.san | length')
 assert_eq "csr/SAN count" "4" "$PKI_CSR_SAN_COUNT"
 
-PKI_CSR_SAN_ALL=$(echo "$PKI_CSR_JSON" | jq -r '.san[]' | tr '\n' ',' | sed 's/,$//')
+PKI_CSR_SAN_ALL=$(echo "$PKI_CSR_JSON" | jq -r '
+    .san[] | to_entries[]
+    | ((.key | ascii_upcase | sub("EMAIL"; "email")) + ":" + .value)
+' | tr '\n' ',' | sed 's/,$//')
 assert_contains "csr/SAN DNS:ee.parity.test" "DNS:ee.parity.test" "$PKI_CSR_SAN_ALL"
 assert_contains "csr/SAN IP:10.0.0.1" "IP:10.0.0.1" "$PKI_CSR_SAN_ALL"
 assert_contains "csr/SAN email:admin@parity.test" "admin@parity.test" "$PKI_CSR_SAN_ALL"
@@ -288,9 +298,11 @@ $PKI csr create --key "$WORK/pki-rsa.key" --cn "pki.parity.test" \
     --san "dns:pki.parity.test,dns:alt.pki.parity.test,ip:10.0.0.2,email:pki@parity.test" \
     -o "$WORK/pki.csr" -q
 
-# openssl must parse pki's CSR
-OSL_CSR_TEXT=$(openssl req -in "$WORK/pki.csr" -noout -text 2>&1)
-assert_contains "reverse/openssl parses pki CSR" "CN = pki.parity.test" "$OSL_CSR_TEXT"
+# openssl must parse pki's CSR — normalize DN spacing: openssl ≤3.4 emits
+# "CN = X" (spaces) while 3.5+ emits "CN=X". Collapse so the assertion works
+# on either.
+OSL_CSR_TEXT=$(openssl req -in "$WORK/pki.csr" -noout -text 2>&1 | sed 's/ = /=/g')
+assert_contains "reverse/openssl parses pki CSR" "CN=pki.parity.test" "$OSL_CSR_TEXT"
 assert_contains "reverse/openssl sees pki DNS SAN" "pki.parity.test" "$OSL_CSR_TEXT"
 assert_contains "reverse/openssl sees pki IP SAN" "10.0.0.2" "$OSL_CSR_TEXT"
 assert_contains "reverse/openssl sees pki email SAN" "pki@parity.test" "$OSL_CSR_TEXT"