[jitsi-dev] [jitsi] Logging of TLS protocols and cipher suites available during connection ... (#20)


#1

...establishment and then the chosen ones.

Add ability to access TLS cipher suite, protocol name and server certificate using an new OperationSet. Only implemented for XMPP so far. Also makes this information available in the call information frame.
Frame for showing the certificate details.
You can merge this Pull Request by running:

  git pull https://github.com/netmackan/jitsi tls-3

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

  https://github.com/jitsi/jitsi/pull/20

-- Commit Summary --

  * Logging of TLS protocols and cipher suites available during connection establishment and then the chosen ones.

-- File Changes --

    M resources/languages/resources.properties (6)
    M src/net/java/sip/communicator/impl/gui/main/call/CallInfoFrame.java (53)
    A src/net/java/sip/communicator/impl/gui/main/call/ViewCertificateFrame.java (155)
    A src/net/java/sip/communicator/impl/protocol/jabber/OperationSetTLSJabberImpl.java (75)
    M src/net/java/sip/communicator/impl/protocol/jabber/ProtocolProviderServiceJabberImpl.java (80)
    A src/net/java/sip/communicator/service/protocol/OperationSetTLS.java (43)

-- Patch Links --

https://github.com/jitsi/jitsi/pull/20.patch
https://github.com/jitsi/jitsi/pull/20.diff

···

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


#2

@@ -70,6 +72,11 @@
     private boolean hasCallInfo;

     /**
+ * Dummy URL to indicate that the certificate should be displayed.
+ */
+ private final String CERTIFICATE_URL = "http://viewCertificate";

Could you please use jitsi://viewCertificate for this?

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/20/files#r11565723


#3

+ if (e.getEventType() == HyperlinkEvent.EventType.ACTIVATED
+ && e.getURL() != null
+ && CERTIFICATE_URL.equals(e.getURL().toString()))
+ {
+ List<Call> calls = callConference.getCalls();
+ if (!calls.isEmpty())
+ {
+ Call aCall = calls.get(0);
+ Certificate[] chain = aCall.getProtocolProvider()
+ .getOperationSet(OperationSetTLS.class)
+ .getServerCertificates();
+
+ ViewCertificateFrame certFrame =
+ new ViewCertificateFrame(chain, null,
+ resources.getI18NString(
+ "service.gui.callinfo.TLS_CERTIFICATE_CONTENT"));

Line is too long. Maybe let the CallInfoFrame implement HyperlinkListener so that you can remove a level of nesting?

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/20/files#r11565732


#4

@@ -235,6 +267,27 @@ private boolean constructCallInfo()
                     resources.getI18NString("service.gui.callinfo.CALL_TRANSPORT"),
                     preferredTransport.toString()));

+ final OperationSetTLS tlsDetails = aCall.getProtocolProvider()

opSetTls would make this variable more recognizable later on

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/20/files#r11565742


#5

+ final SSLSocket socket = jabberService.getSSLSocket();
+ if (socket != null)
+ {
+ try
+ {
+ result = socket.getSession().getPeerCertificates();
+ }
+ catch (SSLPeerUnverifiedException ignored) // NOPMD
+ {
+ // result will be null
+ }
+ }
+ return result;
+ }
+
+}

Could you please
- add comments to the methods in this file (@see is fine if the behavior is really as documented in the interface)
- add @ Override annotations to those that implements the interface
- move the opening brackets of the methods to their own line

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/20/files#r11565766


#6

@@ -70,6 +72,11 @@
     private boolean hasCallInfo;

     /**
+ * Dummy URL to indicate that the certificate should be displayed.
+ */
+ private final String CERTIFICATE_URL = "http://viewCertificate";

Actually that was my first idea. However, for some reason the HyperlinkEvent.getURL() seems to return null for unrecognised schemes. I didn't find any documentation about that observation in the JavaDoc so maybe I should try one more time...

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/20/files#r11565765


#7

@@ -70,6 +72,11 @@
     private boolean hasCallInfo;

     /**
+ * Dummy URL to indicate that the certificate should be displayed.
+ */
+ private final String CERTIFICATE_URL = "http://viewCertificate";

That is weird. We already use the jitsi:// scheme in the chat window when OTR is jumping in and asking to e.g. authenticate a buddy. But don't spend too much on that if it really doesn't work here. The ChatConversationPanel might do something special to catch the clicks.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/20/files#r11565792


#8

@@ -70,6 +72,11 @@
     private boolean hasCallInfo;

     /**
+ * Dummy URL to indicate that the certificate should be displayed.
+ */
+ private final String CERTIFICATE_URL = "http://viewCertificate";

Tested again and it does not work in this case. Maybe it installs a custom URL handler or something, cause just testing this new URL("jitsi://test") throws java.net.MalformedURLException: unknown protocol: jitsi.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/20/files#r11565812


#9

@@ -70,6 +72,11 @@
     private boolean hasCallInfo;

     /**
+ * Dummy URL to indicate that the certificate should be displayed.
+ */
+ private final String CERTIFICATE_URL = "http://viewCertificate";

ChatConversationPanel uses e.getDescription() instead of e.getURL(), which makes sense according to http://docs.oracle.com/javase/7/docs/api/javax/swing/event/HyperlinkEvent.html#getDescription()

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/20/files#r11565869


#10

+ final SSLSocket socket = jabberService.getSSLSocket();
+ if (socket != null)
+ {
+ try
+ {
+ result = socket.getSession().getPeerCertificates();
+ }
+ catch (SSLPeerUnverifiedException ignored) // NOPMD
+ {
+ // result will be null
+ }
+ }
+ return result;
+ }
+
+}

Fixed.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/20/files#r11565933


#11

@@ -70,6 +72,11 @@
     private boolean hasCallInfo;

     /**
+ * Dummy URL to indicate that the certificate should be displayed.
+ */
+ private final String CERTIFICATE_URL = "http://viewCertificate";

Yes, that worked here as well.
Fixed.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/20/files#r11565939


#12

+ if (e.getEventType() == HyperlinkEvent.EventType.ACTIVATED
+ && e.getURL() != null
+ && CERTIFICATE_URL.equals(e.getURL().toString()))
+ {
+ List<Call> calls = callConference.getCalls();
+ if (!calls.isEmpty())
+ {
+ Call aCall = calls.get(0);
+ Certificate[] chain = aCall.getProtocolProvider()
+ .getOperationSet(OperationSetTLS.class)
+ .getServerCertificates();
+
+ ViewCertificateFrame certFrame =
+ new ViewCertificateFrame(chain, null,
+ resources.getI18NString(
+ "service.gui.callinfo.TLS_CERTIFICATE_CONTENT"));

Fixed.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/20/files#r11565940


#13

@@ -235,6 +267,27 @@ private boolean constructCallInfo()
                     resources.getI18NString("service.gui.callinfo.CALL_TRANSPORT"),
                     preferredTransport.toString()));

+ final OperationSetTLS tlsDetails = aCall.getProtocolProvider()

Fixed.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/20/files#r11565942


#14

+ * Distributable under LGPL license.
+ * See terms of license at gnu.org.
+ */
+package net.java.sip.communicator.impl.gui.main.call;
+
+import java.awt.*;
+import java.security.cert.*;
+import javax.swing.*;
+import net.java.sip.communicator.plugin.desktoputil.*;
+import org.jitsi.service.resources.*;
+
+/**
+ * Frame for showing information about a certificate.
+ */
+public class ViewCertificateFrame extends SIPCommFrame {
+

Missed this bracket earlier, can you please also move it to a new line?

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi/pull/20/files#r11566469


#15

Fixed.

···

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


#16

Merged #20.

···

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