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

packet.Config.Time should return time with 999999999 nsec #240

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DmitriyMV
Copy link
Contributor

Currently verifySignature accepts seconds for verification, but internally it uses time.Time for comparison. That means if we created openpgp.Entity with default time field, test can fail with "key expired" error because PublicKey.KeyExpired will think that key creation happened before verifyTime when in reality we just lost the nsec precision.

Currently verifySignature accepts seconds for verification, but internally it
uses time.Time for comparison. That means if we created openpgp.Entity with
default time field, test can fail with "key expired" error because PublicKey.KeyExpired
will think that key creation happened before verifyTime when in reality
we just lost the nsec precision.

Signed-off-by: Dmitriy Matrenichev <[email protected]>
@DmitriyMV
Copy link
Contributor Author

Relevant test added that covers both this and #231

@twiss
Copy link
Member

twiss commented May 15, 2023

Hello 👋 Timestamps in OpenPGP only have second precision. I assume that if you'd serialize and parse the key before using it, this issue would go away - which implies that the fix should be something else, i.e. round down the current time when generating keys (and signatures). Though, that would probably need to be done in go-crypto, not gopenpgp.

@DmitriyMV
Copy link
Contributor Author

DmitriyMV commented May 15, 2023

@twiss
Copy link
Member

twiss commented May 15, 2023

The thing is that both pk.CreationTime and sig.CreationTime, when parsed, come from an integer-precision timestamp (see section 3.5 of RFC 4880). So in that case, truncating them should be unnecessary - but when the objects are created directly (e.g. in NewRSAPublicKey, etc), rather than parsed, we should probably truncate the times, for consistency, yeah. Or, alternatively, we could do it in config.Now - that might be simplest.

@DmitriyMV
Copy link
Contributor Author

openpgp.Entity.EncryptionKey is public method which accepts non-truncated time. I suppose there are also other ways to pass nsec time to KeyExpired functions. Truncating there should safe-guard against that getting in.

@DmitriyMV
Copy link
Contributor Author

@twiss ProtonMail/go-crypto#168 like this?

@lubux lubux added the v2 Targeting GopenPGP v2 label Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2 Targeting GopenPGP v2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants