OpenVPN: Prevent internal server error cause of bad header wrapper

Message ID 1529572373-16580-1-git-send-email-erik.kapfer@ipfire.org
State Accepted
Commit 8ae4010b312830bce82721325f0aeae524b2810a
Headers
Series OpenVPN: Prevent internal server error cause of bad header wrapper |

Commit Message

Erik Kapfer June 21, 2018, 7:12 p.m. UTC
  This fixes #11772 .

If the X509 are deleted, the openvpnctrl output generates a bad header wrapper error from the CGI
which causes an internal server error. The redirection of the openvpnctrl output fixes this.

Signed-off-by: Erik Kapfer <erik.kapfer@ipfire.org>
---
 html/cgi-bin/ovpnmain.cgi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

ummeegge July 2, 2018, 10:26 p.m. UTC | #1
Hi,
just wanted to know if this fix will be applied ?

Best,

Erik


Am Donnerstag, den 21.06.2018, 11:12 +0200 schrieb Erik Kapfer:
> This fixes #11772 .
> 
> If the X509 are deleted, the openvpnctrl output generates a bad
> header wrapper error from the CGI
> which causes an internal server error. The redirection of the
> openvpnctrl output fixes this.
> 
> Signed-off-by: Erik Kapfer <erik.kapfer@ipfire.org>
> ---
>  html/cgi-bin/ovpnmain.cgi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
> index 4bc3473..5cd19a0 100644
> --- a/html/cgi-bin/ovpnmain.cgi
> +++ b/html/cgi-bin/ovpnmain.cgi
> @@ -1181,7 +1181,7 @@ SETTINGS_ERROR:
>  	    delete $confighash{$cgiparams{'$key'}};
>  	}
>  
> -	system ("/usr/local/bin/openvpnctrl -drrd $name");
> +	system ("/usr/local/bin/openvpnctrl -drrd $name
> &>/dev/null");
>      }
>      while ($file = glob("${General::swroot}/ovpn/ca/*")) {
>  	unlink $file;
  
Michael Tremer July 3, 2018, 7:52 p.m. UTC | #2
Yes, I just did.

I collected all the patches and merged all the package updates first and today I
merged all other smaller changes.

Best,
-Michael

On Mon, 2018-07-02 at 14:26 +0200, ummeegge wrote:
> Hi,
> just wanted to know if this fix will be applied ?
> 
> Best,
> 
> Erik
> 
> 
> Am Donnerstag, den 21.06.2018, 11:12 +0200 schrieb Erik Kapfer:
> > This fixes #11772 .
> > 
> > If the X509 are deleted, the openvpnctrl output generates a bad
> > header wrapper error from the CGI
> > which causes an internal server error. The redirection of the
> > openvpnctrl output fixes this.
> > 
> > Signed-off-by: Erik Kapfer <erik.kapfer@ipfire.org>
> > ---
> >  html/cgi-bin/ovpnmain.cgi | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
> > index 4bc3473..5cd19a0 100644
> > --- a/html/cgi-bin/ovpnmain.cgi
> > +++ b/html/cgi-bin/ovpnmain.cgi
> > @@ -1181,7 +1181,7 @@ SETTINGS_ERROR:
> >  	    delete $confighash{$cgiparams{'$key'}};
> >  	}
> >  
> > -	system ("/usr/local/bin/openvpnctrl -drrd $name");
> > +	system ("/usr/local/bin/openvpnctrl -drrd $name
> > &>/dev/null");
> >      }
> >      while ($file = glob("${General::swroot}/ovpn/ca/*")) {
> >  	unlink $file;
  
ummeegge July 3, 2018, 10:18 p.m. UTC | #3
Hi Michael,
thanks for merging. I have seen that you´d applied version 1 of this
patch
https://git.ipfire.org/?p=ipfire-2.x.git;a=blobdiff;f=html/cgi-bin/ovpnmain.cgi;h=c0c7cff6d87f6e18206129ab196172be61683a38;hp=5cd19a0f38f564c54e672814e0b5918134889b17;hb=15a3aa45cf27c61a581f892b5f3a3905335a12b0;hpb=8ae4010b312830bce82721325f0aeae524b2810a

but there´s a version 2 of it 
https://patchwork.ipfire.org/patch/1842/ 
which we should in any case prefer.

This is my fault since i didn´t use the same commit name, just used the same message-id in the commit.

Sorry for that.

Best,

Erik

P.S. I do have some more OpenVPN patches (extensions no bugs), should i commit some more or should we wait until the next release ?

Am Dienstag, den 03.07.2018, 10:52 +0100 schrieb Michael Tremer:
> Yes, I just did.
> 
> I collected all the patches and merged all the package updates first
> and today I
> merged all other smaller changes.
> 
> Best,
> -Michael
> 
> On Mon, 2018-07-02 at 14:26 +0200, ummeegge wrote:
> > 
> > Hi,
> > just wanted to know if this fix will be applied ?
> > 
> > Best,
> > 
> > Erik
> >
  
Michael Tremer July 4, 2018, 12:31 a.m. UTC | #4
Hi,

On Tue, 2018-07-03 at 14:18 +0200, ummeegge wrote:
> Hi Michael,
> thanks for merging. I have seen that you´d applied version 1 of this
> patch
> https://git.ipfire.org/?p=ipfire-2.x.git;a=blobdiff;f=html/cgi-bin/ovpnmain.cg
> i;h=c0c7cff6d87f6e18206129ab196172be61683a38;hp=5cd19a0f38f564c54e672814e0b591
> 8134889b17;hb=15a3aa45cf27c61a581f892b5f3a3905335a12b0;hpb=8ae4010b312830bce82
> 721325f0aeae524b2810a
> 
> but there´s a version 2 of it 
> https://patchwork.ipfire.org/patch/1842/ 
> which we should in any case prefer.

Oh sorry. If you can, please mark the v1 as such in Patchwork. I am not sure if
we can trigger this automatically via email.

> This is my fault since i didn´t use the same commit name, just used the same
> message-id in the commit.
> 
> Sorry for that.

No problem.

> 
> Best,
> 
> Erik
> 
> P.S. I do have some more OpenVPN patches (extensions no bugs), should i commit
> some more or should we wait until the next release ?

What are those?

Best,
-Michael

> 
> Am Dienstag, den 03.07.2018, 10:52 +0100 schrieb Michael Tremer:
> > Yes, I just did.
> > 
> > I collected all the patches and merged all the package updates first
> > and today I
> > merged all other smaller changes.
> > 
> > Best,
> > -Michael
> > 
> > On Mon, 2018-07-02 at 14:26 +0200, ummeegge wrote:
> > > 
> > > Hi,
> > > just wanted to know if this fix will be applied ?
> > > 
> > > Best,
> > > 
> > > Erik
> > >
  
ummeegge July 4, 2018, 3:40 a.m. UTC | #5
Hi Michael,

Am Dienstag, den 03.07.2018, 15:31 +0100 schrieb Michael Tremer:
> Hi,
> 
> On Tue, 2018-07-03 at 14:18 +0200, ummeegge wrote:
> > 
> > Hi Michael,
> > thanks for merging. I have seen that you´d applied version 1 of
> > this
> > patch
> > https://git.ipfire.org/?p=ipfire-2.x.git;a=blobdiff;f=html/cgi-bin/
> > ovpnmain.cg
> > i;h=c0c7cff6d87f6e18206129ab196172be61683a38;hp=5cd19a0f38f564c54e6
> > 72814e0b591
> > 8134889b17;hb=15a3aa45cf27c61a581f892b5f3a3905335a12b0;hpb=8ae4010b
> > 312830bce82
> > 721325f0aeae524b2810a
> > 
> > but there´s a version 2 of it 
> > https://patchwork.ipfire.org/patch/1842/ 
> > which we should in any case prefer.
> Oh sorry. If you can, please mark the v1 as such in Patchwork. I am
> not sure if
> we can trigger this automatically via email.
You mean to mark the first patch as v1 ? In that case i need to setup
the old patch again as a new one and send it as answer to the v2 patch.

> > 
> > P.S. I do have some more OpenVPN patches (extensions no bugs),
> > should i commit
> > some more or should we wait until the next release ?
> What are those?
Wanted to finish the 2.4 OpenVPN project in the course which we did
discussed some time ago. So i thought about this order:

1) Automatic cipher negotiation for RWs only (checkbox in advanced
section)
2) tls-crypt for N2N only (checkbox in N2N main menu).
3) LZ4 compression possibility for N2N and RW (menu with possiblity for
none, lzo, lz4v2)
4) Clean up ovpnmain.cgi from mtu-discovery since there are some old
code blocks left.

