Skip to content

async-signature: move to AFIT #1419

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.

Sign up for GitHub

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

Closed
baloo wants to merge 3 commits into from
Closed

async-signature: move to AFIT #1419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5202647
async-signature: move to `AFIT`
baloo Dec 14, 2023
eb3e80b
async-signature: introduce an `AsyncRandomizedSigner` trait
baloo Dec 14, 2023
cbc3e6c
WIP: async-signature: bring rustc beta for AFIT support
baloo Dec 14, 2023
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
3 changes: 1 addition & 2 deletions .github/workflows/async-signature.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ jobs:
strategy:
matrix:
rust:
- 1.60.0 # MSRV
- stable
- beta
steps:
- uses: actions/checkout@v4
- uses: RustCrypto/actions/cargo-cache@master
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/workspace.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
- uses: RustCrypto/actions/cargo-cache@master
- uses: dtolnay/rust-toolchain@master
with:
toolchain: 1.73.0
toolchain: beta
components: clippy
- run: cargo clippy --all --all-features --tests -- -D warnings

Expand All @@ -36,6 +36,6 @@ jobs:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@master
with:
toolchain: stable
toolchain: beta
components: rustfmt
- run: cargo fmt --all -- --check
3 changes: 2 additions & 1 deletion async-signature/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@ readme = "README.md"
keywords = ["crypto", "ecdsa", "ed25519", "signature", "signing"]
categories = ["cryptography", "no-std"]
edition = "2021"
rust-version = "1.60"
rust-version = "1.75"

[dependencies]
async-trait = "0.1.9"
signature = ">= 2.0, <2.3"

[features]
digest = ["signature/digest"]
rand_core = ["signature/rand_core"]

[package.metadata.docs.rs]
all-features = true
Expand Down
47 changes: 42 additions & 5 deletions async-signature/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ pub use signature::{self, Error};
#[cfg(feature = "digest")]
pub use signature::digest::{self, Digest};

use async_trait::async_trait;
#[cfg(feature = "rand_core")]
use signature::rand_core::CryptoRngCore;

/// Asynchronously sign the provided message bytestring using `Self`
/// (e.g. client for a Cloud KMS or HSM), returning a digital signature.
///
/// This trait is an async equivalent of the [`signature::Signer`] trait.
#[async_trait(?Send)]
#[allow(async_fn_in_trait)]
pub trait AsyncSigner<S: 'static> {
/// Attempt to sign the given message, returning a digital signature on
/// success, or an error if something went wrong.
Expand All @@ -33,7 +34,6 @@ pub trait AsyncSigner<S: 'static> {
async fn sign_async(&self, msg: &[u8]) -> Result<S, Error>;
}

#[async_trait(?Send)]
impl<S, T> AsyncSigner<S> for T
Copy link
Member Author

@baloo baloo Dec 18, 2023

Choose a reason for hiding this comment

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

I'd much rather have a impl<D, S, T> AsyncSigner<S> for T where T: AsyncDigestSigner<D, S> instead of this blanket.

Copy link
Member

@tarcieri tarcieri Dec 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

First, you cannot write such a blanket impl, because D is unconstrained (and indeed working around that issue is the whole purpose of the signature-derive:

error[E0207]: the type parameter `D` is not constrained by the impl trait, self type, or predicates

(And though PrehashSignature can define a default digest to use with a particular S, you still won't be able to write a valid blanket impl with it, because it still won't be constrained)

Second, these traits are intended to correspond 1:1 with the traits in signature. If there isn't a blanket impl between Signer and DigestSigner, it doesn't make sense to have one here because that would be inconsistent.

Third, these blanket impls exist so that you can use a Signer as an AsyncSigner, i.e. if you have an API that accepts AsyncSigner, anyone can plug in a Signer, and the same goes for all of the other traits, eliminating the need to duplicate APIs everywhere just for async.

Copy link
Member Author

@baloo baloo Dec 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I oversimplified the previous comment a bit, but yeah I intended:

impl<S, T> AsyncSigner<S> for T
where
    S: PrehashSignature + 'static,
    T: AsyncDigestSigner<S::Digest, S>,
{
    async fn sign_async(&self, msg: &[u8]) -> Result<S, Error> {
        self.sign_digest_async(S::Digest::new_with_prefix(msg))
            .await
    }
}

This is essentially what's in the #[derive(signature::Signer)] I guess I could add that to signature_derive then (should be in the dependency tree, given the proper feature-flags)?

Copy link
Member

@tarcieri tarcieri Dec 19, 2023

Choose a reason for hiding this comment

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

That does indeed seem to work, but signature has no such blanket impl, so I think it'd be weird for them to be inconsistent.

It could potentially be included in a hypothetical future signature 3.0, although the devil is in the details for these sort of blanket impls, and they may preclude overlapping impls with a valid use case.

where
S: 'static,
Expand All @@ -48,7 +48,7 @@ where
///
/// This trait is an async equivalent of the [`signature::DigestSigner`] trait.
#[cfg(feature = "digest")]
#[async_trait(?Send)]
#[allow(async_fn_in_trait)]
pub trait AsyncDigestSigner<D, S>
where
D: Digest + 'static,
Expand All @@ -60,7 +60,6 @@ where
}

#[cfg(feature = "digest")]
#[async_trait(?Send)]
impl<D, S, T> AsyncDigestSigner<D, S> for T
where
D: Digest + 'static,
Expand All @@ -71,3 +70,41 @@ where
self.try_sign_digest(digest)
}
}

/// Sign the given message using the provided external randomness source.
#[cfg(feature = "rand_core")]
#[allow(async_fn_in_trait)]
pub trait AsyncRandomizedSigner<S> {
/// Sign the given message and return a digital signature
async fn sign_with_rng_async(&self, rng: &mut impl CryptoRngCore, msg: &[u8]) -> S {
self.try_sign_with_rng_async(rng, msg)
.await
.expect("signature operation failed")
}

/// Attempt to sign the given message, returning a digital signature on
/// success, or an error if something went wrong.
///
/// The main intended use case for signing errors is when communicating
/// with external signers, e.g. cloud KMS, HSMs, or other hardware tokens.
async fn try_sign_with_rng_async(
&self,
rng: &mut impl CryptoRngCore,
msg: &[u8],
) -> Result<S, Error>;
}

#[cfg(feature = "rand_core")]
impl<S, T> AsyncRandomizedSigner<S> for T
where
S: 'static,
T: signature::RandomizedSigner<S>,
{
async fn try_sign_with_rng_async(
&self,
rng: &mut impl CryptoRngCore,
msg: &[u8],
) -> Result<S, Error> {
self.try_sign_with_rng(rng, msg)
}
}