Skip to content

Conversation

haydentherapper
Copy link
Contributor

Tink v2.3.0 [1] added the ability to extract individual keys from a Tink keyset.Handle as key objects. This PR modifies KeyHandleToSigner to use such APIs, significantly simplifying it. Note that there is no need to validate keys because they are validated when the keyset.Handle is created [2],[3],[4].

Also refactoring signer_test.go and adding a test case for an invalid key type.

[1] https://github.com/tink-crypto/tink-go/releases/tag/v2.3.0
[2] https://github.com/tink-crypto/tink-go/blob/v2.3.0/signature/ecdsa/key.go#L380
[3] https://github.com/tink-crypto/tink-go/blob/v2.3.0/signature/ecdsa/key.go#L167
[4] https://github.com/tink-crypto/tink-go/blob/v2.3.0/signature/ed25519/key.go#L206-L209

Summary

Release Note

Documentation

Tink v2.3.0 [1] added the ability to extract individual keys from a
Tink `keyset.Handle` as key objects. This PR modifies
`KeyHandleToSigner` to use such APIs, significantly simplifying it.
Note that there is no need to validate keys because they are validated
when the `keyset.Handle` is created [2],[3],[4].

Also refactoring `signer_test.go` and adding a test case for an
invalid key type.

[1] https://github.com/tink-crypto/tink-go/releases/tag/v2.3.0
[2] https://github.com/tink-crypto/tink-go/blob/v2.3.0/signature/ecdsa/key.go#L380
[3] https://github.com/tink-crypto/tink-go/blob/v2.3.0/signature/ecdsa/key.go#L167
[4] https://github.com/tink-crypto/tink-go/blob/v2.3.0/signature/ed25519/key.go#L206-L209

Signed-off-by: Moreno Ambrosin <ambrosin@google.com>
@haydentherapper
Copy link
Contributor Author

This is just #1981 rebased against main, and fixing a lint error.

Copy link

codecov bot commented Apr 23, 2025

Codecov Report

Attention: Patch coverage is 72.97297% with 10 lines in your changes missing coverage. Please review.

Project coverage is 43.75%. Comparing base (cf238ac) to head (a84be00).
Report is 357 commits behind head on main.

Files with missing lines Patch % Lines
pkg/ca/tinkca/signer.go 72.97% 7 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2024       +/-   ##
===========================================
- Coverage   57.93%   43.75%   -14.19%     
===========================================
  Files          50       73       +23     
  Lines        3119     5696     +2577     
===========================================
+ Hits         1807     2492      +685     
- Misses       1154     2983     +1829     
- Partials      158      221       +63     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

haydentherapper added a commit to haydentherapper/sigstore that referenced this pull request Apr 23, 2025
From sigstore/fulcio#2024. Will update Fulcio
and other Sigstore repos to use this shared implementation.

Signed-off-by: Hayden B <8418760+haydentherapper@users.noreply.github.com>
encodingName := ecdsapb.EcdsaSignatureEncoding_name[int32(params.GetEncoding())]
return hashName, curveName, encodingName
}
curve, err := curveFromTinkECDSACurveType(ecdsaPublicKey.Parameters().(*tinkecdsa.Parameters).CurveType())
Copy link
Member

Choose a reason for hiding this comment

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

not sure if a defensive check for ecdsaPublicKey.Parameters() being nil is worth it here

@haydentherapper
Copy link
Contributor Author

Going to hold on this and replace this with the impl in sigstore/sigstore

@haydentherapper haydentherapper marked this pull request as draft April 24, 2025 15:39
haydentherapper added a commit to sigstore/sigstore that referenced this pull request Apr 24, 2025
* Update to use Tink v2.3.0 API

From sigstore/fulcio#2024. Will update Fulcio
and other Sigstore repos to use this shared implementation.

Signed-off-by: Hayden B <8418760+haydentherapper@users.noreply.github.com>

* Fix linting errors

Signed-off-by: Hayden B <8418760+haydentherapper@users.noreply.github.com>

---------

Signed-off-by: Hayden B <8418760+haydentherapper@users.noreply.github.com>
Co-authored-by: Hayden B <8418760+haydentherapper@users.noreply.github.com>
@haydentherapper
Copy link
Contributor Author

The signer in this PR has been moved to sigstore/sigstore, #2026 instead

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

Successfully merging this pull request may close these issues.

4 participants