Skip to content
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.

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

Add APIs to get the creation time of verified detached signatures #157

Merged
merged 2 commits into from
Dec 21, 2021

Conversation

marinthiercelin
Copy link
Collaborator

No description provided.

crypto/signature.go Outdated Show resolved Hide resolved
// with a PGPMessage containing an encrypted detached signature
// returns the creation time of the signature if it succeeds
// and returns a SignatureVerificationError if fails.
func (keyRing *KeyRing) GetVerifiedEncryptedSignatureTimestamp(message *PlainMessage, encryptedSignature *PGPMessage, decryptionKeyRing *KeyRing, verifyTime int64) (int64, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have a use-case in mind for this? It's not needed by my side

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think so, it's more for consistency but we can ditch it

// returns the creation time of the signature if it succeeds
// and returns a SignatureVerificationError if fails.
func (keyRing *KeyRing) GetVerifiedSignatureTimestamp(message *PlainMessage, signature *PGPSignature, verifyTime int64) (int64, error) {
err := keyRing.VerifyDetached(message, signature, verifyTime)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here it verifies all the signatures, but then later we return the timestamp of the first one if any of them succeeds. We should return the timestamp of the succeeding one

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can you have several sigs in a detached signature ?
If yes, how would I find out which one succeeded ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, it can happen. A simple solution here is to accept just one, since the crypto API CheckDetachedSignatureAndHash returns only the signer entity, and not the signature packet.

So basically instead of using signature.getCreationTime() you can just check if the first packet is a signature, then serialise just that one and verify it with verifySignature

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wouldn't this lead to inconsistency between VerifyDetached and GetVerifiedSignatureTimestamp ?
Some sigs would be accepted by the first but not the other one

Copy link
Collaborator

Choose a reason for hiding this comment

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

Indeed. The other solution I see is to do the same but manually iterating over the signature packets, verifying one by one and returning the verifying one's timestamp

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok I tried going over each packet in a loop and return the time stamp for the first packet that verifies

CHANGELOG.md Outdated Show resolved Hide resolved
marinthiercelin and others added 2 commits December 21, 2021 10:02
Add a function to verify a detached signature and access
its creation time.
@wussler wussler merged commit e8c7fa3 into master Dec 21, 2021
@lubux lubux deleted the feat/signature_creation_time branch June 14, 2024 13:03
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.

None yet

4 participants