service: fix missing ppp user option

[domosekai]

Due to a typo, the name option is never saved to ppp-options.

It breaks EAP-MSCHAPv2 authentication because the default name (which is the machine hostname) is sent if both name and user options are absent. When using other authentication methods the value is not used so that's probably why nobody has ever complained.

Issue originally reported to SoftEther VPN at: https://github.com/SoftEtherVPN/SoftEtherVPN/issues/1725

[dkosovic]

Thanks for the pull request.

Looks like I introduced that bug in src/nm-l2tp-service.c with commit# https://github.com/nm-l2tp/NetworkManager-l2tp/commit/66a5355c971a77aaa7b06540025521084116609a back in 2016. I was trying to synchronize the code with NetworkManager-pptp 1.2.1 but somehow screwed up in my copy/pasting.

Looking at the same section of code in the current NetworkManager-pptp :

• https://github.com/NetworkManager/NetworkManager-pptp/blob/1.2.10/src/nm-pptp-service.c#L496

I'm wondering if the src/nm-l2tp-service.c code in question should be using user instead of name like so ?

        /* Username; try L2TP specific username first, then generic username */
        value = nm_setting_vpn_get_data_item(s_vpn, NM_L2TP_KEY_USER);
        if (!value || !*value)
            value = nm_setting_vpn_get_user_name(s_vpn);
        if (value && *value) {
            write_config_option(fd, "user %s\n", value);
        }

I can't remember why I changed it from user to name back in 2016.

[domosekai]

Looks like you didn't. It was name before the change, all the way back to 12 years ago. https://github.com/nm-l2tp/NetworkManager-l2tp/blob/65c0ae8d5463a437d9414c0038652fdb947ce584/src/nm-l2tp-service.c#L1303-L1307

I wondered that as well, because user seems to be the right option and name is for the local authenticator's name. Since we are not authenticating the peer, it's never used and user defaults to name if unset.

[dkosovic]

NetworkManager-sstp is also using user :

• https://gitlab.gnome.org/GNOME/network-manager-sstp/-/blob/master/src/nm-sstp-service.c#L675

While it is using name for TLS certificate identity or subject name.

As the main author of NetworkManager-sstp is also a maintainer of pppd, it adds to the confidence that user is the right option.

I'm happy to go for user.

[domosekai]

OK. Changed to user.

[dkosovic]

I only released a new version of NetworkManager-l2tp in the last couple of weeks, I'm not sure how soon I'll be making the next release, depends on if I find any other bugs. But I might make new Ubuntu PPA packages that incorporate this pull request as a patch and put them up on: https://launchpad.net/~nm-l2tp/+archive/ubuntu/network-manager-l2tp