Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add config option for adding additional TLS fingerprints #1167

Merged
merged 3 commits into from
Oct 12, 2016

Conversation

NegativeMjark
Copy link
Contributor

No description provided.

# Homeservers are permitted to cache the list of TLS fingerprints
# returned in the key responses. It may be necessary to publish the
# fingerprints of a new certificate and wait for the caches on other
# servers to expire before deploying it.
Copy link
Member

Choose a reason for hiding this comment

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

This worries me a bit if we're not explicit about the cache invaliadtion timings

# fingerprints of a new certificate and wait for the caches on other
# servers to expire before deploying it.
tls_fingerprints: []
#- {"sha256": "<base64_encoded_sha256_fingerprint>"}
Copy link
Member

Choose a reason for hiding this comment

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

This is a confusing way of giving an example, I was quite confused for a minute. Maybe:

tls_fingerprints: []
# tls_fingerprints: [{"sha256": "<base64_encoded_sha256_fingerprint>"}, ...]

@@ -49,7 +47,8 @@ class LocalKey(Resource):
"key": # base64 encoded NACL verification key.
}
}
"tls_certificate": # base64 ASN.1 DER encoded X.509 tls cert.
"tls_fingerprints": # Fingerprints of the TLS certs this server uses.
- {"sha256": "..."}
Copy link
Member

Choose a reason for hiding this comment

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

  1. This is JSON not yaml
  2. Are we actually changing the API here? Won't that affect when other servers look up keys?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We aren't changing the API. The comment is just a tiny bit out of date.

Copy link
Member

Choose a reason for hiding this comment

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

\o/

@erikjohnston
Copy link
Member

LGTM

@NegativeMjark NegativeMjark merged commit 9e18e0b into develop Oct 12, 2016
@richvdh richvdh deleted the markjh/fingerprints branch December 1, 2016 14:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants