From 5f21c3fe999df30f779586745f072fb115be53af Mon Sep 17 00:00:00 2001
From: Frederik Rothenberger <frederik.rothenberger@dfinity.org>
Date: Tue, 27 Aug 2024 10:04:44 +0200
Subject: [PATCH] Fix bug in canister sig public key handling (#51)

The last PR (#49) unfortunately introduced a bug (confusion between DER
encoded and raw canister signature public key). This PR fixes it.

In addition, #49 could only be merged, because the unit test for the
rust library were not actually run. CI is changed to no run all rust
tests.
---
 .github/workflows/rust.yml                    |  8 +++---
 .../ic-verifiable-credentials/src/lib.rs      | 27 ++++++++++++-------
 2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
index 387f69f..3ee41c7 100644
--- a/.github/workflows/rust.yml
+++ b/.github/workflows/rust.yml
@@ -51,8 +51,8 @@ jobs:
         working-directory: dummy-issuer
         run: ./build.sh
 
-      # fails if lockfile is out of date
-      # https://users.rust-lang.org/t/check-if-the-cargo-lock-is-up-to-date-without-building-anything/91048/5
-      # Run only dummy_issuer to avoid building dummy_relying_party
+      - name: Create dist folder
+        run: mkdir -p dummy-relying-party/frontend/dist
+
       - name: Cargo tests
-        run: cargo test --package dummy_issuer
+        run: cargo test
diff --git a/rust-packages/ic-verifiable-credentials/src/lib.rs b/rust-packages/ic-verifiable-credentials/src/lib.rs
index 796dff1..131c6fe 100644
--- a/rust-packages/ic-verifiable-credentials/src/lib.rs
+++ b/rust-packages/ic-verifiable-credentials/src/lib.rs
@@ -205,8 +205,8 @@ pub fn verify_credential_jws_with_canister_id(
     let jws_header = jws
         .protected_header()
         .ok_or(invalid_signature_err("missing JWS header"))?;
-    let canister_sig_pk_raw = get_canister_sig_pk_raw(jws_header)?;
-    let canister_sig_pk = CanisterSigPublicKey::try_from_raw(canister_sig_pk_raw.as_slice())
+    let canister_sig_pk_der = get_canister_sig_pk_der(jws_header)?;
+    let canister_sig_pk = CanisterSigPublicKey::try_from(canister_sig_pk_der.as_slice())
         .map_err(|e| key_decoding_err(&format!("invalid canister sig public key: {}", e)))?;
 
     if signing_canister_id != &canister_sig_pk.canister_id {
@@ -220,7 +220,7 @@ pub fn verify_credential_jws_with_canister_id(
     verify_canister_sig(
         &message,
         signature,
-        canister_sig_pk_raw.as_slice(),
+        canister_sig_pk_der.as_slice(),
         root_pk_raw,
     )
     .map_err(|e| invalid_signature_err(&format!("signature verification error: {}", e)))?;
@@ -624,9 +624,19 @@ fn inconsistent_jwt_claims(custom_message: &'static str) -> JwtValidationError {
     ))
 }
 
-/// Extracts and returns raw canister sig public key (without DER-prefix) from the given header.
+/// Extracts and returns the raw canister sig public key (without DER-prefix) from the given header.
 pub fn get_canister_sig_pk_raw(
     jws_header: &JwsHeader,
+) -> Result<Vec<u8>, SignatureVerificationError> {
+    let pk_der = get_canister_sig_pk_der(jws_header)?;
+    let pk_raw = extract_raw_canister_sig_pk_from_der(pk_der.as_slice())
+        .map_err(|e| key_decoding_err(&e.to_string()))?;
+    Ok(pk_raw)
+}
+
+/// Extracts and returns the DER encoded canister sig public key from the given header.
+pub fn get_canister_sig_pk_der(
+    jws_header: &JwsHeader,
 ) -> Result<Vec<u8>, SignatureVerificationError> {
     let jwk = jws_header
         .deref()
@@ -645,9 +655,7 @@ pub fn get_canister_sig_pk_raw(
         .map_err(|_| key_decoding_err("missing JWK oct params"))?;
     let pk_der = decode_b64(jwk_params.k.as_bytes())
         .map_err(|_| key_decoding_err("invalid base64url encoding"))?;
-    let pk_raw = extract_raw_canister_sig_pk_from_der(pk_der.as_slice())
-        .map_err(|e| key_decoding_err(&e.to_string()))?;
-    Ok(pk_raw)
+    Ok(pk_der)
 }
 
 fn construct_verifiable_presentation_jwt(
@@ -896,9 +904,8 @@ mod tests {
             &ic_root_pk,
             CURRENT_TIME_BEFORE_EXPIRY_NS,
         );
-        assert_matches!(result, Err(e) if  { let err_msg = e.to_string();
-            err_msg.contains("invalid signature") &&
-            err_msg.contains("Malformed ThresBls12_381 public key") });
+        let err = result.err().expect("expected error");
+        assert!(err.to_string().contains("invalid BLS signature"))
     }
 
     #[test]