Bug 12641 - Online status wrongly calculated for OpenVPN certs with underscore in their common name.
Summary: Online status wrongly calculated for OpenVPN certs with underscore in their c...
Status: CLOSED NOTABUG
Alias: None
Product: IPFire
Classification: Unclassified
Component: openvpn (show other bugs)
Version: 3
Hardware: all Unspecified
: Will affect most users Major Usability
Assignee: Assigned to nobody - feel free to grab it and work on it
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-06-16 14:58 UTC by Thorsten Schöning
Modified: 2021-06-17 16:42 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thorsten Schöning 2021-06-16 14:58:33 UTC
This is related to Bug 12638 and the attempt to implement some naming scheme for connections and certificates to easier distinguish different groups of people. When using a name like "ams_tschoening" as the common name of the newly created cert for some user, the online status is not properly calculated.

That status depends on the entries in the status log of OpenVPN, in which the common names of the certificates of connected clients are logged. For the example above that would be "ams_tschoening". The current implementation simply iterates all of those lines and tries to find a corresponding common name in the values stored in the file "/var/ipfire/ovpn/ovpnconfig". That should work in theory because the common name used to create a certificate is written into that file, like in the following example:

> 1,on,ams_tschoening,ams_tschoening,host,cert,,,,,,,,,,,,,,,,,,,,,AMS Thorsten Schoening,,,,,,,dynamic,,,,,,,,,no-pas
s,,

The problem is that code iterating and comparing the lines of the status log with the entries of the config file first changes "_" to " " in the common name of the status log. Of course this can never match the correct entry of the config file, hence the online status for some user is wrong. Using a space in the common name instead of an underscore works instead:

> 1,on,ams_tschoening,ams tschoening,host,cert,,,,,,,,,,,,,,,,,,,,,AMS Thorsten Schoening,,,,,,,dynamic,,,,,,,,,no-pas
s,,

The following is the problematic code, especially line 5420:

> 5411                 my $cn;
> 5412                 my @match = ();
> 5413 foreach my $line (@status) {
> 5414         chomp($line);
> 5415         if ( $line =~ /^(.+),(\d+\.\d+\.\d+\.\d+\:\d+),(\d+),(\d+),(.+)/) {
> 5416                 @match = split(m/^(.+),(\d+\.\d+\.\d+\.\d+\:\d+),(\d+),(\d+),(.+)/, $line);
> 5417                 if ($match[1] ne "Common Name") {
> 5418         $cn = $match[1];
> 5419                 }
> 5420                 $cn =~ s/[_]/ /g;
> 5421                 if ($cn eq "$confighash{$key}[2]") {
> 5422         $col1="bgcolor='${Header::colourgreen}'";
> 5423         $active = "<b><font color='#FFFFFF'>$Lang::tr{'capsopen'}</font></b>";
> 5424                 }
> 5425         }
> 5426 }

https://git.ipfire.org/?p=ipfire-2.x.git;a=blob;f=html/cgi-bin/ovpnmain.cgi;h=b98d88529ae4a3813a2fc4eadcd56177a0cd8bd5;hb=HEAD#l5420

I couldn't find any reason for that behaviour, but according the history things have been this way since at least 2007 already. Though, that behaviour doesn't make too much sense to me, especially because if at all, some people try to avoid spaces and use underscores instead. :-)
Comment 1 Michael Tremer 2021-06-17 15:36:02 UTC
This is a not a bug because the web UI does not allow you creating connections with certain characters.
Comment 2 Thorsten Schöning 2021-06-17 16:14:46 UTC
(In reply to Michael Tremer from comment #1)
> This is a not a bug because the web UI does not allow you creating
> connections with certain characters.

It's not about connection names, but the common name which DOES allow underscores to be used. So you have "ovpnconfig" containing the COMMON NAME "ams_tschoening" and the status log of OVPN containing the COMMON NAME "ams_tschoening" as well. then the codes goes and changes the COMMON NAME of the status log to "ams tschoening" and tries to map that to COMMON NAME "ams_tschoening" in ovpnconfig.

It's pretty likely a bug...
Comment 3 Michael Tremer 2021-06-17 16:16:56 UTC
Okay, do you have a proposed change?
Comment 4 Thorsten Schöning 2021-06-17 16:42:51 UTC
(In reply to Michael Tremer from comment #3)
> Okay, do you have a proposed change?

Find the reason why "_" is changed to " " and if it's a valid one, don't allow "_" in common names as this can't work then. I didn't find a reason looking at the history and couldn't think of any valid one, so I guess this change is a left-over which might simply be removed.

It might be that past web-UIs didn't allow to generate common names with "_", but some users might used externally generated certs or on the shell or alike and this should increase compatibility with what the web-UI did or whatever.

Though, mapping the common names of both sources 1:1 seems OK and safe to me. So I suggest simply removing the replacement. Everything else can't even be easily documented to users and stuff.