Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x509-cert: move to all owned types? #765

Closed
tarcieri opened this issue Nov 19, 2022 · 8 comments
Closed

x509-cert: move to all owned types? #765

tarcieri opened this issue Nov 19, 2022 · 8 comments

Comments

@tarcieri
Copy link
Member

The x509-cert crate currently has a hard dependency on alloc, but several types still have a lifetime which borrows from the input, which precludes one-pass decoding from PEM (where Base64 is decoded directly into owned types) and makes writing things like certificate builders harder.

We could potentially move entirely to types which own their backing data, eliminating lifetimes from all types. We could still potentially have a corresponding set of borrowed e.g. Ref types (see #689), but using owned types would make usage more convenient at the cost of losing zero-copy decoding from DER bytes.

@baloo
Copy link
Member

baloo commented Nov 23, 2022

I don't really see a way to make a better certificate builder API without this. The current version of the API kind of work, but it's pretty inconvenient to use.

baloo added a commit to baloo/formats that referenced this issue Nov 25, 2022
@carl-wallace
Copy link
Contributor

I'd prefer to see owned and no copy be peers rather than owned replacing no copy (even if owned is why got the shorter crate name). The draft changes would generate a pile of work to catch up and each could be useful to have.

@tarcieri
Copy link
Member Author

@carl-wallace as I mentioned, we can still have *Ref types which would be useful for #689

@baloo
Copy link
Member

baloo commented Nov 26, 2022

So do we want just duplicate all the types to add a Foo along with FooRef ?
For example the Name<'a>, the tree of types there is pretty heavy.

I'm okay with that, just want to make sure, for example the impl of AnyRef is pretty heavy, and duplicating that is a lot of code.

impl<'a> AnyRef<'a> {
/// [`AnyRef`] representation of the ASN.1 `NULL` type.
pub const NULL: Self = Self {
tag: Tag::Null,
value: ByteSlice::EMPTY,
};
/// Create a new [`AnyRef`] from the provided [`Tag`] and DER bytes.
pub fn new(tag: Tag, bytes: &'a [u8]) -> Result<Self> {
let value = ByteSlice::new(bytes).map_err(|_| ErrorKind::Length { tag })?;
Ok(Self { tag, value })
}
/// Infallible creation of an [`AnyRef`] from a [`ByteSlice`].
pub(crate) fn from_tag_and_value(tag: Tag, value: ByteSlice<'a>) -> Self {
Self { tag, value }
}
/// Get the raw value for this [`AnyRef`] type as a byte slice.
pub fn value(self) -> &'a [u8] {
self.value.as_slice()
}
/// Attempt to decode this [`AnyRef`] type into the inner value.
pub fn decode_into<T>(self) -> Result<T>
where
T: DecodeValue<'a> + FixedTag,
{
self.tag.assert_eq(T::TAG)?;
let header = Header {
tag: self.tag,
length: self.value.len(),
};
let mut decoder = SliceReader::new(self.value())?;
let result = T::decode_value(&mut decoder, header)?;
decoder.finish(result)
}
/// Is this value an ASN.1 `NULL` value?
pub fn is_null(self) -> bool {
self == Self::NULL
}
/// Attempt to decode an ASN.1 `BIT STRING`.
pub fn bit_string(self) -> Result<BitStringRef<'a>> {
self.try_into()
}
/// Attempt to decode an ASN.1 `CONTEXT-SPECIFIC` field.
pub fn context_specific<T>(self) -> Result<ContextSpecific<T>>
where
T: Decode<'a>,
{
self.try_into()
}
/// Attempt to decode an ASN.1 `GeneralizedTime`.
pub fn generalized_time(self) -> Result<GeneralizedTime> {
self.try_into()
}
/// Attempt to decode an ASN.1 `IA5String`.
pub fn ia5_string(self) -> Result<Ia5StringRef<'a>> {
self.try_into()
}
/// Attempt to decode an ASN.1 `OCTET STRING`.
pub fn octet_string(self) -> Result<OctetStringRef<'a>> {
self.try_into()
}
/// Attempt to decode an ASN.1 `OBJECT IDENTIFIER`.
#[cfg(feature = "oid")]
#[cfg_attr(docsrs, doc(cfg(feature = "oid")))]
pub fn oid(self) -> Result<ObjectIdentifier> {
self.try_into()
}
/// Attempt to decode an ASN.1 `OPTIONAL` value.
pub fn optional<T>(self) -> Result<Option<T>>
where
T: Choice<'a> + TryFrom<Self, Error = Error>,
{
if T::can_decode(self.tag) {
T::try_from(self).map(Some)
} else {
Ok(None)
}
}
/// Attempt to decode an ASN.1 `PrintableString`.
pub fn printable_string(self) -> Result<PrintableStringRef<'a>> {
self.try_into()
}
/// Attempt to decode an ASN.1 `TeletexString`.
pub fn teletex_string(self) -> Result<TeletexStringRef<'a>> {
self.try_into()
}
/// Attempt to decode an ASN.1 `VideotexString`.
pub fn videotex_string(self) -> Result<VideotexStringRef<'a>> {
self.try_into()
}
/// Attempt to decode this value an ASN.1 `SEQUENCE`, creating a new
/// nested reader and calling the provided argument with it.
pub fn sequence<F, T>(self, f: F) -> Result<T>
where
F: FnOnce(&mut SliceReader<'a>) -> Result<T>,
{
self.tag.assert_eq(Tag::Sequence)?;
let mut reader = SliceReader::new(self.value.as_slice())?;
let result = f(&mut reader)?;
reader.finish(result)
}
/// Attempt to decode an ASN.1 `UTCTime`.
pub fn utc_time(self) -> Result<UtcTime> {
self.try_into()
}
/// Attempt to decode an ASN.1 `UTF8String`.
pub fn utf8_string(self) -> Result<Utf8StringRef<'a>> {
self.try_into()
}
}

Or do I just throw a macro in there an factorize out both bodies?

@tarcieri
Copy link
Member Author

@baloo for now I’d say make everything owned, and circle back in *Ref types later as part of #689, which needs additional design work anyway

@carl-wallace
Copy link
Contributor

carl-wallace commented Nov 26, 2022 via email

@tarcieri
Copy link
Member Author

Could we have proc macros emit a second set of structures

Really if we want to have unified borrowed/owned types I would suggest evaluating yoke for this purpose (see #734)

It would allow us to define zero-copy types and owned wrappers which borrow from them.

However, if we'd like to have truly heapless support as per #689, I think they are really going to need to be different data structures entirely. It may be necessary to loop in e.g. heapless::Pool to provide backing storage for certain types of objects.

@tarcieri
Copy link
Member Author

This has been completed, as far as I'm aware

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants