Alberto Gonzalez Iniesta | 5 Feb 09:45 2003

Trimmed down permissions for generated keys

Hi James et al!

Intro
-----
openvpn creates pre-shared secret files, for latter use in static key
encryption mode (non-TLS), with the --genkey option

The minor/anecdotal glitch
--------------------------

The permissions for the created file may be/seem to be excessive (0700)
Pointed out by Herbert Xu <herbert <at> gondor.apana.org.au> [1]

The patch
---------

--- openvpn-1.3.2.orig/crypto.c
+++ openvpn-1.3.2/crypto.c
 <at>  <at>  -968,7 +968,7  <at>  <at> 
   struct buffer out = alloc_buf_gc (512);

   /* open key file */
-  fd = open (filename, O_CREAT | O_TRUNC | O_WRONLY, S_IRWXU);
+  fd = open (filename, O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | S_IWUSR);
   if (fd == -1)
     msg (M_ERR, "Cannot open shared secret file %s for write", filename);

Let me know if you like it/agree, James. Thanks,

Alberto
(Continue reading)

James Yonan | 6 Feb 06:12 2003
Picon

Re: Trimmed down permissions for generated keys

Hi Alberto,

Yes, I agree.  The owner-execute permission is unnecessary.  I will add patch
to next release.

James

Alberto Gonzalez Iniesta <agi <at> inittab.org> said:

> Hi James et al!
> 
> Intro
> -----
> openvpn creates pre-shared secret files, for latter use in static key
> encryption mode (non-TLS), with the --genkey option
> 
> The minor/anecdotal glitch
> --------------------------
> 
> The permissions for the created file may be/seem to be excessive (0700)
> Pointed out by Herbert Xu <herbert <at> gondor.apana.org.au> [1]
> 
> The patch
> ---------
> 
> --- openvpn-1.3.2.orig/crypto.c
> +++ openvpn-1.3.2/crypto.c
>  <at>  <at>  -968,7 +968,7  <at>  <at> 
>    struct buffer out = alloc_buf_gc (512);
> 
(Continue reading)

Alberto Gonzalez Iniesta | 6 Feb 12:14 2003

Opened file descriptors in script calls

Hi,

I got another bug report on Debian's openvpn package. It states that
openvpn's file descriptors are kept open while scripts are called. It
claims they should be closed. My C knowledge is far from make a patch
for this one :) And maybe you don't agree with this (James?).

You can see the report here:

http://bugs.debian.org/179551

Thanks.

--

-- 
Alberto Gonzalez Iniesta       | They that give up essential liberty
agi <at> (agi.as|debian.org)        | to obtain a little temporary safety
Encrypted mail preferred       | deserve neither liberty nor safety.

Key fingerprint = 9782 04E7 2B75 405C F5E9  0C81 C514 AF8E 4BA4 01C3

-------------------------------------------------------
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www.vasoftware.com
James Yonan | 8 Feb 04:47 2003
Picon

Re: Opened file descriptors in script calls

Alberto,

Yes, I agree.  The child process that executes a script doesn't need those
file descriptors, so they can be closed.  Since openvpn uses the system()
function to run scripts, and because the system() function doesn't close any
file descriptors on its own, it would be necessary to write an alternate
version of the function (unless it already exists somewhere) which closes the
file descriptors between the fork and the execve calls.

James

Alberto Gonzalez Iniesta <agi <at> inittab.org> said:

> Hi,
> 
> I got another bug report on Debian's openvpn package. It states that
> openvpn's file descriptors are kept open while scripts are called. It
> claims they should be closed. My C knowledge is far from make a patch
> for this one :) And maybe you don't agree with this (James?).
> 
> You can see the report here:
> 
> http://bugs.debian.org/179551
> 
> Thanks.
> 
> -- 
> Alberto Gonzalez Iniesta       | They that give up essential liberty
> agi <at> (agi.as|debian.org)        | to obtain a little temporary safety
> Encrypted mail preferred       | deserve neither liberty nor safety.
(Continue reading)

Aaron Sethman | 10 Feb 02:01 2003

Re: Opened file descriptors in script calls


Here is a simple little replacement for system() that does close file
descriptors.  The main issue with it is though, it ends up picking an
arbitrary number of fds to close.  I picked closing 0 to 99.

