* [PATCH] OpenVPN: Fix to prevent exceedance of OpenSSLs max. validity.
@ 2017-11-11 9:45 Erik Kapfer
2017-11-12 12:15 ` Michael Tremer
2017-11-13 10:22 ` [PATCH] - Added "'TYPE' net" to N2N section since it can only be changed there. - Check for N2N sets days valid maximum to '999999' days. - Check for Roadwarrior sets days valid maximum to '999999' days. - If '999999' days will be exceeded, a warning will be displayed Erik Kapfer
0 siblings, 2 replies; 5+ messages in thread
From: Erik Kapfer @ 2017-11-11 9:45 UTC (permalink / raw)
To: development
[-- Attachment #1: Type: text/plain, Size: 1748 bytes --]
- If the OpenSSL maximum of '999999' will be exceeded over the WUI, the entry in
OpenVPNs database index.txt will be written without a timestamp
and crashes the database which blocks the creation of new clients.
To prevent this, a check has been set which restricts the data field
of 'valid til days' to '6' numerics.
Fixes: #10482
---
html/cgi-bin/ovpnmain.cgi | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
index ceb88c1..8f45f04 100644
--- a/html/cgi-bin/ovpnmain.cgi
+++ b/html/cgi-bin/ovpnmain.cgi
@@ -4039,6 +4039,14 @@ if ($cgiparams{'TYPE'} eq 'net') {
goto VPNCONF_ERROR;
}
+ # Check that OpenSSL maximum of valid days won´t be exceeded
+ if (length($cgiparams{'DAYS_VALID'}) > 6) {
+ $errormessage = $Lang::tr{'invalid input for valid till days'};
+ unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
+ rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
+ goto VPNCONF_ERROR;
+ }
+
if ($cgiparams{'ENABLED'} !~ /^(on|off)$/) {
$errormessage = $Lang::tr{'invalid input'};
goto VPNCONF_ERROR;
@@ -4221,6 +4229,12 @@ if ($cgiparams{'TYPE'} eq 'net') {
goto VPNCONF_ERROR;
}
+ # Check that OpenSSL maximum of valid days won´t be exceeded
+ if (length($cgiparams{'DAYS_VALID'}) > 6) {
+ $errormessage = $Lang::tr{'invalid input for valid till days'};
+ goto VPNCONF_ERROR;
+ }
+
# Replace empty strings with a .
(my $ou = $cgiparams{'CERT_OU'}) =~ s/^\s*$/\./;
(my $city = $cgiparams{'CERT_CITY'}) =~ s/^\s*$/\./;
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] OpenVPN: Fix to prevent exceedance of OpenSSLs max. validity.
2017-11-11 9:45 [PATCH] OpenVPN: Fix to prevent exceedance of OpenSSLs max. validity Erik Kapfer
@ 2017-11-12 12:15 ` Michael Tremer
2017-11-14 13:20 ` ummeegge
2017-11-13 10:22 ` [PATCH] - Added "'TYPE' net" to N2N section since it can only be changed there. - Check for N2N sets days valid maximum to '999999' days. - Check for Roadwarrior sets days valid maximum to '999999' days. - If '999999' days will be exceeded, a warning will be displayed Erik Kapfer
1 sibling, 1 reply; 5+ messages in thread
From: Michael Tremer @ 2017-11-12 12:15 UTC (permalink / raw)
To: development
[-- Attachment #1: Type: text/plain, Size: 2085 bytes --]
Hi,
On Sat, 2017-11-11 at 10:45 +0100, Erik Kapfer wrote:
> - If the OpenSSL maximum of '999999' will be exceeded over the WUI, the entry in
> OpenVPNs database index.txt will be written without a timestamp
> and crashes the database which blocks the creation of new clients.
> To prevent this, a check has been set which restricts the data field
> of 'valid til days' to '6' numerics.
>
> Fixes: #10482
> ---
> html/cgi-bin/ovpnmain.cgi | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
> index ceb88c1..8f45f04 100644
> --- a/html/cgi-bin/ovpnmain.cgi
> +++ b/html/cgi-bin/ovpnmain.cgi
> @@ -4039,6 +4039,14 @@ if ($cgiparams{'TYPE'} eq 'net') {
> goto VPNCONF_ERROR;
> }
>
> + # Check that OpenSSL maximum of valid days won´t be exceeded
> + if (length($cgiparams{'DAYS_VALID'}) > 6) {
> + $errormessage = $Lang::tr{'invalid input for valid till days'};
> + unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
> + rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
> + goto VPNCONF_ERROR;
> + }
> +
I think it would be better just to check if DAYS_VALID is less then
999999. Checking the length of the string wasn't really obvious for me
what was actually going to be achieved here.
> if ($cgiparams{'ENABLED'} !~ /^(on|off)$/) {
> $errormessage = $Lang::tr{'invalid input'};
> goto VPNCONF_ERROR;
> @@ -4221,6 +4229,12 @@ if ($cgiparams{'TYPE'} eq 'net') {
> goto VPNCONF_ERROR;
> }
>
> + # Check that OpenSSL maximum of valid days won´t be exceeded
> + if (length($cgiparams{'DAYS_VALID'}) > 6) {
> + $errormessage = $Lang::tr{'invalid input for valid till days'};
> + goto VPNCONF_ERROR;
> + }
> +
> # Replace empty strings with a .
> (my $ou = $cgiparams{'CERT_OU'}) =~ s/^\s*$/\./;
> (my $city = $cgiparams{'CERT_CITY'}) =~ s/^\s*$/\./;
-Michael
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] OpenVPN: Fix to prevent exceedance of OpenSSLs max. validity.
2017-11-12 12:15 ` Michael Tremer
@ 2017-11-14 13:20 ` ummeegge
2017-11-14 14:11 ` Michael Tremer
0 siblings, 1 reply; 5+ messages in thread
From: ummeegge @ 2017-11-14 13:20 UTC (permalink / raw)
To: development
[-- Attachment #1: Type: text/plain, Size: 2696 bytes --]
Hi Michael,
have now seen the format of the sended patch --> https://lists.ipfire.org/pipermail/development/2017-November/003732.html is somehow broken (seems that sendmail and me have some incompatibilities :-| ) whereby in Git --> https://git.ipfire.org/?p=people/ummeegge/ipfire-2.x.git;a=commit;h=3e58db4871f707f6ea79e6f8ca219ee03008fe76 is it OK… Am currently not sure what i did wrong there.
Michael, if the format is useless let it me know will try then to send it again...
Best,
Erik
Am 12.11.2017 um 13:15 schrieb Michael Tremer:
> Hi,
>
> On Sat, 2017-11-11 at 10:45 +0100, Erik Kapfer wrote:
>> - If the OpenSSL maximum of '999999' will be exceeded over the WUI, the entry in
>> OpenVPNs database index.txt will be written without a timestamp
>> and crashes the database which blocks the creation of new clients.
>> To prevent this, a check has been set which restricts the data field
>> of 'valid til days' to '6' numerics.
>>
>> Fixes: #10482
>> ---
>> html/cgi-bin/ovpnmain.cgi | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
>> index ceb88c1..8f45f04 100644
>> --- a/html/cgi-bin/ovpnmain.cgi
>> +++ b/html/cgi-bin/ovpnmain.cgi
>> @@ -4039,6 +4039,14 @@ if ($cgiparams{'TYPE'} eq 'net') {
>> goto VPNCONF_ERROR;
>> }
>>
>> + # Check that OpenSSL maximum of valid days won´t be exceeded
>> + if (length($cgiparams{'DAYS_VALID'}) > 6) {
>> + $errormessage = $Lang::tr{'invalid input for valid till days'};
>> + unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
>> + rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
>> + goto VPNCONF_ERROR;
>> + }
>> +
>
> I think it would be better just to check if DAYS_VALID is less then
> 999999. Checking the length of the string wasn't really obvious for me
> what was actually going to be achieved here.
>
>> if ($cgiparams{'ENABLED'} !~ /^(on|off)$/) {
>> $errormessage = $Lang::tr{'invalid input'};
>> goto VPNCONF_ERROR;
>> @@ -4221,6 +4229,12 @@ if ($cgiparams{'TYPE'} eq 'net') {
>> goto VPNCONF_ERROR;
>> }
>>
>> + # Check that OpenSSL maximum of valid days won´t be exceeded
>> + if (length($cgiparams{'DAYS_VALID'}) > 6) {
>> + $errormessage = $Lang::tr{'invalid input for valid till days'};
>> + goto VPNCONF_ERROR;
>> + }
>> +
>> # Replace empty strings with a .
>> (my $ou = $cgiparams{'CERT_OU'}) =~ s/^\s*$/\./;
>> (my $city = $cgiparams{'CERT_CITY'}) =~ s/^\s*$/\./;
>
> -Michael
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] OpenVPN: Fix to prevent exceedance of OpenSSLs max. validity.
2017-11-14 13:20 ` ummeegge
@ 2017-11-14 14:11 ` Michael Tremer
0 siblings, 0 replies; 5+ messages in thread
From: Michael Tremer @ 2017-11-14 14:11 UTC (permalink / raw)
To: development
[-- Attachment #1: Type: text/plain, Size: 3427 bytes --]
Hi,
yes, it would have been better to send a patch against next instead of a patch
for the other patch. That makes it a lot harder to see what has actually
changed.
I suppose you can squash both commits together to one and then just send it by
using "git send-email -1 --to development(a)lists.ipfire.org".
https://stackoverflow.com/questions/2563632/how-can-i-merge-two-commits-into-one
Best,
-Michael
On Tue, 2017-11-14 at 14:20 +0100, ummeegge wrote:
> Hi Michael,
> have now seen the format of the sended patch --> https://lists.ipfire.org/pipe
> rmail/development/2017-November/003732.html is somehow broken (seems that
> sendmail and me have some incompatibilities :-| ) whereby in Git --> https://g
> it.ipfire.org/?p=people/ummeegge/ipfire-
> 2.x.git;a=commit;h=3e58db4871f707f6ea79e6f8ca219ee03008fe76 is it OK… Am
> currently not sure what i did wrong there.
> Michael, if the format is useless let it me know will try then to send it
> again...
>
>
> Best,
>
> Erik
>
>
> Am 12.11.2017 um 13:15 schrieb Michael Tremer:
>
> > Hi,
> >
> > On Sat, 2017-11-11 at 10:45 +0100, Erik Kapfer wrote:
> > > - If the OpenSSL maximum of '999999' will be exceeded over the WUI, the
> > > entry in
> > > OpenVPNs database index.txt will be written without a timestamp
> > > and crashes the database which blocks the creation of new clients.
> > > To prevent this, a check has been set which restricts the data field
> > > of 'valid til days' to '6' numerics.
> > >
> > > Fixes: #10482
> > > ---
> > > html/cgi-bin/ovpnmain.cgi | 14 ++++++++++++++
> > > 1 file changed, 14 insertions(+)
> > >
> > > diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
> > > index ceb88c1..8f45f04 100644
> > > --- a/html/cgi-bin/ovpnmain.cgi
> > > +++ b/html/cgi-bin/ovpnmain.cgi
> > > @@ -4039,6 +4039,14 @@ if ($cgiparams{'TYPE'} eq 'net') {
> > > goto VPNCONF_ERROR;
> > > }
> > >
> > > + # Check that OpenSSL maximum of valid days won´t be exceeded
> > > + if (length($cgiparams{'DAYS_VALID'}) > 6) {
> > > + $errormessage = $Lang::tr{'invalid input for valid till
> > > days'};
> > > + unlink
> > > ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.co
> > > nf") or die "Removing Configfile fail: $!";
> > > + rmdir
> > > ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing
> > > Directory fail: $!";
> > > + goto VPNCONF_ERROR;
> > > + }
> > > +
> >
> > I think it would be better just to check if DAYS_VALID is less then
> > 999999. Checking the length of the string wasn't really obvious for me
> > what was actually going to be achieved here.
> >
> > > if ($cgiparams{'ENABLED'} !~ /^(on|off)$/) {
> > > $errormessage = $Lang::tr{'invalid input'};
> > > goto VPNCONF_ERROR;
> > > @@ -4221,6 +4229,12 @@ if ($cgiparams{'TYPE'} eq 'net') {
> > > goto VPNCONF_ERROR;
> > > }
> > >
> > > + # Check that OpenSSL maximum of valid days won´t be
> > > exceeded
> > > + if (length($cgiparams{'DAYS_VALID'}) > 6) {
> > > + $errormessage = $Lang::tr{'invalid input for
> > > valid till days'};
> > > + goto VPNCONF_ERROR;
> > > + }
> > > +
> > > # Replace empty strings with a .
> > > (my $ou = $cgiparams{'CERT_OU'}) =~ s/^\s*$/\./;
> > > (my $city = $cgiparams{'CERT_CITY'}) =~ s/^\s*$/\./;
> >
> > -Michael
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] - Added "'TYPE' net" to N2N section since it can only be changed there. - Check for N2N sets days valid maximum to '999999' days. - Check for Roadwarrior sets days valid maximum to '999999' days. - If '999999' days will be exceeded, a warning will be displayed.
2017-11-11 9:45 [PATCH] OpenVPN: Fix to prevent exceedance of OpenSSLs max. validity Erik Kapfer
2017-11-12 12:15 ` Michael Tremer
@ 2017-11-13 10:22 ` Erik Kapfer
1 sibling, 0 replies; 5+ messages in thread
From: Erik Kapfer @ 2017-11-13 10:22 UTC (permalink / raw)
To: development
[-- Attachment #1: Type: text/plain, Size: 1594 bytes --]
---
html/cgi-bin/ovpnmain.cgi | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
index 8f45f04..9c383f7 100644
--- a/html/cgi-bin/ovpnmain.cgi
+++ b/html/cgi-bin/ovpnmain.cgi
@@ -4040,11 +4040,13 @@ if ($cgiparams{'TYPE'} eq 'net') {
}
# Check that OpenSSL maximum of valid days won´t be exceeded
- if (length($cgiparams{'DAYS_VALID'}) > 6) {
- $errormessage = $Lang::tr{'invalid input for valid till days'};
- unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
- rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
- goto VPNCONF_ERROR;
+ if ($cgiparams{'TYPE'} eq 'net') {
+ if ($cgiparams{'DAYS_VALID'} > '999999') {
+ $errormessage = $Lang::tr{'invalid input for valid till days'};
+ unlink ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}/$cgiparams{'NAME'}.conf") or die "Removing Configfile fail: $!";
+ rmdir ("${General::swroot}/ovpn/n2nconf/$cgiparams{'NAME'}") || die "Removing Directory fail: $!";
+ goto VPNCONF_ERROR;
+ }
}
if ($cgiparams{'ENABLED'} !~ /^(on|off)$/) {
@@ -4230,7 +4232,7 @@ if ($cgiparams{'TYPE'} eq 'net') {
}
# Check that OpenSSL maximum of valid days won´t be exceeded
- if (length($cgiparams{'DAYS_VALID'}) > 6) {
+ if ($cgiparams{'DAYS_VALID'} > '999999') {
$errormessage = $Lang::tr{'invalid input for valid till days'};
goto VPNCONF_ERROR;
}
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-11-14 14:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-11 9:45 [PATCH] OpenVPN: Fix to prevent exceedance of OpenSSLs max. validity Erik Kapfer
2017-11-12 12:15 ` Michael Tremer
2017-11-14 13:20 ` ummeegge
2017-11-14 14:11 ` Michael Tremer
2017-11-13 10:22 ` [PATCH] - Added "'TYPE' net" to N2N section since it can only be changed there. - Check for N2N sets days valid maximum to '999999' days. - Check for Roadwarrior sets days valid maximum to '999999' days. - If '999999' days will be exceeded, a warning will be displayed Erik Kapfer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox