From 254fa3b6ca6f7a34c10171b181540bafcb1a9b8d Mon Sep 17 00:00:00 2001
From: Yi LIU <yi@quantstamp.com>
Date: Tue, 10 Mar 2026 09:16:15 +0800
Subject: [PATCH] fix(cri): iterate SET contents in iter_raw_values(), not SET
 TLV

Previously iter_raw_values() iterated over the full SET TLV, yielding
the SET itself as a single item. Now it parses past the SET header and
iterates over the individual values inside the SET.

Updated rustdoc to document that the method returns raw DER content of
each value within the SET. Added tag assertions in tests to verify that
returned items are the inner values (e.g. UTF8String, SEQUENCE) rather
than the SET envelope.
---
 src/cri_attributes.rs | 13 ++++++++++-
 tests/readcsr.rs      | 52 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/src/cri_attributes.rs b/src/cri_attributes.rs
index e87a02b..56c762c 100644
--- a/src/cri_attributes.rs
+++ b/src/cri_attributes.rs
@@ -33,10 +33,21 @@ impl<'a> X509CriAttribute<'a> {
     }
 
     /// Iterate over the unparsed values of 'SET OF AttributeValue'
+    ///
+    /// Each item yields the raw DER content of an individual value within the SET,
+    /// returned as an [`Any`] object containing the DER-encoded tag, length, and value.
+    ///
+    /// If the SET header cannot be parsed, an empty iterator is returned.
     pub fn iter_raw_values(
         &self,
     ) -> impl Iterator<Item = Result<(Input<'a>, Any<'a>), BerError<Input<'a>>>> {
-        AnyIterator::<DerMode>::new(self.value.clone())
+        // `self.value` contains the full SET TLV; parse past the SET header
+        // to iterate over the individual values inside the SET
+        let content = match <Set>::parse_der_as_input(self.value.clone()) {
+            Ok((_, (_, content))) => content,
+            Err(_) => Input::default(),
+        };
+        AnyIterator::<DerMode>::new(content)
     }
 }
 
diff --git a/tests/readcsr.rs b/tests/readcsr.rs
index 13705b8..b8013e0 100644
--- a/tests/readcsr.rs
+++ b/tests/readcsr.rs
@@ -115,6 +115,58 @@ fn read_csr_with_challenge_password() {
     assert!(found_san);
 }
 
+#[test]
+fn test_iter_raw_values() {
+    let der = pem::parse_x509_pem(CSR_CHALLENGE_PASSWORD).unwrap().1;
+    let (_, csr) = X509CertificationRequest::from_der(&der.contents)
+        .expect("Could not parse CSR with challenge password");
+
+    // Test iter_raw_values on the challenge password attribute (single value)
+    let challenge_attr = csr
+        .certification_request_info
+        .find_attribute(&OID_PKCS9_CHALLENGE_PASSWORD)
+        .expect("Challenge password attribute not found");
+
+    let raw_values: Vec<_> = challenge_attr
+        .iter_raw_values()
+        .collect::<Result<Vec<_>, _>>()
+        .expect("iter_raw_values should yield parseable items");
+    // The challenge password SET has exactly one value
+    assert_eq!(raw_values.len(), 1);
+
+    // The parsed Any should be a UTF8String containing "A challenge password"
+    let (_, any_val) = &raw_values[0];
+    // Verify raw DER: tag should be UTF8String (0x0C), not SET (0x31)
+    assert_eq!(
+        any_val.header.tag(),
+        x509_parser::asn1_rs::Tag::Utf8String,
+        "iter_raw_values should yield individual values inside the SET, not the SET itself"
+    );
+    let s = std::str::from_utf8(any_val.data.as_bytes2())
+        .expect("challenge password value should be valid UTF-8");
+    assert_eq!(s, "A challenge password");
+
+    // Test iter_raw_values on the extension request attribute (single value: a SEQUENCE)
+    let ext_attr = csr
+        .certification_request_info
+        .find_attribute(&OID_PKCS9_EXTENSION_REQUEST)
+        .expect("Extension request attribute not found");
+
+    let raw_values: Vec<_> = ext_attr
+        .iter_raw_values()
+        .collect::<Result<Vec<_>, _>>()
+        .expect("iter_raw_values should yield parseable items");
+    // The extension request SET has exactly one value (the SEQUENCE of extensions)
+    assert_eq!(raw_values.len(), 1);
+    let (_, any_val) = &raw_values[0];
+    // Verify raw DER: tag should be SEQUENCE (0x30), not SET (0x31)
+    assert_eq!(
+        any_val.header.tag(),
+        x509_parser::asn1_rs::Tag::Sequence,
+        "iter_raw_values should yield SET contents, not the SET envelope"
+    );
+}
+
 #[cfg(any(feature = "verify", feature = "verify-aws"))]
 #[test]
 fn read_csr_verify() {