Aaron

int s_system(const char *string)
{
	pid_t pid;
	int x;
 	pid = fork()

 	if(pid == -1)
 	{
         	return -1;
 	}

 	if(pid > 0)
 	{
 	 	int status;
         	wait(&status);
         	return(status);
        }

        for(x = 0; x < 100; x++)
 	{
 		close(x);
 	}
        execlp("/bin/sh", "-c", string);
(Continue reading)

Alberto Gonzalez Iniesta | 10 Feb 11:34 2003

Re: Opened file descriptors in script calls

On Sun, Feb 09, 2003 at 08:01:12PM -0500, Aaron Sethman wrote:
> 
> Here is a simple little replacement for system() that does close file
> descriptors.  The main issue with it is though, it ends up picking an
> arbitrary number of fds to close.  I picked closing 0 to 99.
> 
> Aaron
> 
> 
> int s_system(const char *string)
> {
> 	pid_t pid;
> 	int x;
>  	pid = fork()
> 
>  	if(pid == -1)
>  	{
>          	return -1;
>  	}
> 
>  	if(pid > 0)
>  	{
>  	 	int status;
>          	wait(&status);
>          	return(status);
>         }
> 
>         for(x = 0; x < 100; x++)
>  	{
>  		close(x);
(Continue reading)

Aaron Sethman | 10 Feb 21:51 2003

Re: Opened file descriptors in script calls

On Mon, 10 Feb 2003, Alberto Gonzalez Iniesta wrote:

>
> Hi,
>
> Again, I'm no C hacker, but I think this should be better:
>
> for(x = 3; x < 100; x++)
>
> Since the first 3 fds (stdin, stdout and stderr) should be kept open.
>
Wasn't sure if stdin, stdout and stderr needed to be left open or not in
this case.  Obviously this is an easy fix :)

Regards,

Aaron

-------------------------------------------------------
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www.vasoftware.com
James Yonan | 11 Feb 18:11 2003
Picon

Re: Opened file descriptors in script calls

Aaron Sethman <androsyn <at> ratbox.org> said:

> On Mon, 10 Feb 2003, Alberto Gonzalez Iniesta wrote:
> 
> >
> > Hi,
> >
> > Again, I'm no C hacker, but I think this should be better:
> >
> > for(x = 3; x < 100; x++)
> >
> > Since the first 3 fds (stdin, stdout and stderr) should be kept open.
> >
> Wasn't sure if stdin, stdout and stderr needed to be left open or not in
> this case.  Obviously this is an easy fix :)

Actually, I was just thinking what a pain to have to reimplement system() just
to close a few fds.  There must be a better way, and as it turns out, there is:

/* Set a file descriptor to not be passed across execs */
void
set_cloexec (int fd)
{
  if (fcntl (fd, F_SETFD, FD_CLOEXEC) < 0)
    msg (M_ERR, "Set file descriptor to FD_CLOEXEC failed");
}

Just set the FD_CLOEXEC flag on the fd and it won't be passed across the exec
that runs the shell.

(Continue reading)

Aaron Sethman | 11 Feb 18:18 2003

Re: Opened file descriptors in script calls


On Tue, 11 Feb 2003, James Yonan wrote:
> /* Set a file descriptor to not be passed across execs */
> void
> set_cloexec (int fd)
> {
>   if (fcntl (fd, F_SETFD, FD_CLOEXEC) < 0)
>     msg (M_ERR, "Set file descriptor to FD_CLOEXEC failed");
> }
>
> Just set the FD_CLOEXEC flag on the fd and it won't be passed across the exec
> that runs the shell.

That works even better then :)

Regards,

Aaron

-------------------------------------------------------
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www.vasoftware.com
Wolfgang Ocker | 10 Feb 11:45 2003
Picon

Re: Opened file descriptors in script calls

On Mon, 2003-02-10 at 02:01, Aaron Sethman wrote:
> Here is a simple little replacement for system() that does close file
> descriptors.  The main issue with it is though, it ends up picking an
> arbitrary number of fds to close.  I picked closing 0 to 99.

You can use getdtablesize() to determine the maximum number of open files
available to the process.

Wolfgang

-------------------------------------------------------
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www.vasoftware.com

Gmane