Hi, I filled a JIRA [1] to address the issues related to cypher suites.
Yes, the original PR changes are related to STARTLS code, but I think, that we are fine (at least for now). Gruss Richard [1] https://issues.apache.org/jira/browse/GERONIMO-6793 Am Donnerstag, den 03.12.2020, 20:55 +0000 schrieb Bernd Eckenfels: > Hello, > > Yes I agree, the ciphers would be its own (security) issue. > > and it might be not so simple to fix as Mail servers are notoriously > outdated and TLS-sloppy. Luckily JDK has already some provisions for > demoting old/deprecated ciphers and also disable most of the not > useable insecure ones in the supported list (but still this weakening > of the JDK defaults should be opt-in). > > BTW i haven’t checked all of the code, if you are requesting TLS > context, there might be another point to look out for protocol names > (very unfortunate API design I must say). I imagine this might be > needed for STARTLS code? > > Gruss > Bernd > > > -- > http://bernd.eckenfels.net > > > Von: Zowalla, Richard <richard.zowa...@hs-heilbronn.de> > Gesendet: Donnerstag, Dezember 3, 2020 1:48 PM > An: dev@geronimo.apache.org > Betreff: Re: Re: Geronimo Java Mail 1.6 in TomEE 8.0.5 -> TLS 1.2 / > 1.3 Support? > > > Hi Bernd, > > @1: I think the original intention of the code (before the PR) was to > disallow the use of sslv2 or sslv3 for the tls handling code thus the > hard-coding to tlsv1 as ssl is handled in another part of the class. > But I agree, that we could remove the "else" and consequently all > kind > of hard-coded TLS config. In this way, we would trust in the jdk > defaults. > > @2: I think, that the aspect related to cyphers would be a separate > issue / PR. I agree, that enabling all available cyphers (L602/L603) > is > not a good idea in general, but this code hasn't changed in the > proposed PR :) > > > wdyt? > > Best > Richard > > Am Donnerstag, den 03.12.2020, 12:11 +0000 schrieb Bernd Eckenfels: > > Hello, > > > > Allowing protocols to be configured is good, but I am not sure why > > you fall back to a hand selected list of no configuration is given, > > why not simply use the JDK defaults, they are frequently adjusted > > (just recently the older TLS versions get removed). > > > > Along this line, why enable all supported ciphers? There is a good > > reason why the JDK disables many. I would stick to the default > > ciphers (and protocols). > > > > Gruss > > Bernd > > -- > > http://bernd.eckenfels.net > > > > Von: Zowalla, Richard <richard.zowa...@hs-heilbronn.de> > > Gesendet: Mittwoch, Dezember 2, 2020 4:57 PM > > An: dev@geronimo.apache.org > > Betreff: Re: Re: Geronimo Java Mail 1.6 in TomEE 8.0.5 -> TLS 1.2 / > > 1.3 Support? > > > > Should be fixed now. > > > > Am Mittwoch, den 02.12.2020, 15:32 +0000 schrieb Zowalla, Richard: > > > It is indeed > > > > > > mail.<protocol>.ssl.socketFactory.class > > > > > > (see line 88, MailConnection#MAIL_SSL_FACTORY_CLASS -> uses > > > reflection to create an instance of the specified factory. > > > > > > or > > > > > > mail.<protocol>.ssl.socketFactory > > > > > > (which requires adding a pre-configured and instantiated factory > > > instance into the properties of the mail session) > > > > > > To be complete, I will add this way to the README as well. > > > > > > Am Mittwoch, den 02.12.2020, 16:24 +0100 schrieb Romain Manni- > > Bucau: > > > > Isnt the property mail.<protocol>.ssl.socketFactory ? > > > > > > > > Romain Manni-Bucau > > > > @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book > > > > > > > > > > > > Le mer. 2 déc. 2020 à 16:09, Zowalla, Richard < > > > > richard.zowa...@hs-heilbronn.de> a écrit : > > > > > Okay. Thanks for the feedback - today, I learned a lot about > > the > > > > > insides of Javamail :) > > > > > > > > > > I have updated my PR: > > > > > > > > > > - Updated README.txt to contain some documentation about > > setting > > > > > a > > > > > custom ssl socket factory > > > > > - Dropped TLSv1 in the fallback protocols (if no custom set > > > > > properties > > > > > are present) > > > > > > > > > > Thanks, > > > > > Richard > > > > > > > > > > > > > > > Am Mittwoch, den 02.12.2020, 15:29 +0100 schrieb Romain > Manni- > > > > > Bucau: > > > > > > Guess you can just create a readme in the geronimo-javamail > > > > > root > > > > > > project, will be sufficient as a first step. > > > > > > Abou he default I wonder if dropping tlsv1 cant be good > since > > > > > it will > > > > > > be dropped soon? > > > > > > Otherwise just adding the missing "o" in protocols i'm fine > > > > > with your > > > > > > proposal. > > > > > > > > > > > > We need to refine if we do a javamail subsite or a generic > > spec > > > > > > subsite sill :s. > > > > > > > > > > > > Romain Manni-Bucau > > > > > > @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book > > > > > > > > > > > > > > > > > > Le mer. 2 déc. 2020 à 15:26, Zowalla, Richard < > > > > > > richard.zowa...@hs-heilbronn.de> a écrit : > > > > > > > I updated the diff (cf. v2) to (hopefully) address the > > > > > concerns > > > > > > > raised > > > > > > > (if I understood them correctly). > > > > > > > > > > > > > > If you point me to a location where I can add a README / > > > > > > > documentation, > > > > > > > I would be happy to fill another JIRA with a related PR > to > > > > > document > > > > > > > the > > > > > > > usage of the custom ssl socket factory. > > > > > > > > > > > > > > Am Mittwoch, den 02.12.2020, 13:58 +0000 schrieb Zowalla, > > > > > Richard: > > > > > > > > Thanks for your thoughs - I think, I get the idea. > > > > > > > > > > > > > > > > Maybe: > > > > > > > > > > > > > > > > - Using "mail.smtp.ssl.protocls" to allow easier > > > > > configuration > > > > > > > (as > > > > > > > > proposed in the PR) for > > > > > MailConnection#getConnectedTLSSocket() - > > > > > > > > would > > > > > > > > address 1. > > > > > > > > > > > > > > > > - To address 3. and pre-claim: PR would enable all > > > > > protocols; > > > > > > > maybe > > > > > > > > address this concern by adding a default fallback > > pointing > > > > > to > > > > > > > TLSv1, > > > > > > > > TLSv1.1, TLSv1.2 and TLS v1.3 (if supported) if no > custom > > > > > > > > configuration > > > > > > > > via "mail.smtp.ssl.protocls" is present? > > > > > > > > > > > > > > > > - Documentation is always appreciated ;) > > > > > > > > > > > > > > > > Wdyt? > > > > > > > > > > > > > > > > Am Mittwoch, den 02.12.2020, 14:41 +0100 schrieb Romain > > > > > Manni- > > > > > > > Bucau: > > > > > > > > > Yes but issue that we don't want to enable them all > > too. > > > > > > > > > So to be concrete what about: > > > > > > > > > > > > > > > > > > 1. Enable a smoother configuration (to avoid a custom > > > > > class) > > > > > > > > > 2. Document the custom class case better (at least in > a > > > > > readme) > > > > > > > > > 3. Change a bit default to inherit JVM ones > > > > > > > > > > > > > > > > > > Think we should make the 3 to consider this case > > treated > > > > > (does > > > > > > > not > > > > > > > > > mean it must be in the same PR but more before next > > > > > release). > > > > > > > > > Wdyt? > > > > > > > > > > > > > > > > > > Romain Manni-Bucau > > > > > > > > > @rmannibucau | Blog | Old Blog | Github | LinkedIn | > > > > > Book > > > > > > > > > > > > > > > > > > > > > > > > > > > Le mer. 2 déc. 2020 à 13:20, Zowalla, Richard < > > > > > > > > > richard.zowa...@hs-heilbronn.de> a écrit : > > > > > > > > > > Ah sorry - I misunderstood your comment. > > > > > > > > > > > > > > > > > > > > A custom socket factory would indeed fix the > problem, > > > > > but it > > > > > > > is > > > > > > > > > > rather undocumented. > > > > > > > > > > > > > > > > > > > > Nevertheless I think, that the default fallback > > > > > shouldn't be > > > > > > > > > > hardcoded or at least support some more > protocols... > > > > > > > > > > > > > > > > > > > > Best and thanks for the idea, > > > > > > > > > > Richard > > > > > > > > > > > > > > > > > > > > Am Mittwoch, den 02.12.2020, 12:16 +0000 schrieb > > > > > Zowalla, > > > > > > > > > > Richard: > > > > > > > > > > > Honestly I didn't. I discovered the hard-coded > > > > > > > > > > > String[]("TLSv1") > > > > > > > > > > > in > > > > > > > > > > > MailConnection#getConnectedTLSSocket(), which is > > > > > (imho) a > > > > > > > bit > > > > > > > > > > > odd. > > > > > > > > > > > > > > > > > > > > > > Imho, users should either be allowed to specify > the > > > > > enabled > > > > > > > > > > > (and > > > > > > > > > > > supported) protocols or to use the default ones > > > > > provided by > > > > > > > the > > > > > > > > > > > jdk > > > > > > > > > > > classes :) > > > > > > > > > > > > > > > > > > > > > > This is already done for > > > > > > > MailConnection#getConnectedSSLSocket > > > > > > > > > > > but > > > > > > > > > > > not > > > > > > > > > > > for the TLS handling. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Am Mittwoch, den 02.12.2020, 13:09 +0100 schrieb > > > > > Romain > > > > > > > Manni- > > > > > > > > > > > Bucau: > > > > > > > > > > > > Hi Richard, > > > > > > > > > > > > > > > > > > > > > > > > Did you try a custom socket factory? In such a > > case > > > > > you > > > > > > > fully > > > > > > > > > > > > control > > > > > > > > > > > > it. > > > > > > > > > > > > > > > > > > > > > > > > Romain Manni-Bucau > > > > > > > > > > > > @rmannibucau | Blog | Old Blog | Github | > > LinkedIn > > > > > | > > > > > > > Book > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Le mer. 2 déc. 2020 à 13:01, Zowalla, Richard < > > > > > > > > > > > > richard.zowa...@hs-heilbronn.de > > > > > > > > > > > > > a écrit : > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > > > > > > > I did some debugging and found, that TLSv1 is > > > > > hard- > > > > > > > coded in > > > > > > > > > > > > > MailConnection.java in v1.0.0 of Geronimo > Java > > > > > Mail. > > > > > > > > > > > > > > > > > > > > > > > > > > I filled a JIRA [1], which contains a patch > > > > > proposal. > > > > > > > > > > > > > > > > > > > > > > > > > > Happy to receive some feedback. > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks in advance, > > > > > > > > > > > > > Richard > > > > > > > > > > > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > https://issues.apache.org/jira/browse/GERONIMO-6792 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > > > Richard Zowalla, M.Sc. > > > > > > > > > > Research Associate, PhD Student | Medical > Informatics > > > > > > > > > > > > > > > > > > > > Hochschule Heilbronn – University of Applied > Sciences > > > > > > > > > > Max-Planck-Str. 39 > > > > > > > > > > D-74081 Heilbronn > > > > > > > > > > phone: +49 7131 504 6791 > > > > > > > > > > mail: richard.zowa...@hs-heilbronn.de > > > > > > > > > > web: https://www.mi.hs-heilbronn.de/ > > > > > > -- > > > Richard Zowalla, M.Sc. > > > Research Associate, PhD Student | Medical Informatics > > > > > > Hochschule Heilbronn – University of Applied Sciences > > > Max-Planck-Str. 39 > > > D-74081 Heilbronn > > > phone: +49 7131 504 6791 > > > mail: richard.zowa...@hs-heilbronn.de > > > web: https://www.mi.hs-heilbronn.de/ > >
smime.p7s
Description: S/MIME cryptographic signature