object: introduce signature abstraction#705
Closed
hiddeco wants to merge 2 commits intogo-git:masterfrom
Closed
Conversation
hiddeco
force-pushed
the
signature-abstraction
branch
7 times, most recently
from
73ebe5c to
95b3408
Compare
This refactors the existing PGP verification and signing code into an abstraction which allows multiple signature types to exist in a backwards compatible manner. Signed-off-by: Hidde Beydals <[email protected]>
Signed-off-by: Hidde Beydals <[email protected]>
hiddeco
force-pushed
the
signature-abstraction
branch
from
11ec329 to
e1b64bc
Compare
Member
Author
|
Talked with @pjbgf about this last week, and due to the lack of time I have to finalize the "nice" user part of SSH signatures on user systems (i.e. by taking the We thought it may be an option to introduce the abstraction without providing an implementation with a "keyring" like functionality. As the abstraction itself seems sound, and works as is. Any objections against this @mcuadros? If not, I will rebase the PR and spend one more time thinking about the details of the abstraction itself (e.g. the |
wlynch
added a commit
to wlynch/go-git
that referenced
this pull request
Feb 13, 2024
crypto.Signer was incorrectly used before. Signer documentation says that Signer.Sign should be used on digests, whereas we were using this on message bodies. To fix this, create our own Signer interface (+ signableObject borrowed from go-git#705) that describes more accurately what we want. As before, the expectation is that signer implementations only need to worry about acting on encoded message bodies rather than needing to encode objects themselves. This is technically a breaking change from the previous Signer implementation, but since this is new and hasn't made it into cut release yet, this seems like an acceptible change. Also adds example test showing how signers can be made (uses base64 for consistent outputs).
wlynch
added a commit
to wlynch/go-git
that referenced
this pull request
Feb 13, 2024
crypto.Signer was incorrectly used before. Signer documentation says that Signer.Sign should be used on digests, whereas we were using this on message bodies. To fix this, create our own Signer interface (+ signableObject borrowed from go-git#705) that describes more accurately what we want. As before, the expectation is that signer implementations only need to worry about acting on encoded message bodies rather than needing to encode objects themselves. This is technically a breaking change from the previous Signer implementation, but since this is new and hasn't made it into cut release yet, this seems like an acceptible change. Also adds example test showing how signers can be made (uses base64 for consistent outputs).
wlynch
added a commit
to wlynch/go-git
that referenced
this pull request
Feb 13, 2024
crypto.Signer was incorrectly used before. Signer documentation says that Signer.Sign should be used on digests, whereas we were using this on message bodies. To fix this, create our own Signer interface (+ signableObject borrowed from go-git#705) that describes more accurately what we want. As before, the expectation is that signer implementations only need to worry about acting on encoded message bodies rather than needing to encode objects themselves. This is technically a breaking change from the previous Signer implementation, but since this is new and hasn't made it into cut release yet, this seems like an acceptible change. Also adds example test showing how signers can be made (uses base64 for consistent outputs).
wlynch
added a commit
to wlynch/go-git
that referenced
this pull request
Feb 13, 2024
crypto.Signer was incorrectly used before. Signer documentation says that Signer.Sign should be used on digests, whereas we were using this on message bodies. To fix this, create our own Signer interface (+ signableObject borrowed from go-git#705) that describes more accurately what we want. As before, the expectation is that signer implementations only need to worry about acting on encoded message bodies rather than needing to encode objects themselves. This is technically a breaking change from the previous Signer implementation, but since this is new and hasn't made it into cut release yet, this seems like an acceptible change. Also adds example test showing how signers can be made (uses base64 for consistent outputs).
github-actions
bot
added
the
stale
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
2 participants
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.
This is work in progress to further facilitate #400. As this introduces (non-breaking) but important contract changes, I would like to have eyes on this early on.