[jitsi-dev] [libjitsi] Added certificate signature algorithm override for DTLS (#24)


#1

Added support for overriding the x509 certificate signature algorithm via system property when use in DTLS with WebRTC. If the system property is not overridden, the "old" value of SHA1withRSA is used. Allowing this to be overridden provides better support for Chrome and Firefox. To use the override, add this Java option to your startup: -Dorg.jitsi.impl.transform.dtls.SIGNATURE_ALGORITHM=SHA256withRSA
You can merge this Pull Request by running:

  git pull https://github.com/mondain/libjitsi patch-2

Or you can view, comment on it, or merge it online at:

  https://github.com/jitsi/libjitsi/pull/24

-- Commit Summary --

  * Added certificate signature algorithm override for DTLS

-- File Changes --

    M src/org/jitsi/impl/neomedia/transform/dtls/DtlsControlImpl.java (17)

-- Patch Links --

https://github.com/jitsi/libjitsi/pull/24.patch
https://github.com/jitsi/libjitsi/pull/24.diff

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/24


#2

It would be better if this were handled through the ConfigService instead of a system property.
Also please add neomedia (i.e. the complete package name) to the property name.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/24#issuecomment-65549050


#3

The placement of the code and property key were suggested by Boris. I am not familiar with the ConfigService, but if that is only for OSGI; I would suggest against using it. Also I updated the property name to reflect the full package path.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/24#issuecomment-65624915


#4

When Chrome and Firefox create their local certificates, they default to SHA256 presently. I have read that Chrome and Firefox will accept a peer using SHA1, but I don't know if that is still in effect. Either way the support for multiple hashing routines needs to be available. More info on this:

RFC 5763 and RFC 5764 (DTLS-SRTP) do not mandate a specific hash for the fingerprint attribute in the SDP. RFC 5763 refers to RFC 4572 "Connection-Oriented Media Transport over TLS in SDP”. Its section 5 clearly opens the door to multiple hash functions:

   hash-func    =  "sha-1" / "sha-224" / "sha-256" /
                         "sha-384" / "sha-512" /
                         "md5" / "md2" / token
                         ; Additional hash functions can only come
                         ; from updates to RFC 3279
···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/24#issuecomment-65674951


#5

The ConfigService is not only for OSGi, it controls almost everything configurable inside libjitsi. See e.g. https://github.com/jitsi/libjitsi/blob/master/src/org/jitsi/impl/neomedia/device/DeviceConfiguration.java#L325 for an example on how to use it.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/24#issuecomment-65718257


#6

Sunsetting Sha-1 http://googleonlinesecurity.blogspot.ca/2014/09/gradually-sunsetting-sha-1.html

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/24#issuecomment-74083927


#7

I still don't mind having this configurable, but not from a system property. Please use the config service as mentioned before. And while you're at it, please respect the coding guidelines with the line length limit of 80 chars.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/24#issuecomment-74087869


#8

I don't get why you all format the way that you do; are you guys using old 40 column CRT monitors? Irregardless, I have been trying to format your way with these pulls, so it must have slipped through the cracks. Lastly, I've done this pull twice already, so if the config service is the final way to go I'll be glad to modify this a third time. Keep up the excellent work.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/24#issuecomment-74092909


#9

If you're still interested in bringing this up-to-date, please do.
- I don't know how 80 chars came into existence originally, AFAIK its a preference of Emil. For new code the point is consistency. Personally I don't like it either.
- Sorry for doing so much work about this simple change. But yes, the config service is the preferred way to override the default if the service is available.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/24#issuecomment-145361218


#10

I'll try to get an update for this in the next few days.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/24#issuecomment-148113870


#11

Thanks!
There seem to be conflicts now, can you please rebase your patch on master? Also there are still some formatting issues and the config service needs to be null-checked before accessed.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/24#issuecomment-151397038


#12

Ok, will do.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/24#issuecomment-151466798


#13

I used rebase, if this seems confusing @ibauersachs, then I will gladly resubmit this from a new "cleaner" fork.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/24#issuecomment-151482787


#14

This seems like a merge rather a rebase - and it still conflicts. Please also squash your commits. Otherwise the actual change looks okay now.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/24#issuecomment-151498266


#15

I'm gonna close and recreate this from master, my commit is fairly simple and doesn't need all this extra baggage.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/24#issuecomment-151503958


#16

Closed #24.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/libjitsi/pull/24#event-446795930