There is more but to get the old list shorter for the first.

Best,

Erik
  
Michael Tremer July 4, 2018, 11:59 p.m. UTC | #6
On Tue, 2018-07-03 at 19:40 +0200, ummeegge wrote:
> Hi Michael,
> 
> Am Dienstag, den 03.07.2018, 15:31 +0100 schrieb Michael Tremer:
> > Hi,
> > 
> > On Tue, 2018-07-03 at 14:18 +0200, ummeegge wrote:
> > > 
> > > Hi Michael,
> > > thanks for merging. I have seen that you´d applied version 1 of
> > > this
> > > patch
> > > https://git.ipfire.org/?p=ipfire-2.x.git;a=blobdiff;f=html/cgi-bin/
> > > ovpnmain.cg
> > > i;h=c0c7cff6d87f6e18206129ab196172be61683a38;hp=5cd19a0f38f564c54e6
> > > 72814e0b591
> > > 8134889b17;hb=15a3aa45cf27c61a581f892b5f3a3905335a12b0;hpb=8ae4010b
> > > 312830bce82
> > > 721325f0aeae524b2810a
> > > 
> > > but there´s a version 2 of it 
> > > https://patchwork.ipfire.org/patch/1842/ 
> > > which we should in any case prefer.
> > 
> > Oh sorry. If you can, please mark the v1 as such in Patchwork. I am
> > not sure if
> > we can trigger this automatically via email.
> 
> You mean to mark the first patch as v1 ? In that case i need to setup
> the old patch again as a new one and send it as answer to the v2 patch.

