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

Edge local fingerprint looks like SDP ones #683

Closed
ibc opened this issue Apr 25, 2017 · 11 comments
Closed

Edge local fingerprint looks like SDP ones #683

ibc opened this issue Apr 25, 2017 · 11 comments

Comments

@ibc
Copy link
Contributor

ibc commented Apr 25, 2017

In Edge latest version, the fingerprints values of iceGatherer.getLocalParameters() have a value such as "75:1B:81:93:B7:ED:27:7E:42:BE:D6:C4:8E:F7:04:3A:49:CE:3F:EE" rather than "751b8193b7ed277e42bed6c48ef7043a49ce3fee".

Should I understand that this is a bug in Edge? or should implementations be ready to deal with both syntaxes (by detecting usage of ":")?

@fippo
Copy link
Contributor

fippo commented Apr 25, 2017

It has been like that forever. The spec says

The value of the certificate fingerprint in lowercase hex string as expressed utilizing the syntax of 'fingerprint' in [RFC4572] Section 5.

which, if you go to https://tools.ietf.org/html/rfc4572#section-5 is

The hash value is represented as a sequence of uppercase hexadecimal bytes, separated by colons

It is somewhat odd to require lowercase while 4572 says uppercase.

@ibc
Copy link
Contributor Author

ibc commented Apr 25, 2017

Thanks. And sorry, I meant RTCDtlsTransport. So it seems the ORTC spec is wrong regarding this. What about mandating uppercase separated by colons and that's all?

@robin-raymond
Copy link
Contributor

The specs themselves are inconsistent. We went back and forth on this one... I personally favour JS style lowercase byte arrays without : but to make the shims easier we followed the spec. The trouble is one spec mandates lower case : and another uppercase : and the examples/samples are inconsistent. Sadly, I think it's best clients/implementations handle any of these cases, with or without :.

@ibc
Copy link
Contributor Author

ibc commented Apr 25, 2017

The trouble is one spec mandates lower case : and another uppercase :

Well, what I see is that one spec mandates lowercase without : by wrongly referring to another spec that mandates the opposite.

@robin-raymond
Copy link
Contributor

@ibc no, the two specs I'm referring to are not the ORTC spec. There are two "related" specs with differing versions.

@ibc
Copy link
Contributor Author

ibc commented Apr 25, 2017

Fine but, anyhow, this is wrong:

The value of the certificate fingerprint in lowercase hex string as expressed utilizing the syntax of 'fingerprint' in [RFC4572] Section 5

This is just about removing such a reference to RFC 4572.

@robin-raymond
Copy link
Contributor

@ibc I agree and would say if we are mandating it look like this RFC with the : then it should be uppercase like this specification despite other conflicting specifications that exist since this is likely the most authoritative of the ones we want to model.

@ibc
Copy link
Contributor Author

ibc commented Apr 25, 2017

Agreed. Also, AFAIK, there are some case-sensitive implementations, so forcing it to be always uppercase seems to make things easier.

@lgrahl
Copy link
Contributor

lgrahl commented Apr 25, 2017

I'd also upvote uppercase.

@aboba
Copy link
Contributor

aboba commented May 12, 2017

I vote for uppercase as in RFC 4572 update.

@robin-raymond
Copy link
Contributor

Ok, uppercase it will be with : between bytes.

aboba added a commit that referenced this issue May 18, 2017
@aboba aboba closed this as completed May 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants