plasma-nm: vpn: openconnect: pkcs11: add support for separate pin value with updated coding style
Needs ReviewPublic

Authored by thorstenb on Oct 4 2017, 5:53 AM.

Details

Reviewers
jgrulich
sbmultimedia
Group Reviewers
Plasma
Summary

openconnect: patch which add support for seperate pin value for pkcs11 url

plasma-nm openconnect should support separate pin value in config file for pkcs11 url. (Pin should not be visible in log)

https://bugs.kde.org/show_bug.cgi?id=384652

Diff Detail

Repository
R116 Plasma Network Management Applet
Branch
arcpatch-D7795_1
Lint
No Linters Available
Unit
No Unit Test Coverage
thorstenb created this revision.Oct 4 2017, 5:53 AM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 4 2017, 5:54 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
thorstenb retitled this revision from fix Coding style to plasma-nm: vpn: openconnect: pkcs11: add support for separate pin value with updated coding style.Oct 4 2017, 5:57 AM
thorstenb edited the summary of this revision. (Show Details)
thorstenb edited the summary of this revision. (Show Details)Oct 4 2017, 5:59 AM
thorstenb added reviewers: Plasma, jgrulich.

I discussed this witn Openconnect maintainer and I was told basically the same I told you first. NM_OPENCONNECT_KEY_PIN is something what NetworkManager-openconnect plugin doesn't support. Do you have your own patched nm-openconnect plugin or something like that?

The openconnect-plugin is not used for parsing this variable. Maybe it was in the past.
I add my pin in "nm-servie-defines.h" near NM_OPENCONNECT_KEY_USERCERT. It was not used.
I try to add this code in openconnect plugin in first place, but it was not working because the code in openconnect-plugin is not used anymore. Imho only the virtual "need_secrets" method is used from plugin. "connect" and "disconnect" are not used.
So i searched for a different place where all the vpn variables were used. And a fellow told me to have a look on plasma-nm.
I moved my stuff from openconnect plugin to plasma-nm. Now its working.
What else could i say? I am shure openconnect-plugin is not in use, because i renamed userkey in userkey2 in openconnect-plugin. But the connection ist still working.

thorstenb updated this revision to Diff 20364.Oct 5 2017, 6:55 AM
thorstenb edited the summary of this revision. (Show Details)

fix for pin-value in middle of line

The openconnect-plugin is not used for parsing this variable. Maybe it was in the past.
I add my pin in "nm-servie-defines.h" near NM_OPENCONNECT_KEY_USERCERT. It was not used.
I try to add this code in openconnect plugin in first place, but it was not working because the code in openconnect-plugin is not used anymore. Imho only the virtual "need_secrets" method is used from plugin. "connect" and "disconnect" are not used.
So i searched for a different place where all the vpn variables were used. And a fellow told me to have a look on plasma-nm.
I moved my stuff from openconnect plugin to plasma-nm. Now its working.
What else could i say? I am shure openconnect-plugin is not in use, because i renamed userkey in userkey2 in openconnect-plugin. But the connection ist still working.

I'm not sure I understand. Let me describe how this is working:

  1. You create openconnect connection in our editor (kcm), where only certain properties are allowed to be configured. These properties are defined by NetworkManager openconnect plugin, there is difference between plasma-nm openconnect plugin and NetworkManager plugin.
  2. Once this connection is created and you try to activate it, NM plugin uses properties/values you configured through our editor and attempts to connect to a server you specified in "gateway" field in our connection editor
  3. Then OpenconnectAuth (from plasma-nm) comes in and tries to establish a connection with the server. In OpenconnectAuthWidget::readConfig() function we read configuration which is stored in NM plugin, which is the same configuration you specified in our editor. Given there is no "pin" property specified (in both plasma-nm plugin and NM plugin), thus the dataMap we attempt to read shouldn't contain any properties which are not defined in NM openconnect plugin.

In theory this will work if you manually modify your openconnect connection in /etc/NetworkManager/system-connections, but it's not the way openconnect devs want to handle passing this "pin" property. I will have to probably discuss this deeper with Openconnect devs what would be the correct way to support this.

thorstenb added a comment.EditedOct 5 2017, 7:34 AM

What you are missing is the Gui part to store the pin in the network-manager config file.
This is correct. I've not implementet a GUI yet. This is just the part for reading the config file values.
All values in group [vpn] were read to the dataMap. No filtering from the plugin is done.
Maybe a filter is missing or not working?
For my case i deliver a fully preconfigured configuration file which is read-only for the gui user.
If a Gui user wants to use pkcs11 with pin, it is not neccessary to hide pin.
The pin could be provided within the uri or the gui will ask interactively for the pin.
This is already working.
Current supported workflows:

  1. If a Gui user enter a pkcs11 uri without pin, but the pin is necessary. The Gui ask for the pin.
  2. If a Gui user enter a pkcs11 uri with pin, this one is used.

Not supported enterprise case: pin is not visible for gui user
This is what i want to add with this patch.

Restricted Application added 1 blocking reviewer(s): jgrulich. · View Herald TranscriptNov 1 2020, 4:08 AM
anthr added a subscriber: anthr.Nov 1 2021, 9:43 PM

Has this ever been implemented? I need to use a yubikey which requires this.

sbmultimedia accepted this revision.Dec 30 2021, 3:17 PM