[m-rev.] for review: Implement secure temporary file creation for .NET.

Sebastian Godelet sebastian.godelet at outlook.com
Tue May 10 13:01:45 AEST 2016


Hi Paul,

...
> Maybe we should make open(..., O_CREAT | O_EXCL) / mkdir and their
> equivilents in Java C# and Erlang the primative operation, and write the logic
> that generates random names and tries them in Mercury, then we don't
> need to rely on any support for mkstemp etc.  What do you think?

This is an excellent idea :)

> 
> All we need from each target language is open and mkdir with O_CREAT |
> O_EXCL behavior that also lets us set the file's permissions atomically.  We
> could still do it with less but it's be insecure.

As we have almost covered every OS/backend combination (sans Erlang), we shouldn't go for insecure code.
I was even thinking about a way to allow secure Java temporary file creation even when we only require Java 6
See how it is done in another place in io.m:

        if (ok && ML_access_types_includes_execute(AccessTypes)) {
            // File.canExecute() was added in Java 1.6 but we only require
            // Java 1.5.
            try {
                java.lang.reflect.Method canExecute =
                    file.getClass().getMethod(""canExecute"");
                ok = (Boolean) canExecute.invoke(file);
            }

The same mechanism could be used to try to load the new Java 7 classes (via the Class.forName thingy).
This could be done at initialisation time to not slow down things too much, IMHO better than having that code outside the library.

> 
> > >
> > > Although mkstemps is non-standard, it's still a realistic
> > > alternative since we already support systems without mkstemp.  So
> > > we'd continue to support two
> > > implementations: generate names ourselves and use open / mkstemps. -
> > > I'll make this a separate proposal.
> >
> > For the make_temp_file, should there be always two versions? : One that
> creates a temporary file, and one that creates and opens a tempory file? The
> implementations of those two could and should be different, currently we
> create a file and then close the stream, but for example on Windows it is
> much more efficient to open a stream temporary and let the system delete it
> when the file handle is closed. Like the O_TMPFILE - Linux only? - flag I
> mentioned earlier or the FILE_ATTRIBUTE_TEMPORARY the
> FILE_FLAG_DELETE_ON_CLOSE Windows equivalent (although we use
> _wopen at the moment instead of CreateFile).
> 
> I would like to see that yes.  Returning the newly created and opened file is
> ideal.  What I was referring to however was how on the C backend
> conditional compilation means that mkstemp is used when it's available and a
> loop with a call to open is used otherwise.

I think we should - besides having foreign function calls for "open" and friends also an abstract not exported type
:- type native_filestream.

Which in C# would be a small record which is either a FileStream (which is returned by File.Create, or a wrapped file descriptor on Mono),
and in C stand for a file descriptor (returned by open and mkstemp(s)) and so on.
This way we could implement temporary file creation once for each backend, and use it for both use-cases:
Create and commit a temporary file to the file system (so open it and call close until that succeeds),
And a separate procedure which allows to create and leave a temporary file stream open (not sure how we can convert from a native file stream to a Mercury one).
Doing this in one call offers the added benefit of allowing temporary files to avoid the cache manager under Windows and Linux including automatic deletion on file stream closure.

% C# FileStream
% C int (file descriptor)
% Java ?
:- type native_filestream.

% do_open_temp_file(Dir, Prefix, Suffix, FilePath, Stream, Success, ErrMsg, !IO)
% mkstemps / open / FileStream.Create with permissions, etc.
:- proc do_open_temp_file(string::in, string::in, string::in, string::out, native_filestream::out, bool::out, string::out, io::di, io::uo) is det.

% do_close_temp_file(Stream, Success, ErrMsg, !IO)
% This will be the while loop with the EINTR logic or a simple FileStream.Close call, etc.
:- proc do_open_temp_file(native_filestream::in, bool::out, string::out, io::di, io::uo) is det.

% do_create_temp_dir(Dir, Prefix, Suffix, DirPath, Success, ErrMsg, !IO)
% the extracted existing directory logic we have at the moment
:- proc do_create_temp_dir(string::in, string::in, string::in, string::out, bool::out, string::out, io::di, io::uo) is det.

> 
> > I would also volunteer to implement the mkdtemp(s) implementation for
> Windows if you don't mind. It would be very similar to the .NET/Windows
> make_temp_directory implementation (i.e. use the CreateDirectoryW Win32
> call, etc.). After all that is what the .NET API is wrapping in the first place.
> 
> Sounds good but let's wait until we decide what the best general approach is
> first.

OK

> 
> 
> --
> Paul Bone

Cheers,

Sebastian.


More information about the reviews mailing list