add support for ecdsa signatures #108

Geal · 2022-10-11T19:21:18Z

Support for ECDSA signatures on curve secp256r1 biscuit#165

Since we can now have multiple algorithms, we cannot know during deserialization the right size of a signature, so that check is now removed. A signature with an invalid size will generate an error later during verification anyway

codspeed-hq · 2024-05-26T20:33:02Z

CodSpeed Performance Report

Merging #108 will not alter performance

_{Comparing fipscuit (543d0f1) with v5 (4556320)}

Summary

✅ 12 untouched benchmarks

codecov · 2024-05-26T20:34:53Z

Codecov Report

Attention: Patch coverage is 62.80193% with 154 lines in your changes missing coverage. Please review.

Please upload report for BASE (v5@4556320). Learn more about missing BASE report.
Report is 1 commits behind head on v5.

Files with missing lines	Patch %	Lines
biscuit-auth/src/crypto/ed25519.rs	54.32%	37 Missing ⚠️
biscuit-auth/src/crypto/p256.rs	55.26%	34 Missing ⚠️
biscuit-auth/src/crypto/mod.rs	72.97%	30 Missing ⚠️
biscuit-auth/src/token/builder.rs	32.00%	17 Missing ⚠️
biscuit-capi/src/lib.rs	0.00%	14 Missing ⚠️
biscuit-auth/src/token/unverified.rs	0.00%	11 Missing ⚠️
biscuit-parser/src/builder.rs	0.00%	4 Missing ⚠️
biscuit-auth/src/token/mod.rs	76.92%	3 Missing ⚠️
biscuit-auth/examples/third_party.rs	0.00%	2 Missing ⚠️
biscuit-auth/examples/testcases.rs	98.33%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@          Coverage Diff          @@
##             v5     #108   +/-   ##
=====================================
  Coverage      ?   65.16%           
=====================================
  Files         ?       27           
  Lines         ?     6972           
  Branches      ?        0           
=====================================
  Hits          ?     4543           
  Misses        ?     2429           
  Partials      ?        0

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests

divarvel · 2024-11-19T15:29:25Z

biscuit-auth/benches/token.rs

