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/ 
> 
> 

Attachment: smime.p7s
Description: S/MIME cryptographic signature

Reply via email to