Skip to content

dsa: reduce Box<[u8]> allocation in SignatureEncoding::to_vec #969

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 17 additions & 26 deletions dsa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ use pkcs8::spki::ObjectIdentifier;

mod components;
mod generate;
mod signature_ref;
mod signing_key;
mod size;
mod verifying_key;
Expand All @@ -76,10 +77,10 @@ pub const OID: ObjectIdentifier = ObjectIdentifier::new_unwrap("1.2.840.10040.4.

use alloc::{boxed::Box, vec::Vec};
use pkcs8::der::{
self, Decode, DecodeValue, Encode, EncodeValue, FixedTag, Header, Length, Reader, Sequence,
Writer, asn1::UintRef,
self, Decode, DecodeValue, Encode, EncodeValue, Header, Length, Reader, Sequence, Writer,
};
use signature::SignatureEncoding;
use signature_ref::{SignatureBoxed, SignatureRef};

/// Container of the DSA signature
#[derive(Clone, Debug)]
Expand Down Expand Up @@ -109,43 +110,33 @@ impl Signature {
pub fn s(&self) -> &NonZero<BoxedUint> {
&self.s
}

fn to_boxed(&self) -> SignatureBoxed {
SignatureBoxed::new(self)
}
fn to_der_using_ref(&self) -> der::Result<Vec<u8>> {
self.to_boxed().to_ref()?.to_der()
}
}

impl<'a> DecodeValue<'a> for Signature {
type Error = der::Error;

fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> der::Result<Self> {
reader.read_nested(header.length, |reader| {
let r = UintRef::decode(reader)?;
let s = UintRef::decode(reader)?;

let r = BoxedUint::from_be_slice(r.as_bytes(), r.as_bytes().len() as u32 * 8)
.map_err(|_| UintRef::TAG.value_error())?;
let s = BoxedUint::from_be_slice(s.as_bytes(), s.as_bytes().len() as u32 * 8)
.map_err(|_| UintRef::TAG.value_error())?;

let r = NonZero::new(r)
.into_option()
.ok_or(UintRef::TAG.value_error())?;
let s = NonZero::new(s)
.into_option()
.ok_or(UintRef::TAG.value_error())?;

Ok(Self::from_components(r, s))
})
let signature_ref = SignatureRef::decode_value(reader, header)?;

signature_ref.to_owned()
}
}

impl EncodeValue for Signature {
fn value_len(&self) -> der::Result<Length> {
UintRef::new(&self.r.to_be_bytes())?.encoded_len()?
+ UintRef::new(&self.s.to_be_bytes())?.encoded_len()?
// TODO: avoid Box<[u8]> allocation here
self.to_boxed().to_ref()?.value_len()
}

fn encode_value(&self, writer: &mut impl Writer) -> der::Result<()> {
UintRef::new(&self.r.to_be_bytes())?.encode(writer)?;
UintRef::new(&self.s.to_be_bytes())?.encode(writer)?;
Ok(())
self.to_boxed().to_ref()?.encode_value(writer)
}
}

Expand Down Expand Up @@ -177,7 +168,7 @@ impl SignatureEncoding for Signature {
}

fn to_vec(&self) -> Vec<u8> {
self.to_der().expect("DER encoding error")
self.to_der_using_ref().expect("DER encoding error")
}
}

Expand Down
78 changes: 78 additions & 0 deletions dsa/src/signature_ref.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
use alloc::boxed::Box;
use crypto_bigint::{BoxedUint, NonZero};
use pkcs8::der::{
self, Decode, DecodeValue, Encode, EncodeValue, FixedTag, Header, Length, Reader, Sequence,
Writer, asn1::UintRef,
};

use crate::Signature;

pub(crate) struct SignatureBoxed {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is essentially what dsa::Signature is (dsa::Signature is composed of NonZero<BoxedUint>)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't it be possible to get an iterator over the inner limbs? and get big endian from there? without allocating a new box.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

der checks for leading zeros when encoding UIntRef, so it would be too tricky

Copy link
Member

@baloo baloo May 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well that's just up to the consumer of the said iterator to strip leading zeroes? I don't know.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah nevermind, you have to have a buffer for SignatureRef, that can't be an iterator of limbs.

r: Box<[u8]>,
s: Box<[u8]>,
}
impl SignatureBoxed {
pub fn new(sig: &Signature) -> Self {
Self {
r: sig.r().to_be_bytes(),
s: sig.s().to_be_bytes(),
}
}

pub fn to_ref(&self) -> der::Result<SignatureRef<'_>> {
Ok(SignatureRef {
r: UintRef::new(&self.r)?,
s: UintRef::new(&self.s)?,
})
}
}

pub(crate) struct SignatureRef<'a> {
r: UintRef<'a>,
s: UintRef<'a>,
}
impl<'a> SignatureRef<'a> {
pub fn to_owned(&self) -> der::Result<Signature> {
let r = BoxedUint::from_be_slice(self.r.as_bytes(), self.r.as_bytes().len() as u32 * 8)
.map_err(|_| UintRef::TAG.value_error())?;
let s = BoxedUint::from_be_slice(self.s.as_bytes(), self.s.as_bytes().len() as u32 * 8)
.map_err(|_| UintRef::TAG.value_error())?;

let r = NonZero::new(r)
.into_option()
.ok_or(UintRef::TAG.value_error())?;
let s = NonZero::new(s)
.into_option()
.ok_or(UintRef::TAG.value_error())?;

Ok(Signature::from_components(r, s))
}

fn decode_value_inner<R: Reader<'a>>(reader: &mut R) -> der::Result<Self> {
Ok(SignatureRef {
r: UintRef::decode(reader)?,
s: UintRef::decode(reader)?,
})
}
}

impl<'a> DecodeValue<'a> for SignatureRef<'a> {
type Error = der::Error;

fn decode_value<R: Reader<'a>>(reader: &mut R, header: Header) -> der::Result<Self> {
reader.read_nested(header.length, Self::decode_value_inner)
}
}

impl EncodeValue for SignatureRef<'_> {
fn value_len(&self) -> der::Result<Length> {
self.r.encoded_len()? + self.s.encoded_len()?
}

fn encode_value(&self, writer: &mut impl Writer) -> der::Result<()> {
self.r.encode(writer)?;
self.s.encode(writer)?;
Ok(())
}
}
impl<'a> Sequence<'a> for SignatureRef<'a> {}