Improves BER GeneralizedTime decoders, support for CER/DER strict mode

Cargo.toml

@@ -24,7 +24,7 @@ repository = "https://github.com/XAMPPRocky/rasn.git"
 
 [workspace.dependencies]
 pretty_assertions = "1.1"
-chrono = { version = "0.4.19", default-features = false, features = ["alloc"] }
+chrono = { version = "0.4.26", default-features = false, features = ["alloc"] }
 bitvec = { version = "1.0.1", default-features = false, features = ["alloc"] }
 
 [features]

src/ber.rs

@@ -196,4 +196,91 @@ mod tests {
             }
         }
     }
+    #[test]
+    fn test_generalized_time() {
+        use chrono::{DateTime, FixedOffset, NaiveDate, Utc};
+        // "20801009130005.342Z"
+        let offset = chrono::FixedOffset::east_opt(0).unwrap();
+        let dt = NaiveDate::from_ymd_opt(2080, 10, 9)
+            .unwrap()
+            .and_hms_micro_opt(13, 0, 5, 342_000)
+            .unwrap()
+            .and_local_timezone(offset);
+        round_trip!(
+            ber,
+            GeneralizedTime,
+            GeneralizedTime::from(dt.unwrap(),),
+            &[
+                0x18, 0x13, 0x32, 0x30, 0x38, 0x30, 0x31, 0x30, 0x30, 0x39, 0x31, 0x33, 0x30, 0x30,
+                0x30, 0x35, 0x2e, 0x33, 0x34, 0x32, 0x5a
+            ]
+        );
+
+        // https://github.com/XAMPPRocky/rasn/issues/57
+        let data = [
+            24, 19, 43, 53, 49, 54, 49, 53, 32, 32, 48, 53, 50, 52, 48, 57, 52, 48, 50, 48, 90,
+        ];
+        assert!(crate::der::decode::<crate::types::Open>(&data).is_err());
+        assert!(crate::ber::decode::<crate::types::Open>(&data).is_err());
+
+        // "20180122132900Z"
+        round_trip!(
+            ber,
+            GeneralizedTime,
+            GeneralizedTime::from(
+                NaiveDate::from_ymd_opt(2018, 1, 22)
+                    .unwrap()
+                    .and_hms_opt(13, 29, 0)
+                    .unwrap()
+                    .and_utc()
+            ),
+            &[
+                0x18, 0x0f, 0x32, 0x30, 0x31, 0x38, 0x30, 0x31, 0x32, 0x32, 0x31, 0x33, 0x32, 0x39,
+                0x30, 0x30, 0x5a
+            ]
+        );
+        // "20180122130000Z"
+        round_trip!(
+            ber,
+            GeneralizedTime,
+            GeneralizedTime::from(
+                NaiveDate::from_ymd_opt(2018, 1, 22)
+                    .unwrap()
+                    .and_hms_opt(13, 0, 0)
+                    .unwrap()
+                    .and_utc()
+            ),
+            &[
+                0x18, 0x0f, 0x32, 0x30, 0x31, 0x38, 0x30, 0x31, 0x32, 0x32, 0x31, 0x33, 0x30, 0x30,
+                0x30, 0x30, 0x5a
+            ]
+        );
+
+        // "20230122130000-0500" - converts to canonical form "20230122180000Z"
+        let offset = FixedOffset::east_opt(-3600 * 5).unwrap();
+        let dt1: DateTime<FixedOffset> = GeneralizedTime::from(DateTime::<Utc>::from(
+            NaiveDate::from_ymd_opt(2023, 1, 22)
+                .unwrap()
+                .and_hms_opt(13, 0, 0)
+                .unwrap()
+                .and_local_timezone(offset)
+                .unwrap(),
+        ));
+        round_trip!(
+            ber,
+            GeneralizedTime,
+            dt1,
+            &[
+                0x18, 0x0f, 0x32, 0x30, 0x32, 0x33, 0x30, 0x31, 0x32, 0x32, 0x31, 0x38, 0x30, 0x30,
+                0x30, 0x30, 0x5a
+            ]
+        );
+        // "20230122130000-0500" as bytes
+        let data = [
+            24, 19, 50, 48, 50, 51, 48, 49, 50, 50, 49, 51, 48, 48, 48, 48, 45, 48, 53, 48, 48,
+        ];
+        let result = crate::ber::decode::<crate::types::GeneralizedTime>(&data);
+        assert!(result.is_ok());
+        assert_eq!(dt1, result.unwrap());
+    }
 }

src/ber/de.rs

@@ -18,6 +18,7 @@ use crate::{
     },
     Decode,
 };
