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

[Adolf Belka <adolf.belka@ipfire.org>]

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

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.

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.

Regards,

Adolf.

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><global_
>>> settings.png><deprectaed_ciphers_warning.png>
>>
> 
>