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

Use the certificate fingerprint for the key id #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

joshperry
Copy link

Bumped the version of forge to the latest which will calculate a pretty standard SHA1 fingerprint. Use this fingerprint as the kid value. Just think it is a better identifier than exposing the disk filename.

@jaredhanson
Copy link
Owner

Would this be the correct thing to place in the x5t field?
http://tools.ietf.org/html/draft-ietf-jose-json-web-key-31#section-4.8

If you want the kid to equal the fingerprint, I'd recommend naming the files after the fingerprint. I personally use filenames (and thus kids) as dates (20140715.pem). These IDs are sent to other applications (which may cache them) and need to be consistent for correct processing, so this change would break existing applications. I think making an assumption as to what the correct kid value is too inflexible, and should be left up to the application (which can name files whatever it wants).

I'd take a patch that includes the fingerprint in the x5t field, however, assuming that is the correct place for it.

@joshperry
Copy link
Author

Good points, I don't need the fingerprint itself so x5t isn't very important for me. I just didn't like exposing the filenames and the thumbprint is often used to uniquely identify certificates. Like you said though, there's no reason I can't have the filename be 35311d2b1b694455dbf6fa8a52a3f76da500503b.

Re: caching, I was actually also going to work on a patch to set HTTP caching headers based on the file modify dates, cert dates, or maybe even just a hash of the certificate serial numbers. This would allow an application to cache all the certs until any of them were changed. Not sure if it is a common use-case to have a large number of keys published, this would be less optimal than caching on a date-based kid if so.

Thoughts?

@jaredhanson
Copy link
Owner

I'm all in favor of setting HTTP headers that help in caching. I'd lean in favor of modify dates on the files, rather than cert serial numbers. JWKS supports raw public keys, not just certs, which don't have a lot of the extraneous metadata, so it's best target the common properties (fs times in this case).

@joshperry
Copy link
Author

Good point about naked keys.

How would you feel about having an extensible provider system for providing the key and any metadata from some source. For example, a provider that enumerated files and provided the keys and file time metadata, and another for storing keys in a database. It would be much easier for us to manage the keys in Mongo or something vs. the filesystem.

This would allow anyone to set up a provider however they'd like to provide the required key material and metadata.

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

2 participants