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

Getting the fingerprint of an RTCCertificate #738

Merged
merged 3 commits into from
Sep 23, 2016
Merged

Conversation

aboba
Copy link
Contributor

@aboba aboba commented Aug 10, 2016

Fix for Issue #720

@aboba aboba changed the title Getting the fingerprint of a RTCCertificate Getting the fingerprint of an RTCCertificate Aug 10, 2016
@@ -3657,6 +3657,7 @@ <h2 id="sec.cert-mgmt">Certificate Management</h2>
<div>
<pre class="idl">interface RTCCertificate {
readonly attribute DOMTimeStamp expires;
readonly attribute RTCDtlsFingerprint fingerprint;
Copy link
Member

Choose a reason for hiding this comment

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

And what if we want to include two hash functions?

@fluffy
Copy link
Contributor

fluffy commented Aug 23, 2016

Imagine that in the future we end up with a weak and strong hash algo during a transition time period and it is possible to cause collisions in the weak one. It seems like if one side will except the weak, then this open up for a MITM attack where the attacker takes the strong hash, then compute a weak hash that is the same. This may not be an issue but I would be more comfortable not accidentally comparing hash from two different hash algorithm. Due to that I would prefer some approach where the hash algo as well as the fingerprint was returned and be nice to support multiple hash algos.

@aboba
Copy link
Contributor Author

aboba commented Aug 24, 2016

There can only be one fingerprint and algorithm per certificate. Section 5.2.1 of JSEP states:

"the digest algorithm used for the fingerprint MUST match that used in the certificate signature."

@martinthomson
Copy link
Member

That's a terrible requirement, for the reasons @fluffy outlined. See https://tools.ietf.org/html/draft-ietf-mmusic-4572-update-05 and rtcweb-wg/jsep#305

@rshpount
Copy link

The requirement that "the digest algorithm used for the fingerprint MUST match that used in the certificate signature" was misguided even according to its author. There is no relationship between certificate signature and fingerprint hash. Furthermore, RFC 4572 was specifically updated to enable multiple fingerprints per certificate.

@fluffy
Copy link
Contributor

fluffy commented Aug 24, 2016

I think we need to have the IETF resolve the question of how security of fingerprints for certs works then we can easily make this API work for whatever the answer is.

@aboba
Copy link
Contributor Author

aboba commented Aug 24, 2016

@rshpount RFC 4572 Update does allow for multiple fingerprints per certificate:
https://tools.ietf.org/html/draft-ietf-mmusic-4572-update

However, JSEP only references RFC 4572, not the update.
Question posed in IETF RTCWEB WG as to whether JSEP should be revised:
https://www.ietf.org/mail-archive/web/rtcweb/current/msg16039.html

@alvestrand
Copy link
Contributor

If it's possible to generate a fingerprint with any algorithm, should we have a fingerprint() function that takes an algorithm as a parameter?

I think this is in "waiting for IETF" state.

@aboba
Copy link
Contributor Author

aboba commented Sep 1, 2016

@alvestrand As I understand it, JSEP will be updated to reference RFC 4572 Update (which allows multiple fingerprints).

Update to allow multiple fingerprints
@martinthomson
Copy link
Member

Two more problems:

  1. This needs to be asynchronous
  2. You can't have a dictionary attribute

Fix Travis breakage
@alvestrand
Copy link
Contributor

  1. Why asnc, given that we can precompute all the values we want to give (and only have to give one)?
  2. FrozenArray seems to be legal (used in IceServers).

@alvestrand alvestrand merged commit c594cd2 into master Sep 23, 2016
@martinthomson
Copy link
Member

I would strongly prefer that it be async because otherwise I have to always have a fingerprint available, which is more complicated to manage because there are many ways in which an RTCCertificate is instantiated. This is particularly relevant when it's persisted (to indexedDB for example) where it becomes much harder to manage, particularly if you want to change your opinion about the hash function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants