Re: [PATCH] OpenVPN: Move the OpenSSL configuration file out of /var/ipfire

[Michael Tremer <michael.tremer@ipfire.org>]

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

Hello,

Thanks for testing this.

> On 8 Jun 2024, at 09:40, Adolf Belka <adolf.belka(a)ipfire.org> wrote:
> 
> Hi Michael,
> 
> On 07/06/2024 18:01, Michael Tremer wrote:
>> We should not have any configuration files that we share in this place,
>> therefore this patch is moving it into /usr/share/openvpn where we
>> should be able to update it without any issues.
>> 
>> Signed-off-by: Michael Tremer <michael.tremer(a)ipfire.org>
>> ---
>>  config/ovpn/openvpn-crl-updater |  3 +--
>>  config/rootfiles/common/openvpn |  2 +-
>>  html/cgi-bin/ovpnmain.cgi       | 20 ++++++++++----------
>>  lfs/openvpn                     |  6 ++++++
>>  4 files changed, 18 insertions(+), 13 deletions(-)
>> 
>> diff --git a/config/ovpn/openvpn-crl-updater b/config/ovpn/openvpn-crl-updater
>> index 5fbe21080..5008d6725 100644
>> --- a/config/ovpn/openvpn-crl-updater
>> +++ b/config/ovpn/openvpn-crl-updater
>> @@ -43,7 +43,6 @@ OVPN="/var/ipfire/ovpn"
>>  CRL="${OVPN}/crls/cacrl.pem"
>>  CAKEY="${OVPN}/ca/cakey.pem"
>>  CACERT="${OVPN}/ca/cacert.pem"
>> -OPENSSLCONF="${OVPN}/openssl/ovpn.cnf"
>>    # Check if CRL is presant or if OpenVPN is active
>>  if [ ! -e "${CAKEY}" ]; then
>> @@ -76,7 +75,7 @@ UPDATE="14"
>>  ## Mainpart
>>  # Check if OpenVPNs CRL needs to be renewed
>>  if [ ${NEXTUPDATE} -le ${UPDATE} ]; then
>> -    if openssl ca -gencrl -keyfile "${CAKEY}" -cert "${CACERT}" -out "${CRL}" -config "${OPENSSLCONF}"; then
>> +    if openssl ca -gencrl -keyfile "${CAKEY}" -cert "${CACERT}" -out "${CRL}" -config "/usr/share/openvpn/ovpn.cnf"; then
>>   logger -t openvpn "CRL has been updated"
>>      else
>>   logger -t openvpn "error: Could not update CRL"
>> diff --git a/config/rootfiles/common/openvpn b/config/rootfiles/common/openvpn
>> index d9848a579..c0d49bfad 100644
>> --- a/config/rootfiles/common/openvpn
>> +++ b/config/rootfiles/common/openvpn
>> @@ -25,6 +25,7 @@ usr/sbin/openvpn-authenticator
>>  #usr/share/doc/openvpn/openvpn.8.html
>>  #usr/share/man/man5/openvpn-examples.5
>>  #usr/share/man/man8/openvpn.8
>> +usr/share/openvpn/openssl.cnf
> In the rootfile the file name is not only moved from /var/ipfire/ovpn/openssl/ but also renamed from ovpn.cnf to openssl.cnf but all the rest of the code continues to use ovpn.cnf

Oh.

