tink icon indicating copy to clipboard operation
tink copied to clipboard

Feature request: add create date / time on keys.

Open chanced opened this issue 5 years ago • 6 comments

It would be nice if keys stored their creation date/time. I need to be able to phase keys out of rotation based on their date. I'm not crazy about storing information about the keys outside of tink itself.

I could take a shot at putting together a pull request but Tink spreads across a couple of languages I'm not proficient in.

Thank you

chanced avatar Oct 20 '20 07:10 chanced

I have the same requirement and have been considering different approaches to work around not having creation timestamp in KeysetInfo, none of which seem ideal (separate metadata, abusing key IDs to embed a timestamp, etc).

@chanced Did you end up arriving at a satisfactory solution? Like you I dread having to synchronize metadata about keys in a keyset, external to the keyset.

Would a project maintainer be willing to weigh in with advice?

cpu avatar Mar 12 '21 15:03 cpu

This is, indeed, an issue we eventually need to address.

However, at the moment I personally tend to the variant that we have an always incrementing ID in the keyset instead of the creation time as it makes testing easier. Would that solve your problem?

At the moment, you will probably have to store the information outside of Tink.

tholenst avatar Mar 12 '21 15:03 tholenst

@tholenst Thanks for commenting :-)

However, at the moment I personally tend to the variant that we have an always incrementing ID in the keyset instead of the creation time as it makes testing easier. Would that solve your problem?

I will have to give this a little bit more thought but I think it's a step in the right direction. Probably if I can reframe my problem to be defined by the number of activated keys in the keyset and always deactivate the lowest ID first on a rotation event that would be sufficient 👍

I still think there are likely use-cases that would benefit from knowing the timespan between keys being added to the keyset. If the primary hesitance is the complexity with testing I'm not sure I'm fully swayed. It should be relatively straightforward to test with a mock source of time no?

cpu avatar Mar 12 '21 15:03 cpu

Yes, one can indeed make arguments for both, and I'm interested in hearing from people.

I just look at a lot of code in other libraries which do similar things and which has a creation time. Doing so, I found more issues with the creation time, where people worked around it. Conversely, almost nobody used it in a positive way and seemed glad that it existed. However, this may also be because usage within Google is going to be different from outside. (Also there are some other problems with the creation time, for example one has to answer the question whether the creation time of a public key in the public keyset is the time the public keyset was created, or the corresponding time in the private keyset -- most likely the correct answer is the one in the private keyset though, as that seems much more useful).

tholenst avatar Mar 12 '21 15:03 tholenst

I just look at a lot of code in other libraries which do similar things and which has a creation time. Doing so, I found more issues with the creation time, where people worked around it. Conversely, almost nobody used it in a positive way and seemed glad that it existed.

@tholenst Interesting! Could you briefly summarize some of the issues being worked around? I'm trying to plan a path forward and would appreciate making a few less of the mistakes others have already made :-)

It sounds like you surveyed internal code but generally I'm also interested in public examples of periodically rotating keys in a Tink keyset if you happen to have pointers handy.

Also there are some other problems with the creation time, for example one has to answer the question whether the creation time of a public key in the public keyset is the time the public keyset was created, or the corresponding time in the private keyset -- most likely the correct answer is the one in the private keyset though, as that seems much more useful

That's a good point. This is my first time taking Tink for a spin generally and have likely missed other potentials for complexity. I agree with you that I'd expect to be principally concerned with the private keyset but it does highlight that there is nuance here.

cpu avatar Mar 12 '21 15:03 cpu

@cpu No, I have backburnered refactoring my crypto package. Aside from being able to phase out keys, I have to refactor how I handle deterministic encryption (which relies on rotation).

However, at the moment I personally tend to the variant that we have an always incrementing ID in the keyset instead of the creation time as it makes testing easier. Would that solve your problem?

@tholenst Yes, a monotonically incrementing ID would be ideal, actually. Would the ID be separate of the current key ID? It may present challenges to existing implementations if the current ID generation became sequential. It would be a solvable challenge though, where all existing keys would need to be phased out.

One of the ways I'm using Tink is to generate my JWKS. It is a non-trivial task, in go at least, to get to the underlying data. I realize that the API isn't really designed for my usecase but my point is, I'll happily dig through it to acquire the newly minted sequential ids.

chanced avatar Mar 12 '21 18:03 chanced

In Java one can now use the KeysetHandle.Builder to assign ids manually. This allows users to use increasing IDs. I assume that there is no other need for this. If there is, please file another issue.

tholenst avatar Jan 26 '23 15:01 tholenst