Switch storage to `Cow<'a, X>` and implement `ToOwned`?

[indygreg]

I accidentally [1] implemented my own x509 parser as part of implementing RFC 5652 for https://crates.io/crates/cryptographic-message-syntax. When I realized the horrors I had committed, I wanted to switch to using this crate. However, I couldn't coerce it into working easily because data types in this crate have lifetimes and always hold references. e.g. &'a [u8]. There's no option to obtain an owned version of the data structures. And since Rust doesn't allow you to have self-referential data structures [easily], I couldn't get this crate to work given some of my constraints that need 'static / owned lifetimes.

Are the maintainers of this crate receptive to the idea of mass changing all references in structs to Cow<'a, X> and implementing ToOwned so all data types can be owned, cloned, and not subjected to lifetime constraints? If you are, perhaps I'll find time to do the work so I can delete the one-off x509 crate (https://crates.io/crates/x509-certificate) I begrudgingly created.

[1] This was accidental because when I was following the rabbit hole of recursively defining ASN.1 types for RFC 5652, I didn't realize I was reinventing x509 certificate parsing because this was the first time I was exposed to the internals of x509 certificates. Only after I had implemented a full RFC 5652 ASN.1 decoder did I realize many of the types were foundational and general crypto primitives. Oops!

[chifflier]

Hi,
Overall I'm not against the possibility to have owned objects. I think it will help serialization.
A switch to Cow is possible, and it would indeed allow owning objects. It will require to first convert the crate der-parser to an owned model, which is something I was considering in the roadmap for the next major version rusticata/der-parser#37
Any help would be appreciated!

[chifflier]

I have committed a work-in-progress in branch cow-v0.1

I did not implement ToOwned yet. In der-parser I implemented to to_owned method manually (not sure if implementing the trait is needed) to return a 'static object. I'm wondering if that is also needed here.

@indygreg can you test this branch and give some feedback?

[horazont]

@chifflier Hi! I am facing a similar issue where I want to pass and persist the parsed X.509 structure somewhere. So using a Cow backing store makes sense to me.

However, trying to build the dev/cow-v0.1 branch, I get compiler errors:

cargo build output

error[E0599]: no method named `into_bytes` found for struct `BerObject` in the current scope
  --> src/extensions.rs:91:67
   |
91 |             let (i, value) = map_res(parse_der_octetstring, |x| x.into_bytes())(i)?;
   |                                                                   ^^^^^^^^^^ method not found in `BerObject<'_>`

error[E0599]: no method named `into_bytes` found for enum `BerObjectContent` in the current scope
   --> src/extensions.rs:598:18
    |
598 |                 .into_bytes()
    |                  ^^^^^^^^^^ method not found in `BerObjectContent<'_>`

error[E0599]: no method named `into_bytes` found for enum `BerObjectContent` in the current scope
   --> src/extensions.rs:564:18
    |
564 |                 .into_bytes()
    |                  ^^^^^^^^^^ method not found in `BerObjectContent<'_>`

error[E0599]: no method named `into_bytes` found for struct `BerObject` in the current scope
   --> src/extensions.rs:777:50
    |
777 |             authority_cert_serial.and_then(|o| o.into_bytes().map(Cow::Borrowed).ok());
    |                                                  ^^^^^^^^^^ method not found in `BerObject<'_>`

error[E0599]: no method named `into_bytes` found for enum `BerObjectContent` in the current scope
   --> src/extensions.rs:813:14
    |
813 |             .into_bytes()
    |              ^^^^^^^^^^ method not found in `BerObjectContent<'_>`

error[E0599]: no method named `as_bytes` found for struct `BerObject` in the current scope
   --> src/x509.rs:101:25
    |
101 |         self.attr_value.as_bytes().map_err(|e| e.into())
    |                         ^^^^^^^^ method not found in `BerObject<'a>`

error[E0599]: no method named `as_bytes` found for struct `BerObject` in the current scope
   --> src/x509.rs:109:25
    |
109 |         self.attr_value.as_bytes().map_err(|e| e.into())
    |                         ^^^^^^^^ method not found in `BerObject<'a>`

error[E0599]: no method named `as_bytes` found for reference `&BerObject<'_>` in the current scope
   --> src/x509.rs:379:18
    |
379 |             attr.as_bytes()
    |                  ^^^^^^^^ method not found in `&BerObject<'_>`

error[E0599]: no method named `into_bytes` found for struct `BerObject` in the current scope
   --> src/x509.rs:436:19
    |
436 |     let slice = o.into_bytes().ok()?;
    |                   ^^^^^^^^^^ method not found in `BerObject<'_>`

error: aborting due to 9 previous errors

For more information about this error, try `rustc --explain E0599`.
error: could not compile `x509-parser`

Is that expected or am I holding it wrong?

[baloo]

I had a similar need and first thought it would be great to use Cow as well. (the commit just above is a rebase on the current master, if you need).
It ends up being just the same as you still need to handle the lifetime just the same. (My input buffer does not outlives the cert).

I ended up using ouroboros instead:

use std::fmt;

use ouroboros::self_referencing;
use x509_parser::{certificate::X509Certificate, prelude::FromDer};

#[self_referencing]
pub struct X509Cert {
    der_buf: Vec<u8>,
    #[borrows(der_buf)]
    #[covariant]
    cert: X509Certificate<'this>,
}

impl X509Cert {
    pub fn from_der(der: &[u8]) -> Result<Self, ()> {
        // Because we're self-referencing the buffer and the parsed certificate
        // we need to parse it twice.
        // Once to get the actual length of the buffer (and cut off any tail).
        // And a second time to actually store the parsed value.

        let (rest, _cert) = X509Certificate::from_der(der).map_err(|_| ())?;
        let der_buf: Vec<u8> = der[..(der.len() - rest.len())].into();

        X509CertTryBuilder {
            der_buf,
            cert_builder: |buf| match X509Certificate::from_der(&buf[..]) {
                Err(_) => Err(()),
                Ok((_rest, cert)) => Ok(cert),
            },
        }
        .try_build()
    }

    pub fn cert(&self) -> &X509Certificate<'_> {
        self.borrow_cert()
    }
}

impl fmt::Debug for X509Cert {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        self.borrow_cert().fmt(f)
    }
}

That works for me, for now.

[toksdotdev]

I'm curious if this issue is still on the roadmap? I recently ran into a similar problem, and I'll prefer not to use the ouroboros crate which feels more like a workaround.

[adsnaider]

I would also like to see this implemented. Some time ago I created https://docs.rs/ownit/latest/ownit/ for a different parser I was working on. Maybe you will find it useful?

[chifflier]

Hi,
I need to take more time to think about that and try to find a clean API. asn1-rs also has a trait ToStatic which seems to be similar to ownit.
There is a huge update coming for asn1-rs (few weeks I hope), so at the moment I'm only doing maintenance on x509-parser, but as soon as it will be released I will also update this create.

[adsnaider]

Makes sense @chifflier. For what it's worth, the main advantage to ownit would be the derive macro, but if you don't care about that it appears we landed on literally the same trait :P. Anyway, thanks for the awesome crate and the hard work!