No, not as v1, but in Patchwork, when you log in manually, you can set a patch
as superseeded. It is a bit annoying to do this manually, but I do not know
about any better way.

> > > 
> > > P.S. I do have some more OpenVPN patches (extensions no bugs),
> > > should i commit
> > > some more or should we wait until the next release ?
> > 
> > What are those?
> 
> Wanted to finish the 2.4 OpenVPN project in the course which we did
> discussed some time ago. So i thought about this order:
> 
> 1) Automatic cipher negotiation for RWs only (checkbox in advanced
> section)

Isn't that something you would always want?

> 2) tls-crypt for N2N only (checkbox in N2N main menu).
> 3) LZ4 compression possibility for N2N and RW (menu with possiblity for
> none, lzo, lz4v2)

Yes, that should be a dropdown then instead of a checkbox.

> 4) Clean up ovpnmain.cgi from mtu-discovery since there are some old
> code blocks left.

Okay, cool.

> There is more but to get the old list shorter for the first.

I guess it is best to start with the cleanup and then send in the other things
one patch, or one patchset at a time.

Best,
-Michael

> 
> Best,
> 
> Erik
  
ummeegge July 5, 2018, 8:59 a.m. UTC | #7
Hi Michael,

Am Mittwoch, den 04.07.2018, 14:59 +0100 schrieb Michael Tremer:

> > 
> > You mean to mark the first patch as v1 ? In that case i need to
> > setup
> > the old patch again as a new one and send it as answer to the v2
> > patch.
> 
> No, not as v1, but in Patchwork, when you log in manually, you can
> set a patch
> as superseeded. It is a bit annoying to do this manually, but I do
> not know
> about any better way.
OK, new ways :-). But i do have currently no access with my
credentials.


> 
> > > > 
> > > > P.S. I do have some more OpenVPN patches (extensions no bugs),
> > > > should i commit
> > > > some more or should we wait until the next release ?
> > > 
> > > What are those?
> > 
> > Wanted to finish the 2.4 OpenVPN project in the course which we did
> > discussed some time ago. So i thought about this order:
> > 
> > 1) Automatic cipher negotiation for RWs only (checkbox in advanced
> > section)
> 
> Isn't that something you would always want?
Might be a good opportunity for people with lot´s of clients and old
configuration files but an updated OpenVPN client. No new config
transfer is needed in that case but AES-GCM can nevertheless be used,
if too old (< 2.3.x), the before configured algorithms will be used.


> 
> > 2) tls-crypt for N2N only (checkbox in N2N main menu).
> > 3) LZ4 compression possibility for N2N and RW (menu with possiblity
> > for
> > none, lzo, lz4v2)
> 
> Yes, that should be a dropdown then instead of a checkbox.
Done already.

> 
> > 4) Clean up ovpnmain.cgi from mtu-discovery since there are some
> > old
> > code blocks left.
> 
> Okay, cool.
> 
> > There is more but to get the old list shorter for the first.
> 
> I guess it is best to start with the cleanup and then send in the
> other things
> one patch, or one patchset at a time.
This is how we do it.

Best,


Erik
  

Patch

diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
index 4bc3473..5cd19a0 100644
--- a/html/cgi-bin/ovpnmain.cgi
+++ b/html/cgi-bin/ovpnmain.cgi
@@ -1181,7 +1181,7 @@  SETTINGS_ERROR:
 	    delete $confighash{$cgiparams{'$key'}};
 	}
 
-	system ("/usr/local/bin/openvpnctrl -drrd $name");
+	system ("/usr/local/bin/openvpnctrl -drrd $name &>/dev/null");
     }
     while ($file = glob("${General::swroot}/ovpn/ca/*")) {
 	unlink $file;