>>  var/ipfire/ovpn/ca
>>  var/ipfire/ovpn/caconfig
>>  var/ipfire/ovpn/ccd
>> @@ -35,7 +36,6 @@ var/ipfire/ovpn/certs/serial
>>  var/ipfire/ovpn/crls
>>  var/ipfire/ovpn/n2nconf
>>  #var/ipfire/ovpn/openssl
>> -var/ipfire/ovpn/openssl/ovpn.cnf
>>  var/ipfire/ovpn/openvpn-authenticator
>>  var/ipfire/ovpn/ovpn-leases.db
>>  var/ipfire/ovpn/ovpnconfig
>> diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
>> index c92d0237d..f0172978f 100755
>> --- a/html/cgi-bin/ovpnmain.cgi
>> +++ b/html/cgi-bin/ovpnmain.cgi
>> @@ -1836,7 +1836,7 @@ END
>>   '-days', '999999', '-newkey', 'rsa:4096', '-sha512',
>>   '-keyout', "${General::swroot}/ovpn/ca/cakey.pem",
>>   '-out', "${General::swroot}/ovpn/ca/cacert.pem",
>> - '-config',"${General::swroot}/ovpn/openssl/ovpn.cnf")) {
>> + '-config', "/usr/share/openvpn/ovpn.cnf")) {
>>   $errormessage = "$Lang::tr{'cant start openssl'}: $!";
>>   goto ROOTCERT_ERROR;
>>       }
>> @@ -1868,7 +1868,7 @@ END
>>   '-keyout', "${General::swroot}/ovpn/certs/serverkey.pem",
>>   '-out', "${General::swroot}/ovpn/certs/serverreq.pem",
>>   '-extensions', 'server',
>> - '-config', "${General::swroot}/ovpn/openssl/ovpn.cnf" )) {
>> + '-config', "/usr/share/openvpn/ovpn.cnf" )) {
>>   $errormessage = "$Lang::tr{'cant start openssl'}: $!";
>>   unlink ("${General::swroot}/ovpn/certs/serverkey.pem");
>>   unlink ("${General::swroot}/ovpn/certs/serverreq.pem");
>> @@ -1885,7 +1885,7 @@ END
>>   '-in',  "${General::swroot}/ovpn/certs/serverreq.pem",
>>   '-out', "${General::swroot}/ovpn/certs/servercert.pem",
>>   '-extensions', 'server',
>> - '-config', "${General::swroot}/ovpn/openssl/ovpn.cnf");
>> + '-config', "/usr/share/openvpn/ovpn.cnf");
>>   if ($?) {
>>       $errormessage = "$Lang::tr{'openssl produced an error'}: $?";
>>       unlink ("${General::swroot}/ovpn/ca/cakey.pem");
>> @@ -1904,7 +1904,7 @@ END
>>   # System call is safe, because all arguments are passed as array.
>>   system('/usr/bin/openssl', 'ca', '-gencrl',
>>   '-out', "${General::swroot}/ovpn/crls/cacrl.pem",
>> - '-config', "${General::swroot}/ovpn/openssl/ovpn.cnf" );
>> + '-config', "/usr/share/openvpn/ovpn.cnf" );
>>   if ($?) {
>>       $errormessage = "$Lang::tr{'openssl produced an error'}: $?";
>>       unlink ("${General::swroot}/ovpn/certs/serverkey.pem");
>> @@ -2426,8 +2426,8 @@ else
>>     if ($confighash{$cgiparams{'KEY'}}) {
>>   # Revoke certificate if certificate was deleted and rewrite the CRL
>> - &General::system("/usr/bin/openssl", "ca", "-revoke", "${General::swroot}/ovpn/certs/$confighash{$cgiparams{'KEY'}}[1]cert.pem", "-config", "${General::swroot}/ovpn/openssl/ovpn.cnf");
>> - &General::system("/usr/bin/openssl", "ca", "-gencrl", "-out", "${General::swroot}/ovpn/crls/cacrl.pem", "-config", "${General::swroot}/ovpn/openssl/ovpn.cnf");
>> + &General::system("/usr/bin/openssl", "ca", "-revoke", "${General::swroot}/ovpn/certs/$confighash{$cgiparams{'KEY'}}[1]cert.pem", "-config", "/usr/share/openvpn/ovpn.cnf");
>> + &General::system("/usr/bin/openssl", "ca", "-gencrl", "-out", "${General::swroot}/ovpn/crls/cacrl.pem", "-config", "/usr/share/openvpn/ovpn.cnf");
>>    ###
>>  # m.a.d net2net
>> @@ -2480,7 +2480,7 @@ else
>>   &General::system("/usr/local/bin/openvpnctrl", "-drrd", "$confighash{$cgiparams{'KEY'}}[1]");
>>     delete $confighash{$cgiparams{'KEY'}};
>> - &General::system("/usr/bin/openssl", "ca", "-gencrl", "-out", "${General::swroot}/ovpn/crls/cacrl.pem", "-config", "${General::swroot}/ovpn/openssl/ovpn.cnf");
>> + &General::system("/usr/bin/openssl", "ca", "-gencrl", "-out", "${General::swroot}/ovpn/crls/cacrl.pem", "-config", "/usr/share/openvpn/ovpn.cnf");
>>   &General::writehasharray("${General::swroot}/ovpn/ovpnconfig", \%confighash);
>>     } else {
>> @@ -4053,7 +4053,7 @@ if ($cgiparams{'TYPE'} eq 'net') {
>>   '-batch', '-notext',
>>   '-in', $filename,
>>   '-out', "${General::swroot}/ovpn/certs/$cgiparams{'NAME'}cert.pem",
>> - '-config',"${General::swroot}/ovpn/openssl/ovpn.cnf");
>> + '-config', "/usr/share/openvpn/ovpn.cnf");
>>       if ($?) {
>>   $errormessage = "$Lang::tr{'openssl produced an error'}: $?";
>>   unlink ($filename);
>> @@ -4266,7 +4266,7 @@ if ($cgiparams{'TYPE'} eq 'net') {
>>   '-newkey', 'rsa:4096',
>>   '-keyout', "${General::swroot}/ovpn/certs/$cgiparams{'NAME'}key.pem",
>>   '-out', "${General::swroot}/ovpn/certs/$cgiparams{'NAME'}req.pem",
>> - '-config',"${General::swroot}/ovpn/openssl/ovpn.cnf")) {
>> + '-config', "/usr/share/openvpn/ovpn.cnf")) {
>>       $errormessage = "$Lang::tr{'cant start openssl'}: $!";
>>       unlink ("${General::swroot}/ovpn/certs/$cgiparams{'NAME'}key.pem");
>>       unlink ("${General::swroot}/ovpn/certs/$cgiparams{'NAME'}req.pem");
>> @@ -4280,7 +4280,7 @@ if ($cgiparams{'TYPE'} eq 'net') {
>>   '-batch', '-notext',
>>   '-in',  "${General::swroot}/ovpn/certs/$cgiparams{'NAME'}req.pem",
>>   '-out', "${General::swroot}/ovpn/certs/$cgiparams{'NAME'}cert.pem",
>> - '-config',"${General::swroot}/ovpn/openssl/ovpn.cnf");
>> + '-config', "/usr/share/openvpn/ovpn.cnf");
>>       if ($?) {
>>   $errormessage = "$Lang::tr{'openssl produced an error'}: $?";
>>   unlink ("${General::swroot}/ovpn/certs/$cgiparams{'NAME'}key.pem");
>> diff --git a/lfs/openvpn b/lfs/openvpn
>> index b71b4ccc9..0704aa438 100644
>> --- a/lfs/openvpn
>> +++ b/lfs/openvpn
>> @@ -101,6 +101,12 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
>>   chown root:root /etc/fcron.daily/openvpn-crl-updater
>>   chmod 750 /etc/fcron.daily/openvpn-crl-updater
>>  + # Move the OpenSSL configuration file out of /var/ipfire
>> + mkdir -pv /usr/share/openvpn
> This creates the new directory.
>> + mv -v /var/ipfire/ovpn/openssl/ovpn.cnf \
>> + /usr/share/openvpn/
> This then moves the ovpn.cnf file from the old location to the new one but keeps the name the same. This will then mismatch with the rootfile change.
>> + rmdir -v /usr/share/openvpn
> This then seems to me to be trying to delete the newly created directory which seems incorrect to me unless I have misunderstood what is trying to be done with this overall patch, which could also be the case.

Yes, I have no idea what I did when I developed this the first time. Nothing good obviously.

I will send patches.

-Michael

> Regards,
> Adolf.
>> +
>>   # Install authenticator
>>   install -v -m 755 $(DIR_SRC)/config/ovpn/openvpn-authenticator \
>>   /usr/sbin/openvpn-authenticator
> 
> -- 
> Sent from my laptop