+use chrono::{DateTime, FixedOffset, NaiveDateTime, Utc};
 
 pub use self::{config::DecoderOptions, error::Error};
 
@@ -110,6 +111,117 @@ impl<'input> Decoder<'input> {
 
         Ok(result)
     }
+    /// Parse any GeneralizedTime string, allowing for any from ASN.1 definition
+    /// TODO, move to type itself?
+    pub fn parse_any_generalized_time_string(
+        string: alloc::string::String,
+    ) -> Result<types::GeneralizedTime, Error> {
+        // Reference https://obj-sys.com/asn1tutorial/node14.html
+        // If data contains ., 3 decimal places of seconds are expected
+        // If data contains explict Z, result is UTC
+        // If data contains + or -, explicit timezone is given
+        // If neither Z nor + nor -, purely local time is implied
+        let len = string.len();
+        // Helper function to deal with fractions and without timezone
+        let parse_without_timezone = |string: &str| -> Result<NaiveDateTime, Error> {
+            // Handle both decimal cases (dot . and comma , )
+            let string: &str = &string.replace(",", ".");
+            if string.contains('.') {
+                // Use chrono to parse the string every time, since we don't the know the number of decimals places
+                NaiveDateTime::parse_from_str(string, "%Y%m%d%H%.f")
+                    .or_else(|_| NaiveDateTime::parse_from_str(string, "%Y%m%d%H%M%.f"))
+                    .or_else(|_| NaiveDateTime::parse_from_str(string, "%Y%m%d%H%M%S%.f"))
+                    .map_err(|_| Error::InvalidDate)
+            } else {
+                let fmt_string = match string.len() {
+                    8 => "%Y%m%d",
+                    10 => "%Y%m%d%H",
+                    12 => "%Y%m%d%H%M",
+                    14 => "%Y%m%d%H%M%S",
+                    _ => "",
+                };
+                match fmt_string.len() {
+                    l if l > 0 => NaiveDateTime::parse_from_str(string, fmt_string)
+                        .map_err(|_| Error::InvalidDate),
+                    _ => Err(Error::InvalidDate),
+                }
+            }
+        };
+        if string.ends_with('Z') {
+            let naive = parse_without_timezone(&string[..len - 1])?;
+            return Ok(DateTime::<Utc>::from_utc(naive, Utc).into());
+        }
+        // Check for timezone offset
+        if len > 5
+            && string
+                .chars()
+                .nth(len - 5)
+                .map_or(false, |c| c == '+' || c == '-')
+        {
+            let naive = parse_without_timezone(&string[..len - 5])?;
+            let sign = match string.chars().nth(len - 5) {
+                Some('+') => 1,
+                Some('-') => -1,
+                _ => {
+                    return Err(Error::InvalidDate);
+                }
+            };
+            let offset_hours = string
+                .chars()
+                .skip(len - 4)
+                .take(2)
+                .collect::<alloc::string::String>()
+                .parse::<i32>()
+                .map_err(|_| Error::InvalidDate)?;
+            let offset_minutes = string
+                .chars()
+                .skip(len - 2)
+                .take(2)
+                .collect::<alloc::string::String>()
+                .parse::<i32>()
+                .map_err(|_| Error::InvalidDate)?;
+            if offset_hours > 23 || offset_minutes > 59 {
+                return Err(Error::InvalidDate);
+            }
+            let offset = FixedOffset::east_opt(sign * (offset_hours * 3600 + offset_minutes * 60))
+                .ok_or(Error::InvalidDate)?;
+            return Ok(DateTime::<FixedOffset>::from_local(naive, offset).into());
+        }
+
+        // Parse without timezone details
+        let naive = parse_without_timezone(&string)?;
+        Ok(DateTime::<Utc>::from_local(naive, Utc).into())
+    }
+    /// Enforce CER/DER restrictions defined in Section 11.7, strictly raise error on non-compliant
+    pub fn parse_canonical_generalized_time_string(
+        string: alloc::string::String,
+    ) -> Result<types::GeneralizedTime, Error> {
+        let len = string.len();
+        // Helper function to deal with fractions of seconds and without timezone
+        let parse_without_timezone = |string: &str| -> core::result::Result<NaiveDateTime, Error> {
+            let len = string.len();
+            if string.contains('.') {
+                // https://github.com/chronotope/chrono/issues/238#issuecomment-378737786
+                NaiveDateTime::parse_from_str(string, "%Y%m%d%H%M%S%.f")
+                    .map_err(|_| Error::InvalidDate)
+            } else if len == 14 {
+                NaiveDateTime::parse_from_str(string, "%Y%m%d%H%M%S")
+                    .map_err(|_| Error::InvalidDate)
+            } else {
+                // CER/DER encoding rules don't allow for timezone offset +/
+                // Or missing seconds/minutes/hours
+                // Or comma , instead of dot .
+                // Or local time without timezone
+                Err(Error::InvalidDate)
+            }
+        };
+        return if string.ends_with('Z') {
+            let naive = parse_without_timezone(&string[..len - 1])?;
+            Ok(DateTime::<Utc>::from_utc(naive, Utc).into())
+        } else {
+            Err(Error::InvalidDate)
+        };
+    }
 }
 
 impl<'input> crate::Decoder for Decoder<'input> {
@@ -354,51 +466,11 @@ impl<'input> crate::Decoder for Decoder<'input> {
 
     fn decode_generalized_time(&mut self, tag: Tag) -> Result<types::GeneralizedTime> {
         let string = self.decode_utf8_string(tag, <_>::default())?;
-        // Reference https://obj-sys.com/asn1tutorial/node14.html
-        // If data contains ., 3 decimal places of seconds are expected
-        // If data contains explict Z, result is UTC
-        // If data contains + or -, explicit timezone is given
-        // If neither Z nor + nor -, purely local time is implied
-        // FIXME for future, NaiveDatetime is right in last case. Others want DateTime<UTC>
-        // or DateTime<FixedOffset> respectively
-        // FIXME, supposedly, minutes and seconds are optional, and would have to be handled
-        // in the no decimal point cases
-        let format = if string.contains('Z') {
-            if string.contains('.') {
-                "%Y%m%d%H%M%S%.3fZ"
-            } else {
-                match string.len() {
-                    11 => "%Y%m%d%HZ",
-                    13 => "%Y%m%d%H%MZ",
-                    _ => "%Y%m%d%H%M%SZ",
-                }
-            }
-        } else if string.contains('+') || string.contains('-') {
-            if string.contains('.') {
-                "%Y%m%d%H%M%S%.3f%z"
-            } else {
-                match string.len() {
-                    15 => "%Y%m%d%H%%z",
-                    17 => "%Y%m%d%H%M%z",
-                    _ => "%Y%m%d%H%M%S%z",
-                }
-            }
-        } else if string.contains('.') {
-            "%Y%m%d%H%M%S%.3f"
+        if self.config.encoding_rules.is_ber() {
+            Self::parse_any_generalized_time_string(string)
         } else {
-            match string.len() {
-                10 => "%Y%m%d%H",
-                12 => "%Y%m%d%H%M",
-                _ => "%Y%m%d%H%M%S",
-            }
-        };
-
-        chrono::NaiveDateTime::parse_from_str(&string, format)
-            .ok()
-            .context(error::InvalidDateSnafu)
-            .map(|date| {
-                types::GeneralizedTime::from_utc(date, chrono::FixedOffset::east_opt(0).unwrap())
-            })
+            Self::parse_canonical_generalized_time_string(string)
+        }
     }
 
     fn decode_utc_time(&mut self, tag: Tag) -> Result<types::UtcTime> {

src/ber/enc.rs

@@ -4,6 +4,7 @@ mod config;
 mod error;
 
 use alloc::{collections::VecDeque, string::ToString, vec::Vec};
+use chrono::Timelike;
 
 use super::Identifier;
 use crate::{
@@ -240,6 +241,27 @@ impl Encoder {
                 .insert(tag, core::mem::take(&mut self.output));
         }
     }
+    #[must_use]
+    /// Canonical byte presentation for CER/DER as defined in X.690 section 11.7.
+    /// Also used for BER on this crate.
+    pub fn datetime_to_canonical_generalized_time_bytes(
+        value: &chrono::DateTime<chrono::FixedOffset>,
+    ) -> Vec<u8> {
+        let mut string;
+        // Convert to UTC so we can always append Z.
+        let value = value.naive_utc();
+        if value.nanosecond() > 0 {
+            string = value.format("%Y%m%d%H%M%S.%f").to_string();
+            // No trailing zeros with fractions
+            while string.ends_with('0') {
+                string.pop();
+            }
+        } else {
+            string = value.format("%Y%m%d%H%M%S").to_string();
+        }
+        string.push('Z');
+        string.into_bytes()
+    }
 }
 
 impl crate::Encoder for Encoder {
@@ -449,11 +471,7 @@ impl crate::Encoder for Encoder {
     ) -> Result<Self::Ok, Self::Error> {
         self.encode_primitive(
             tag,
-            value
-                .naive_utc()
-                .format("%Y%m%d%H%M%SZ")
-                .to_string()
-                .as_bytes(),
+            Self::datetime_to_canonical_generalized_time_bytes(value).as_slice(),
         );
 
         Ok(())