Re: [PATCH v2 7/7] OpenVPN: Moved TLS auth to advanced encryption section

[ummeegge <ummeegge@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 22546 bytes --]

Hi Adolf,
you can find it with better overview on Patchwork -->
https://patchwork.ipfire.org/patch/3712/
you can get there the specific diffs (button right side) and a list of
the series by clicking on "Related --> Show"

Best,

Erik

Am Freitag, dem 22.01.2021 um 23:08 +0100 schrieb Adolf Belka:
> Hi Erik,
> 
> Where can I get hold of your latest version of ovpnmain.cgi
> 
> Regards,
> 
> Adolf.
> 
> On 15/01/2021 05:56, ummeegge wrote:
> > Good morning Adolf,
> > 
> > Am Donnerstag, dem 14.01.2021 um 19:50 +0100 schrieb Adolf Belka:
> > > Hi Erik,
> > > 
> > > I have been keeping an eye on this OpenVPN thread anyway so I am
> > > willing to pick this up and see if I can help.
> > > 
> > > As I am no where near as proficient in OpenVPN as you I may end
> > > up
> > > needing some guidance but am willing to give it a go and see how
> > > things go.
> > Will try to help you out if i can.
> > 
> > > I have a couple of bugs with my name against them that I want to
> > > get
> > > moving first but then I will pick this topic up if you are okay
> > > with
> > > that.
> > As mentioned before, i am more then happy with this.
> > 
> > > Regards,
> > > 
> > > Adolf.
> > Best,
> > 
> > Erik
> > 
> > > On 13/01/2021 10:18, ummeegge wrote:
> > > > Hi Michael,
> > > > there is currently a lot of other real world stuff around me
> > > > and my
> > > > time is really less since things out there do not get easier
> > > > these
> > > > days, at least for me.
> > > > 
> > > > If more time and motivation (which is most important) comes up
> > > > i
> > > > will
> > > > may go for another try to accomplish this topic with your
> > > > suggestions.
> > > > I would also finish this conversation here since it is hardly
> > > > comprehensible and a possible v3 would differs a lot.
> > > > 
> > > > If someone else wanted to jump in this topic with solutions i
> > > > am
> > > > more
> > > > then happy with this.
> > > > 
> > > > All the best,
> > > > 
> > > > Erik
> > > > 
> > > > 
> > > > Am Dienstag, dem 29.12.2020 um 11:44 +0100 schrieb Michael
> > > > Tremer:
> > > > > Hi,
> > > > > 
> > > > > > On 16 Dec 2020, at 17:30, ummeegge <ummeegge(a)ipfire.org>
> > > > > > wrote:
> > > > > > 
> > > > > > Hi Michael,
> > > > > > 
> > > > > > Am Dienstag, dem 15.12.2020 um 12:58 +0100 schrieb Michael
> > > > > > Tremer:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > > On 14 Dec 2020, at 16:12, ummeegge
> > > > > > > > <ummeegge(a)ipfire.org>
> > > > > > > > wrote:
> > > > > > > > 
> > > > > > > > Hi Michael,
> > > > > > > > thanks for looking into this.
> > > > > > > > 
> > > > > > > > Am Montag, dem 14.12.2020 um 14:43 +0100 schrieb
> > > > > > > > Michael
> > > > > > > > Tremer:
> > > > > > > > > Hello Erik,
> > > > > > > > > 
> > > > > > > > > Yes, I am very much interested in this.
> > > > > > > > > 
> > > > > > > > > And I have spent pretty much an hour to make sense
> > > > > > > > > out of
> > > > > > > > > all
> > > > > > > > > of
> > > > > > > > > this, and unfortunately have to report that I failed.
> > > > > > > > that´s not good to hear. OK, let me explain... I tried
> > > > > > > > to
> > > > > > > > come
> > > > > > > > up
> > > > > > > > with
> > > > > > > > a solution where we already have spoken more times
> > > > > > > > about in
> > > > > > > > the
> > > > > > > > past, i
> > > > > > > > tried to follow up your suggestions, the latest about
> > > > > > > > that
> > > > > > > > topic
> > > > > > > > was
> > > > > > > > here -->
> > > > > > > >    
> > > > > > > > https://lists.ipfire.org/pipermail/development/2020-October/008441.html
> > > > > > > > in Oktober to bring up "a select box where multiple
> > > > > > > > algorithms
> > > > > > > > can
> > > > > > > > be
> > > > > > > > chosen (like in IPsec)". Since --cipher is deprectaed
> > > > > > > > and
> > > > > > > > will
> > > > > > > > be
> > > > > > > > replaced by --data-ciphers which is a >=2.5.0 option we
> > > > > > > > need
> > > > > > > > another
> > > > > > > > selection field for the --data-ciphers-fallback which
> > > > > > > > is a
> > > > > > > > replacement
> > > > > > > > for --cipher and the server uses this directive to talk
> > > > > > > > also to
> > > > > > > > client
> > > > > > > > which are <2.5.0 .
> > > > > > > Yes, the ciphers. But as far as I understand this, you
> > > > > > > are
> > > > > > > offering
> > > > > > > to select the whole cipher suite. That is something
> > > > > > > different.
> > > > > > > 
> > > > > > > And you are offering different options for TLSv1.3 and
> > > > > > > lower,
> > > > > > > which
> > > > > > > we do not do in IPsec either. You have one selection for
> > > > > > > both
> > > > > > > IKEv1
> > > > > > > and IKEv2.
> > > > > > Yes, i thougt it might be an idea for the advanced section
> > > > > > be
> > > > > > it is
> > > > > > not
> > > > > > really needed since OpenVPN uses his own choice for the
> > > > > > control-
> > > > > > chanel
> > > > > > if nothing has been configured.
> > > > > That probably isn’t good then. The UI should at least show
> > > > > the
> > > > > user
> > > > > what is being used.
> > > > > 
> > > > > If we want OpenVPN to chose what is best, there should be
> > > > > some
> > > > > option
> > > > > that says “automatic” at least.
> > > > > 
> > > > > > > > OpenVPN presses to use cipher negotiation which we have
> > > > > > > > avoid
> > > > > > > > since
> > > > > > > > the
> > > > > > > > release of 2.4 with --ncp-disable, this option is no
> > > > > > > > also
> > > > > > > > deprecated so
> > > > > > > > we need to take action on this.
> > > > > > > I will skip my moan about introducing something and then
> > > > > > > deprecating
> > > > > > > it shortly after…
> > > > > > It was good that we did not act at that time.
> > > > > In hindsight yes. But we didn’t know back then that things
> > > > > would
> > > > > change again, and we do not know now if things will change
> > > > > another
> > > > > time in the near future.
> > > > > 
> > > > > So, we have to do “best-effort” here.
> > > > > 
> > > > > > > I agree that we need to act.
> > > > > > > 
> > > > > > > > We need to have then two new options, two seletion
> > > > > > > > boxes
> > > > > > > > which
> > > > > > > > are
> > > > > > > > odd
> > > > > > > > to see in the global section. I tried it in the
> > > > > > > > advanced
> > > > > > > > server
> > > > > > > > section
> > > > > > > > too and it looks even more confusing with all the other
> > > > > > > > already
> > > > > > > > existing options, nothing i wanted to do. So i thought
> > > > > > > > your
> > > > > > > > above
> > > > > > > > linked suggestions are pretty straight forward and the
> > > > > > > > best
> > > > > > > > solution to
> > > > > > > > build an own crypto section which do have already
> > > > > > > > defaults,
> > > > > > > > nobody
> > > > > > > > needs to do there anyting, the global section has been
> > > > > > > > cleaned
> > > > > > > > up
> > > > > > > > and
> > > > > > > > it should be even easier for everone to overview.
> > > > > > > We have touched this topic multiple times. And I remember
> > > > > > > the
> > > > > > > last
> > > > > > > conclusion as follows:
> > > > > > > 
> > > > > > > The average user does not have to touch the advanced
> > > > > > > options.
> > > > > > > They
> > > > > > > have to put in their subnet, select a port and protocol
> > > > > > > maybe
> > > > > > > and
> > > > > > > that is about it. They do not need to think about what
> > > > > > > cipher
> > > > > > > to
> > > > > > > use,
> > > > > > > because we would have selected a reasonable default. If
> > > > > > > they
> > > > > > > want
> > > > > > > to
> > > > > > > change something (because of hardware requirements, etc.)
> > > > > > > they
> > > > > > > can do
> > > > > > > that on the advanced page.
> > > > > > This is exactly what i did.
> > > > > Okay. So why is there an extra page then, and not a section
> > > > > on
> > > > > the
> > > > > advanced page?
> > > > > 
> > > > > I like the now cleaner look of the main page which only has
> > > > > the
> > > > > bare
> > > > > necessities.
> > > > > 
> > > > > > > I agree that this is all not idea, but adding a third
> > > > > > > page to
> > > > > > > this
> > > > > > > all makes it rather more confusing than less.
> > > > > > Not sure if i understand you here right. You mentioned
> > > > > > above
> > > > > > the
> > > > > > advanced page but i understand you here that you do not
> > > > > > want a
> > > > > > third
> > > > > > one ?
> > > > > There is the main page, where things can be configured
> > > > > (subnet,
> > > > > port,
> > > > > protocol, …), then advanced options, and now the cryptography
> > > > > options.
> > > > > 
> > > > > They are all the same - in that they directly change the
> > > > > OpenVPN
> > > > > configuration - and we only wanted to split them by two
> > > > > things:
> > > > > 
> > > > > a) Is it likely that the user will change them?
> > > > > 
> > > > > b) Or is this an option that the average user should not
> > > > > touch.
> > > > > 
> > > > > The advanced settings page can be as long as we like. It does
> > > > > not
> > > > > have to be pretty.
> > > > > 
> > > > > > > > It made for me no sense to leave parts of the crypto
> > > > > > > > (TLS-
> > > > > > > > auth
> > > > > > > > and
> > > > > > > > HMACs) in the global section, especially because there
> > > > > > > > are
> > > > > > > > also
> > > > > > > > even
> > > > > > > > more algorithms, but also new options for the TLS
> > > > > > > > authentication
> > > > > > > > which
> > > > > > > > i have all integrated so i moved them also into that
> > > > > > > > place
> > > > > > > > (patch
> > > > > > > > 6/7
> > > > > > > > and 7/7).
> > > > > > > I remember that we have agreed to move the existing
> > > > > > > crypto
> > > > > > > options to
> > > > > > > the advanced page.
> > > > > > Yes, me too and i thought you meant an dedicated crypto
> > > > > > page
> > > > > > like
> > > > > > it is
> > > > > > for IPSec but you meant the advanced server section ? If
> > > > > > yes we
> > > > > > would
> > > > > > mix then routing logging, DNS/WINS options with MTU related
> > > > > > configure
> > > > > > options. Also, the crypto section can have also some
> > > > > > additionals
> > > > > > like
> > > > > > key lieftime, tls-version (if someone wants only TLSv1.3)
> > > > > > and
> > > > > > may
> > > > > > some
> > > > > > other upcoming stuff.
> > > > > That can all go to the advanced page in my opinion.
> > > > > 
> > > > > > > > The control-channel cipher selection (patch 5/7) is new
> > > > > > > > and
> > > > > > > > should
> > > > > > > > be
> > > > > > > > really a part for the advanced ones, therefor no
> > > > > > > > defaults
> > > > > > > > has
> > > > > > > > been
> > > > > > > > set,
> > > > > > > > it simply won´t appear if nothing has been configured.
> > > > > > > What is the benefit of choosing a different cipher for
> > > > > > > the
> > > > > > > control
> > > > > > > channel?
> > > > > > This one is not really needed, as mentioned above OpenVPN
> > > > > > makes
> > > > > > then
> > > > > > this desicion by it´s own.
> > > > > Okay, but why do we have it then? :)
> > > > > 
> > > > > > > > I have the problem that I cannot get on top of these
> > > > > > > > things.
> > > > > > > > You
> > > > > > > > are
> > > > > > > > submitting *massive* patchsets, in this case I even
> > > > > > > > believe
> > > > > > > > that
> > > > > > > > the
> > > > > > > > whole things has been submitted a second time, although
> > > > > > > > I
> > > > > > > > do
> > > > > > > > not
> > > > > > > > know
> > > > > > > > if there are any changes.
> > > > > > > > Yes there are. Old and deprecated ciphers will now be
> > > > > > > > detected
> > > > > > > > and
> > > > > > > > a
> > > > > > > > warning comes up in the global section to change them
> > > > > > > > as
> > > > > > > > soon
> > > > > > > > as
> > > > > > > > possible (patch 3/7), also all languages has been
> > > > > > > > translated
> > > > > > > > and
> > > > > > > > has
> > > > > > > > been included.
> > > > > > > > Also i wanted to split it in more specific topics for
> > > > > > > > better
> > > > > > > > overview
> > > > > > > > but it seems that i have been failed.
> > > > > > > I do not think that you have failed. I just think that it
> > > > > > > is
> > > > > > > not
> > > > > > > clear what the patch intended to do. So I cannot really
> > > > > > > look
> > > > > > > at
> > > > > > > it
> > > > > > > and verify if the code actually does what you want it to
> > > > > > > do.
> > > > > > > 
> > > > > > > Could it have been split into even smaller changes? Yes,
> > > > > > > would
> > > > > > > that
> > > > > > > have helped? Maybe. But I do not think that we should
> > > > > > > waste
> > > > > > > too
> > > > > > > much
> > > > > > > time to reformat the patches. I want the changes to be
> > > > > > > ready
> > > > > > > to
> > > > > > > be
> > > > > > > merged as soon as possible.
> > > > > > OK. Let´s go through this then. I have attached also
> > > > > > screenshots
> > > > > > for
> > > > > > all changes for a better first impression.
> > > > > That was very helpful to me.
> > > > > 
> > > > > > > > I could not even find out *what* you are actually
> > > > > > > > trying to
> > > > > > > > do.
> > > > > > > > There
> > > > > > > > are more and more buttons being added and I have not
> > > > > > > > the
> > > > > > > > slightest
> > > > > > > > idea
> > > > > > > > what I am supposed to do with them. I have given my
> > > > > > > > thoughts
> > > > > > > > about
> > > > > > > > the
> > > > > > > > cipher selection and I felt that I have not been heard.
> > > > > > > > One intention was that you do not need anything to do
> > > > > > > > except
> > > > > > > > you
> > > > > > > > want
> > > > > > > > to do some advanced crypto stuff. The rest should have
> > > > > > > > already
> > > > > > > > been
> > > > > > > > made...
> > > > > > > > 
> > > > > > > > 
> > > > > > > > I fear that this is all way too complicated. I cannot
> > > > > > > > review
> > > > > > > > what I
> > > > > > > > do
> > > > > > > > not even understand what the desired outcome is. And
> > > > > > > > failing to
> > > > > > > > understand that either means that it is either way too
> > > > > > > > complicated
> > > > > > > > or I
> > > > > > > > have not read enough anywhere about all of this.
> > > > > > > > Please check out the above linked topic where you have
> > > > > > > > wrote
> > > > > > > > suggestions that i simply tried to achieve but again,
> > > > > > > > it
> > > > > > > > seems
> > > > > > > > that
> > > > > > > > i
> > > > > > > > was not successfull with this...
> > > > > > > So, maybe help me to understand why the user would want
> > > > > > > to
> > > > > > > use
> > > > > > > different ciphers for the control channel and for
> > > > > > > TLSv1.3/2.
> > > > > > This is not needed  and i can easily let it out but
> > > > > > mentioned
> > > > > > beneath
> > > > > > OpenVPN decided to give TLSv1.2 another directive then
> > > > > > TLSv1.3
> > > > > > therefor
> > > > > > the two boxes.
> > > > > Yes, I think choosing this once is enough.
> > > > > 
> > > > > > > I consider my ciphers as follows:
> > > > > > > 
> > > > > > > * Which algorithms do I trust? If I do not trust AES for
> > > > > > > example,
> > > > > > > I
> > > > > > > would unselect it for everything, regardless if that is
> > > > > > > the
> > > > > > > control
> > > > > > > or data channel).
> > > > > > > 
> > > > > > > * What does my hardware support? I would want to choose
> > > > > > > AES
> > > > > > > if my
> > > > > > > hardware has acceleration for it.
> > > > > > > 
> > > > > > > * Then I would sort them by most secure first (e.g. AES-
> > > > > > > 256,
> > > > > > > then
> > > > > > > AES-192 and so on…)
> > > > > > > 
> > > > > > > I can replicate that with only one multiple-select box
> > > > > > > that
> > > > > > > simply
> > > > > > > lists:
> > > > > > > 
> > > > > > > * AES-256-GCM
> > > > > > > * AES-256-CBC
> > > > > > > * AES-128-GCM
> > > > > > > * AES-128-CBC
> > > > > > > * ChaCha20-Poly1305
> > > > > > > * Blowfish
> > > > > > > * ...
> > > > > > This is a problen there is exactly the same, this are two
> > > > > > directives. -
> > > > > > -data-cipher does NCP whereby --data-ciphers-fallback
> > > > > > substitutes -
> > > > > > -
> > > > > > cipher. If we leave --data-ciphers-fallback out and work
> > > > > > only
> > > > > > with
> > > > > > --
> > > > > > data-ciphers' clients which do not have this directive can
> > > > > > not
> > > > > > connect.
> > > > > No no, we want to be compatible with older clients and there
> > > > > should
> > > > > be a dropdown that works just like the old cipher selection.
> > > > > 
> > > > > It should be possible to turn it off though by adding a
> > > > > “disabled”
> > > > > option.
> > > > > 
> > > > > > Also, client which are <=2.5.0 do not understands both
> > > > > > directives
> > > > > > and
> > > > > > simply do not starts. To find a way out in this foggy world
> > > > > > i
> > > > > > delivered
> > > > > > a checkbox while client creation so the user have the
> > > > > > possiblity to
> > > > > > bring the new directive into client.ovpn but also old
> > > > > > clients
> > > > > > can
> > > > > > be
> > > > > > changed/updated via the edit menu.
> > > > > The clients should not have any cipher configuration now
> > > > > since
> > > > > they
> > > > > should automatically negotiate it - which is enabled by
> > > > > default.
> > > > > 
> > > > > > > And a second one that has
> > > > > > > 
> > > > > > > * SHA512
> > > > > > > * SHA384
> > > > > > > * SHA256
> > > > > > > * …
> > > > > > The HMACs can not be used for negotiation, this is only a
> > > > > > single
> > > > > > selection.
> > > > > Well, that is bad. That means we still do not have full
> > > > > negotiation?
> > > > > 
> > > > > > > The code will then have to convert this into the fancy
> > > > > > > names
> > > > > > > for
> > > > > > > the
> > > > > > > OpenVPN configuration file. But I suppose that should be
> > > > > > > easy
> > > > > > > to
> > > > > > > do.
> > > > > > > 
> > > > > > > That way, the user can be certain that they have chosen
> > > > > > > the
> > > > > > > right
> > > > > > > thing for everything in one go.
> > > > > > > 
> > > > > > > > So, could you please summarise for me what you want to
> > > > > > > > achieve
> > > > > > > > here?
> > > > > > > > Hope i have it done above.
> > > > > > > Thank you for that.
> > > > > > > 
> > > > > > > I really appreciate all this time that you have invested
> > > > > > > in
> > > > > > > this,
> > > > > > > and
> > > > > > > the code looks clean. I just think we have been on
> > > > > > > different
> > > > > > > pages
> > > > > > > about what we want it to look and feel like for the user.
> > > > > > Thank you too. This one really needs a lot of time and
> > > > > > testing
> > > > > > around
> > > > > > and also with help from Adolf it comes to this where it
> > > > > > currently
> > > > > > is.
> > > > > Great teamwork!
> > > > > 
> > > > > > > > Is this supposed to make OpenVPN more compatible,
> > > > > > > > secure,
> > > > > > > > or
> > > > > > > > anything
> > > > > > > > else?
> > > > > > > > Yes both but also easier since the user do not need to
> > > > > > > > do
> > > > > > > > anything
> > > > > > > > but
> > > > > > > > he should become more security under the hood, the
> > > > > > > > backward
> > > > > > > > and
> > > > > > > > the
> > > > > > > > forward compatibility should be in a better standing
> > > > > > > > than
> > > > > > > > before
> > > > > > > > and if
> > > > > > > > someone wants to go into the depth, this is even also
> > > > > > > > possible.
> > > > > > > I think that you have implemented the “pro” version here.
> > > > > > > It
> > > > > > > has
> > > > > > > all
> > > > > > > the buttons you need and uses all features of OpenVPN.
> > > > > > > But it
> > > > > > > is
> > > > > > > complicated. So maybe lets hide it from the user that
> > > > > > > there
> > > > > > > is a
> > > > > > > control and data channel. Does the user need to know
> > > > > > > about
> > > > > > > this?
> > > > > > > I do
> > > > > > > not think so. It will work either way.
> > > > > > No there is no must for the control-channel cipher
> > > > > > selection...
> > > > > > If
> > > > > > one
> > > > > > is in, it is sometimes nice to walk through all that ;-).
> > > > > > 
> > > > > > > > Why do we not talk about this before things are being
> > > > > > > > implemented,
> > > > > > > > or
> > > > > > > > did I just miss the discussion?
> > > > > > > > Yes i think you missed some of that, hopefully i could
> > > > > > > > remind
> > > > > > > > you
> > > > > > > > on
> > > > > > > > some points.
> > > > > > > This is good progress :)
> > > > > > Happy to here this.
> > > > > Okay, so what are the next steps now?
> > > > > 
> > > > > -Michael
> > > > > 
> > > > > > 
> > > > > > Best,
> > > > > > 
> > > > > > Erik
> > > > > > 
> > > > > > > -Michael
> > > > > > > 
> > > > > > > > -Michael
> > > > > > > > 
> > > > > > > > Best,
> > > > > > > > 
> > > > > > > > Erik
> > > > > > > > 
> > > > > > > > 
> > > > > > > > > On 14 Dec 2020, at 14:03, ummeegge
> > > > > > > > > <ummeegge(a)ipfire.org>
> > > > > > > > > wrote:
> > > > > > > > > 
> > > > > > > > > Hi all,
> > > > > > > > > just wanted to ask if there is still interest in this
> > > > > > > > > patch
> > > > > > > > > series ?
> > > > > > > > > 
> > > > > > > > > Best,
> > > > > > > > > 
> > > > > > > > > Erik
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > <advanced_encryption_with_defaults.png><client_version.png>
> > > > > > <glo
> > > > > > bal_
> > > > > > settings.png><deprectaed_ciphers_warning.png>
> > > > 
> >