Repository: netty/netty
Issue: FingerprintTrustManagerFactory javadoc note cryptic
Labels: cleanup
documentation

the javadoc says:

"Never use this {@link TrustManagerFactory} in production unless you are sure exactly what you are doing with it."

I was actually sure that in order to prevent MITM attacks, I want to pin the serverside certificates to a list of well known fingerprints. I do not want to accept other certificates.

So I think I know what I am doing, but this note made me feel uneasy. Why is it formulated so strongly? Why is it not explaining what "this TrustmanagerFactory" actually does?
I am currently trying to figure out if there is an unintentional side effect, but can't find it :-)


Issue comments:
It only checks the fingerprint. It does not traverse the certificate chain at all. Theoretically, there's a chance where an attacker can forge a certificate with identical fingerprint, although it's not going to be very easy.

I agree that it needs better documentation though. Would you be interested in sending us a PR?

Ah ok, it makes sense to point that out. Will make a PR to improve docs.

Thanks a lot, @CodingFabian !

@CodingFabian any progress here ?

woops, forgot. coming

@CodingFabian 😍 

Fixed by https://github.com/netty/netty/pull/5195


Pull request review comments:
@CodingFabian maybe add a `<a href=""...>` for `Man-in-the-middle` 

be consistent ... either use Man or man .

`This {@link TrustManagerFactory} will` -> `This {@link TrustManagerFactory} will <strong>only</strong>`


Patch that resolved the issue:
diff --git a/handler/src/main/java/io/netty/handler/ssl/util/FingerprintTrustManagerFactory.java b/handler/src/main/java/io/netty/handler/ssl/util/FingerprintTrustManagerFactory.java
index c4563a99dde..79e434655a9 100644
--- a/handler/src/main/java/io/netty/handler/ssl/util/FingerprintTrustManagerFactory.java
+++ b/handler/src/main/java/io/netty/handler/ssl/util/FingerprintTrustManagerFactory.java
@@ -39,11 +39,19 @@
 /**
  * An {@link TrustManagerFactory} that trusts an X.509 certificate whose SHA1 checksum matches.
  * <p>
- * <strong>NOTE:</strong>
- * Never use this {@link TrustManagerFactory} in production unless you are sure exactly what you are doing with it.
- * </p><p>
+ * <strong>NOTE:</strong> It is recommended to verify certificates and their chain to prevent
+ * <a href="https://en.wikipedia.org/wiki/Man-in-the-middle_attack">Man-in-the-middle attacks</a>.
+ * This {@link TrustManagerFactory} will <strong>only</strong> verify that the fingerprint of certificates match one
+ * of the given fingerprints. This procedure is called
+ * <a href="https://en.wikipedia.org/wiki/Transport_Layer_Security#Certificate_pinning">certificate pinning</a> and
+ * is an effective protection. For maximum security one should verify that the whole certificate chain is as expected.
+ * It is worth mentioning that certain firewalls, proxies or other appliances found in corporate environments,
+ * actually perform Man-in-the-middle attacks and thus present a different certificate fingerprint.
+ * </p>
+ * <p>
  * The SHA1 checksum of an X.509 certificate is calculated from its DER encoded format.  You can get the fingerprint of
  * an X.509 certificate using the {@code openssl} command.  For example:
+ *
  * <pre>
  * $ openssl x509 -fingerprint -sha1 -in my_certificate.crt
  * SHA1 Fingerprint=4E:85:10:55:BC:7B:12:08:D1:EA:0A:12:C9:72:EE:F3:AA:B2:C7:CB


Test patch: