cms: Implement types from RFC 6031 - Cryptographic Message Syntax (CMS) Symmetric Key Package Content Type

[callumadair]

Implementation of types as defined in RFC 6031

Several queries as I was implementing:

• I've used Utf8StringRef in the types that require an ASN.1 UTF8String, would it be preferable to have the structs own this data and use the String type instead?
• const-oid seems to generate relevantt oids by parsing RFC documents, should I therefore add the RFC-6031.txt file to the ones already present in oidbindgen?

[baloo]

• I've used Utf8StringRef in the types that require an ASN.1 UTF8String, would it be preferable to have the structs own this data and use the String type instead?

Or a proper rust enum? Should we reject values with small differences from the one defined in the RFC?

[tarcieri]

I've used Utf8StringRef in the types that require an ASN.1 UTF8String, would it be preferable to have the structs own this data and use the String type instead?

As a general rule you can prefer the allocating types over the *Ref types for cms, since it has a hard alloc dependency

[callumadair]

If there are no more review comments, is this able to be merged?

[baloo]

Or a proper rust enum? Should we reject values with small differences from the one defined in the RFC?

Would that be completely crazy?

diff --git a/cms/src/symmetric_key.rs b/cms/src/symmetric_key.rs
index 041162ca6c..abab33a8cf 100644
--- a/cms/src/symmetric_key.rs
+++ b/cms/src/symmetric_key.rs
@@ -1,7 +1,11 @@
 //! SymmetricKeyPackage types

 use alloc::{string::String, vec::Vec};
-use der::{Enumerated, Sequence, asn1::OctetString};
+use der::{
+    DecodeValue, EncodeValue, Enumerated, ErrorKind, FixedTag, Header, Length, Reader, Sequence,
+    Tag, Writer,
+    asn1::{OctetString, Utf8StringRef},
+};
 use x509_cert::attr::Attribute;

 /// The `SymmetricKeyPackage` type is defined in [RFC 6031 Section 2.0].
@@ -138,7 +142,64 @@
 /// ```
 ///
 /// [RFC 6031 Section 3.2.7]: https://datatracker.ietf.org/doc/html/rfc6031#section-3.2.7
-pub type Encoding = String;
+#[derive(Copy, Clone, PartialEq, Eq)]
+pub enum Encoding {
+    /// "DECIMAL"
+    Decimal,
+    /// "HEXADECIMAL"
+    Hexadecimal,
+    /// "ALPHANUMERIC"
+    Alphanumeric,
+    /// "BASE64"
+    Base64,
+    /// "BINARY"
+    Binary,
+}
+
+impl AsRef<str> for Encoding {
+    fn as_ref(&self) -> &str {
+        match self {
+            Self::Decimal => "DECIMAL",
+            Self::Hexadecimal => "HEXADECIMAL",
+            Self::Alphanumeric => "ALPHANUMERIC",
+            Self::Base64 => "BASE64",
+            Self::Binary => "BINARY",
+        }
+    }
+}
+
+impl<'d> DecodeValue<'d> for Encoding {
+    type Error = der::Error;
+
+    fn decode_value<R: Reader<'d>>(reader: &mut R, header: Header) -> der::Result<Self> {
+        let value = Utf8StringRef::decode_value(reader, header)?;
+        match value.as_ref() {
+            "DECIMAL" => Ok(Self::Decimal),
+            "HEXADECIMAL" => Ok(Self::Hexadecimal),
+            "ALPHANUMERIC" => Ok(Self::Alphanumeric),
+            "BASE64" => Ok(Self::Base64),
+            "BINARY" => Ok(Self::Binary),
+            _ => Err(der::Error::new(
+                ErrorKind::Value { tag: Self::TAG },
+                reader.position(),
+            )),
+        }
+    }
+}
+
+impl EncodeValue for Encoding {
+    fn value_len(&self) -> der::Result<Length> {
+        Utf8StringRef::new(self.as_ref())?.value_len()
+    }
+
+    fn encode_value(&self, encoder: &mut impl Writer) -> der::Result<()> {
+        Utf8StringRef::new(self.as_ref())?.encode_value(encoder)
+    }
+}
+
+impl FixedTag for Encoding {
+    const TAG: Tag = String::TAG;
+}

 /// The `ResponseFormat` type is defined in [RFC 6031 Section 3.2.7].
 ///

I guess a macro could help with the boilerplate (PinUsageMode and PSKCKeyUsage also need this treatment)

[callumadair]

Or a proper rust enum? Should we reject values with small differences from the one defined in the RFC?

Would that be completely crazy?

Ohh, I see how you mean now @baloo. I had thought you meant something like this in terms of ownnership vs Ref strings:

pub enum Utf8String<'string> {
    Owned(String),
    Ref(Utf8StringRef<'string>),
}

I think you're correct in that they would be perfect for an enum. Additionally a macro would probably be beneficial as I think there are other RFCst that do have similar predefined strings as values, though I wonder if its implementation might be out of scope for this PR.
Something like the below maybe?

#[derive(EncodeAsStr)]
pub enum Encoding {
...
}

[baloo]

I'm not aware of any other RFC where this happens, but there are many of them and I don't know them all.

