[v2] vpnmain.cgi: set SubjectAlternativeName default during root certificate generation

  Some IPsec implementations such as OpenIKED require SubjectAlternativeName
data on certificates and refuse to establish connections otherwise.

The StrongSwan project also recommends it (see:
https://wiki.strongswan.org/projects/strongswan/wiki/SimpleCA) although
it is currently not enforced by their IPsec software.

For convenience purposes and to raise awareness, this patch adds a default
SubjectAlternativeName based on the machines hostname or IP address. Existing
certificates remain unchanged for obvious reasons.

Fixes #11594

Signed-off-by: Peter Müller <peter.mueller@ipfire.org>
---
 html/cgi-bin/vpnmain.cgi | 2 ++
 1 file changed, 2 insertions(+)
  

Hello,

> On 5 Jan 2020, at 18:11, Peter Müller <peter.mueller@ipfire.org> wrote:
> 
> Some IPsec implementations such as OpenIKED require SubjectAlternativeName
> data on certificates and refuse to establish connections otherwise.
> 
> The StrongSwan project also recommends it (see:
> https://wiki.strongswan.org/projects/strongswan/wiki/SimpleCA) although
> it is currently not enforced by their IPsec software.
> 
> For convenience purposes and to raise awareness, this patch adds a default
> SubjectAlternativeName based on the machines hostname or IP address. Existing
> certificates remain unchanged for obvious reasons.
> 
> Fixes #11594
> 
> Signed-off-by: Peter Müller <peter.mueller@ipfire.org>
> ---
> html/cgi-bin/vpnmain.cgi | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/html/cgi-bin/vpnmain.cgi b/html/cgi-bin/vpnmain.cgi
> index 33b504bc9..9b7bd81ca 100644
> --- a/html/cgi-bin/vpnmain.cgi
> +++ b/html/cgi-bin/vpnmain.cgi
> @@ -822,8 +822,10 @@ END
> 			close IPADDR;
> 			chomp ($ipaddr);
> 			$cgiparams{'ROOTCERT_HOSTNAME'} = (gethostbyaddr(pack("C4", split(/\./, $ipaddr)), 2))[0];
> +			$cgiparams{'SUBJECTALTNAME'} = "DNS:" . (gethostbyaddr(pack("C4", split(/\./, $ipaddr)), 2))[0];

This relies on DNS working at the time of generating the certificate which obviously is a very bad idea.

Since the original code is like this, I guess there is not point in changing it, but you could have however just copied the value of ROOTCERT_HOSTNAME to avoid a second DNS lookup.

> 			if ($cgiparams{'ROOTCERT_HOSTNAME'} eq '') {
> 				$cgiparams{'ROOTCERT_HOSTNAME'} = $ipaddr;
> +				$cgiparams{'SUBJECTALTNAME'} = "IP:" . $ipaddr;
> 			}
> 		}

Does overwriting SUBJECTALTNAME work? There is a place where the user can set this. Is that still being honoured?

-Michael

> 		$cgiparams{'ROOTCERT_COUNTRY'} = $vpnsettings{'ROOTCERT_COUNTRY'} if (!$cgiparams{'ROOTCERT_COUNTRY'});
> -- 
> 2.16.4
>
  

Hello Michael, hello *,

> Hello,
> 
>> On 5 Jan 2020, at 18:11, Peter Müller <peter.mueller@ipfire.org> wrote:
>>
>> Some IPsec implementations such as OpenIKED require SubjectAlternativeName
>> data on certificates and refuse to establish connections otherwise.
>>
>> The StrongSwan project also recommends it (see:
>> https://wiki.strongswan.org/projects/strongswan/wiki/SimpleCA) although
>> it is currently not enforced by their IPsec software.
>>
>> For convenience purposes and to raise awareness, this patch adds a default
>> SubjectAlternativeName based on the machines hostname or IP address. Existing
>> certificates remain unchanged for obvious reasons.
>>
>> Fixes #11594
>>
>> Signed-off-by: Peter Müller <peter.mueller@ipfire.org>
>> ---
>> html/cgi-bin/vpnmain.cgi | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/html/cgi-bin/vpnmain.cgi b/html/cgi-bin/vpnmain.cgi
>> index 33b504bc9..9b7bd81ca 100644
>> --- a/html/cgi-bin/vpnmain.cgi
>> +++ b/html/cgi-bin/vpnmain.cgi
>> @@ -822,8 +822,10 @@ END
>> 			close IPADDR;
>> 			chomp ($ipaddr);
>> 			$cgiparams{'ROOTCERT_HOSTNAME'} = (gethostbyaddr(pack("C4", split(/\./, $ipaddr)), 2))[0];
>> +			$cgiparams{'SUBJECTALTNAME'} = "DNS:" . (gethostbyaddr(pack("C4", split(/\./, $ipaddr)), 2))[0];
> 
> This relies on DNS working at the time of generating the certificate which obviously is a very bad idea.
I consider this being useful if a machine has a correct hostname set. If it fails,
the CGI will fall back to the IP address assigned to red0/ppp0.
> 
> Since the original code is like this, I guess there is not point in changing it, but you could have however just copied the value of ROOTCERT_HOSTNAME to avoid a second DNS lookup.
Agreed. I will hand in a third version of this patch.
> 
>> 			if ($cgiparams{'ROOTCERT_HOSTNAME'} eq '') {
>> 				$cgiparams{'ROOTCERT_HOSTNAME'} = $ipaddr;
>> +				$cgiparams{'SUBJECTALTNAME'} = "IP:" . $ipaddr;
>> 			}
>> 		}
> 
> Does overwriting SUBJECTALTNAME work? There is a place where the user can set this. Is that still being honoured?
As far as I am concerned, yes.

Thanks, and best regards,
Peter Müller

> 
> -Michael
> 
>> 		$cgiparams{'ROOTCERT_COUNTRY'} = $vpnsettings{'ROOTCERT_COUNTRY'} if (!$cgiparams{'ROOTCERT_COUNTRY'});
>> -- 
>> 2.16.4
>>
>