@@ -11,7 +11,7 @@ use rand::rngs::OsRng;

 fn create_block_1(b: &mut Bencher) {
    let mut rng = OsRng;
-    let root = KeyPair::new_with_rng(&mut rng);
+    let root = KeyPair::new_with_rng(Algorithm::Ed25519, &mut rng);


it could be done in another PR, but adding benches for other signature algorithms would be nice (eg making the bench suite parametric on the algorithm, and running it for each supported algorithm)

biscuit-auth/src/crypto/mod.rs

biscuit-auth/src/datalog/symbol.rs

biscuit-auth/src/token/builder.rs

divarvel · 2024-11-19T15:49:10Z

biscuit-auth/src/token/unverified.rs

@@ -99,7 +101,8 @@ impl UnverifiedBiscuit {
    /// since the public key is integrated into the token, the keypair can be
    /// discarded right after calling this function
    pub fn append(&self, block_builder: BlockBuilder) -> Result<Self, error::Token> {


i assume the primary use-case for ECDSA is for the authority block and external signatures, and that we’re ok using ed25519 for attenuation blocks

if people are using ECDSA somewhere, it is likely that they would use it everywhere. Would you prefer that the algorithm stays the same over the entire token?

i assume the need for using ecdsa is mostly for sensitive keys that need hardware support (eg HSM / smart card, etc). Block keys are not as sensitive as they’re single use, so i guess defaulting to ed25519 is ok here. Also, block keys need the secret key to be accessed directly and stored in the token, so that would not work with a HSM

biscuit-auth/src/token/unverified.rs

biscuit-auth/src/token/mod.rs

biscuit-auth/src/capi.rs

divarvel · 2024-11-19T15:53:16Z

biscuit-auth/Cargo.toml

@@ -50,6 +50,10 @@ biscuit-parser = { version = "0.1.2", path = "../biscuit-parser" }
 biscuit-quote = { version = "0.2.2", optional = true, path = "../biscuit-quote" }
 chrono = { version = "0.4.26", optional = true, default-features = false, features = ["serde"] }
 serde_json = "1.0.117"
+ecdsa = { version = "0.16.9", features = ["signing", "verifying", "pem", "alloc", "pkcs8", "serde"] }


should we feature-gate ecdsa support?

we can, but what would it bring? Reducing the number of dependencies?

yes, that’s what we’ve done for pem/der support iirc

the API already exposes the algorithm in key creation everywhere, feature gating it after that would make it a little bit more annoying

Co-authored-by: Clément Delafargue <[email protected]>

divarvel · 2024-11-19T19:59:12Z

biscuit-auth/Cargo.toml

@@ -50,6 +50,10 @@ biscuit-parser = { version = "0.1.2", path = "../biscuit-parser" }
 biscuit-quote = { version = "0.2.2", optional = true, path = "../biscuit-quote" }
 chrono = { version = "0.4.26", optional = true, default-features = false, features = ["serde"] }
 serde_json = "1.0.117"
+ecdsa = { version = "0.16.9", features = ["signing", "verifying", "pem", "alloc", "pkcs8", "serde"] }


yes, that’s what we’ve done for pem/der support iirc

divarvel · 2024-11-19T20:07:39Z

biscuit-auth/src/token/unverified.rs

@@ -99,7 +101,8 @@ impl UnverifiedBiscuit {
    /// since the public key is integrated into the token, the keypair can be
    /// discarded right after calling this function
    pub fn append(&self, block_builder: BlockBuilder) -> Result<Self, error::Token> {


i assume the need for using ecdsa is mostly for sensitive keys that need hardware support (eg HSM / smart card, etc). Block keys are not as sensitive as they’re single use, so i guess defaulting to ed25519 is ok here. Also, block keys need the secret key to be accessed directly and stored in the token, so that would not work with a HSM

Geal added 8 commits October 11, 2022 21:08

add a signature type

ca5e60a

helper methods

863c723

indicate the algorithm of the next private key

b242be8

cleanup

92da6e8

start the boilerplate around managing multiple types of keys

13581cf

start support for p256 signatures

be9e6a4

p256 support

d0113b3

Merge branch 'master' into fipscuit

36e7362

divarvel mentioned this pull request Jul 5, 2023

Roadmap biscuit-auth/biscuit#12

Open

Geal added 3 commits August 24, 2023 21:45

Merge branch 'main' into fipscuit

ba0f80e

reintroduce ed25519-dalek 2.0

ad56ade

Merge branch 'main' into fipscuit

d0f335f

Geal mentioned this pull request Jan 23, 2024

DID / DPKI integration biscuit-auth/biscuit#105

Open

Geal added 4 commits March 26, 2024 21:25

Merge branch 'main' into fipscuit

b52b929

fix testcases

60514db

move impl

92f88f8

PEM support

9a8cd39

Geal mentioned this pull request May 12, 2024

Support for signing algorithms found in AWS KMS biscuit-auth/biscuit-java#93

Open

Merge branch 'main' into fipscuit

daa0bfd

Geal mentioned this pull request May 20, 2024

Support for ECDSA signatures on curve secp256r1 biscuit-auth/biscuit#165

Merged

4 tasks

wip

2985990

Geal changed the base branch from main to v5 May 23, 2024 07:38

Geal added 8 commits May 23, 2024 09:39

Merge branch 'v5' into fipscuit

410340e

update protobuf enum

27da40e

update serialization formats

93c2aae

remove unuse code

f104b65

add samples

047ded5

Since we can now have multiple algorithms, we cannot know during deserialization the right size of a signature, so that check is now removed. A signature with an invalid size will generate an error later during verification anyway

parsing and printing

3bd1d04

Merge branch 'v5' into fipscuit

d9d68bc

fix sample file name

5aeb701

Geal marked this pull request as ready for review May 23, 2024 12:40

Geal added 5 commits May 25, 2024 14:52

Merge branch 'v5' into fipscuit

0c97e37

specify the algorithm when creating a KeyPair

ce02b60

capi fixes

8108420

Merge branch 'v5' into fipscuit

c7378ce

fix samples build

c88fff0

Geal added 6 commits June 15, 2024 15:58

make the algorithm Copy

346d4de

Merge branch 'v5' into fipscuit

d58d352

Merge branch 'v5' into fipscuit

56fa6c3

update for bwk

69f58dd

Merge branch 'v5' into fipscuit

cfc7de7

update bwk

86323c6

divarvel reviewed Nov 19, 2024

View reviewed changes

Geal and others added 6 commits November 19, 2024 18:57

remove the Display implementation for public keys

ae3d779

replace the Display implementation

b0490ae

From implementations

bdb8c74

update p256

da9dade

add a test for ecdsa parsing

1e080fb

Update biscuit-auth/src/token/mod.rs

caeaad9

Co-authored-by: Clément Delafargue <[email protected]>

divarvel approved these changes Nov 19, 2024

View reviewed changes

Geal added 3 commits November 19, 2024 21:57

Merge branch 'v5' into fipscuit

8ec8bba

Merge branch 'v5' into fipscuit

50b6306

fix key_pair_new usage

543d0f1

Geal merged commit d956655 into v5 Nov 20, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support for ecdsa signatures #108

add support for ecdsa signatures #108

Geal commented Oct 11, 2022 •

edited

Loading

codspeed-hq bot commented May 26, 2024 •

edited

Loading

codecov bot commented May 26, 2024 •

edited

Loading

divarvel Nov 19, 2024

divarvel Nov 19, 2024

Geal Nov 19, 2024

divarvel Nov 19, 2024

divarvel Nov 19, 2024

Geal Nov 19, 2024

divarvel Nov 19, 2024

Geal Nov 20, 2024

divarvel Nov 19, 2024

divarvel Nov 19, 2024

add support for ecdsa signatures #108

add support for ecdsa signatures #108

Conversation

Geal commented Oct 11, 2022 • edited Loading

codspeed-hq bot commented May 26, 2024 • edited Loading

Merging #108 will not alter performance

Summary

codecov bot commented May 26, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Geal commented Oct 11, 2022 •

edited

Loading

codspeed-hq bot commented May 26, 2024 •

edited

Loading

codecov bot commented May 26, 2024 •

edited

Loading