I had a macro_rules in mind. I don't think we need an additional derive crate just for that. Anyway, that was more a question for others to chime in :)

[tarcieri]

That enum looks like Cow, which we will hopefully support in the next release

[baloo]

hum? That was not my question.

My question was about the use of an enum with the variations of values like:

/// The `Encoding` type is defined in [RFC 6031 Section 3.2.7].
///
/// ```text
///    Encoding ::= UTF8STRING ("DECIMAL" | "HEXADECIMAL" |
///                 "ALPHANUMERIC" |"BASE64" |"BINARY")
/// ```
///
/// [RFC 6031 Section 3.2.7]: https://datatracker.ietf.org/doc/html/rfc6031#section-3.2.7
pub enum Encoding {
    /// "DECIMAL"
    Decimal,
    /// "HEXADECIMAL"
    Hexadecimal,
    /// "ALPHANUMERIC"
    Alphanumeric,
    /// "BASE64"
    Base64,
    /// "BINARY"
    Binary,
}

Instead of accepting any form of string in that field.

[tarcieri]

@baloo I was referring to this enum: #1952 (comment)

[baloo]

@callumadair the macro would look like:

diff --git a/cms/src/symmetric_key.rs b/cms/src/symmetric_key.rs
index 041162ca6c..68fd8ea2ef 100644
--- a/cms/src/symmetric_key.rs
+++ b/cms/src/symmetric_key.rs
@@ -1,7 +1,11 @@
 //! SymmetricKeyPackage types

 use alloc::{string::String, vec::Vec};
-use der::{Enumerated, Sequence, asn1::OctetString};
+use der::{
+    DecodeValue, EncodeValue, Enumerated, ErrorKind, FixedTag, Header, Length, Reader, Sequence,
+    Tag, Writer,
+    asn1::{OctetString, Utf8StringRef},
+};
 use x509_cert::attr::Attribute;

 /// The `SymmetricKeyPackage` type is defined in [RFC 6031 Section 2.0].
@@ -130,15 +134,82 @@
     pub max: der::asn1::Int,
 }

-/// The `Encoding` type is defined in [RFC 6031 Section 3.2.7].
-///
-/// ```text
-///    Encoding ::= UTF8STRING ("DECIMAL" | "HEXADECIMAL" |
-///                 "ALPHANUMERIC" |"BASE64" |"BINARY")
-/// ```
-///
-/// [RFC 6031 Section 3.2.7]: https://datatracker.ietf.org/doc/html/rfc6031#section-3.2.7
-pub type Encoding = String;
+macro_rules! impl_str_enum {
+    (
+        $(#[$attr:meta])* $vis: vis enum $name: ident {
+            $( $(#[$attr_item: meta])* $item: ident => $value: expr,)*
+        }
+    )=> {
+        $(#[$attr])*
+        $vis enum $name {
+            $( $(#[$attr_item])* $item, )*
+        }
+
+        impl AsRef<str> for $name {
+            fn as_ref(&self) -> &str {
+                match self {
+                    $( Self::$item => $value, )*
+                }
+            }
+        }
+
+        impl<'d> DecodeValue<'d> for $name {
+            type Error = der::Error;
+
+            fn decode_value<R: Reader<'d>>(reader: &mut R, header: Header) -> der::Result<Self> {
+                let value = Utf8StringRef::decode_value(reader, header)?;
+
+                match value.as_ref() {
+                    $( $value => Ok(Self::$item), )*
+                    _ => Err(der::Error::new(
+                        ErrorKind::Value { tag: Self::TAG },
+                        reader.position(),
+                    )),
+                }
+            }
+        }
+
+        impl EncodeValue for $name {
+            fn value_len(&self) -> der::Result<Length> {
+                Utf8StringRef::new(self.as_ref())?.value_len()
+            }
+
+            fn encode_value(&self, encoder: &mut impl Writer) -> der::Result<()> {
+                Utf8StringRef::new(self.as_ref())?.encode_value(encoder)
+            }
+        }
+
+        impl FixedTag for $name {
+            const TAG: Tag = String::TAG;
+        }
+
+
+    };
+}
+
+impl_str_enum!(
+    /// The `Encoding` type is defined in [RFC 6031 Section 3.2.7].
+    ///
+    /// ```text
+    ///    Encoding ::= UTF8STRING ("DECIMAL" | "HEXADECIMAL" |
+    ///                 "ALPHANUMERIC" |"BASE64" |"BINARY")
+    /// ```
+    ///
+    /// [RFC 6031 Section 3.2.7]: https://datatracker.ietf.org/doc/html/rfc6031#section-3.2.7
+    #[derive(Copy, Clone, PartialEq, Eq)]
+    pub enum Encoding {
+        /// "DECIMAL"
+        Decimal => "DECIMAL",
+        /// "HEXADECIMAL",
+        Hexadecimal => "HEXADECIMAL",
+        /// "ALPHANUMERIC",
+        Alphanumeric => "ALPHANUMERIC",
+        /// "BASE64",
+        Base64 => "BASE64",
+        /// "BINARY",
+        Binary => "BINARY",
+    }
+);

 /// The `ResponseFormat` type is defined in [RFC 6031 Section 3.2.7].
 ///