Closed
Conversation
Contributor
Author
|
@leelynne Can you take a look at this PR? Let me know if you have feedback/suggestions. |
Contributor
Author
|
@leelynne Do you know when you'll be able to take a look at this PR? |
getvictor
added a commit
to fleetdm/fleet
that referenced
this pull request
Jul 14, 2025
For #30473 This change adds a vendored `httpsig-go` library to our repo. We cannot use the upstream library because it has not merged the change we need: remitly-oss/httpsig-go#25 Thus, we need our own copy at this point. The instructions for keeping this library up to date (if needed) are in `UPDATE_INSTRUCTIONS`. None of the coderabbitai review comments are relevant to the code/features we are going to use for HTTP message signatures. We will use this library in subsequent PRs for the TPM-backed HTTP message signature feature. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Introduced a Go library for HTTP message signing and verification, supporting multiple cryptographic algorithms (RSA, ECDSA, Ed25519, HMAC). * Added utilities for key management, including JWK and PEM key handling. * Provided HTTP client and server helpers for automatic request signing and signature verification. * Implemented structured error handling and metadata extraction for signatures. * **Documentation** * Added comprehensive README, usage examples, and update instructions. * Included license and configuration files for third-party and testing tools. * **Tests** * Added extensive unit, integration, and fuzz tests covering signing, verification, and key handling. * Included official RFC test vectors and various test data files for robust validation. * **Chores** * Integrated continuous integration workflows and ignore files for code quality and security analysis. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Collaborator
|
@getvictor My apologies I need to get my github notifications in order! Taking a look now. |
leelynne
requested changes
Jul 21, 2025
Collaborator
leelynne
left a comment
There was a problem hiding this comment.
Would be able to able to add a test for usage of crypto.Signer? If not, no worries I will add the funcitonality and test. I need to add a set of tests for sign anyway.
Contributor
Author
|
Please add a test. I'm confident it works since we have integration tests using this library, and we'll be shipping it to production in ~2 weeks (after QA is done). Also, I assume you will merge the PRs that you approved. I don't have permissions to merge them myself. |
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I am using HTTP signatures with a TPM (Trusted Platform Module), where the private key for the algorithm is located in the hardware, and I must use system APIs for signing.
This change allows the usage of a custom
crypto.Signer:I have verified this as working on our project, and we plan to put these changes into production within the next month:
https://github.com/fleetdm/fleet/tree/victor/29935-tpm-agent