Subject: Re: Temporary file access API

From: Stephen Frost <[email-masked]>
Date: Tue, 1 Mar 2022 09:34:31 -0500

Greetings,

* Antonin Houska ([email-masked]) wrote:
> Here I'm starting a new thread to discuss a topic that's related to the
> Transparent Data Encryption (TDE), but could be useful even without that.  The
> problem has been addressed somehow in the Cybertec TDE fork, and I can post
> the code here if it helps. However, after reading [1] (and the posts
> upthread), I've got another idea, so let's try to discuss it first.

Yeah, I tend to agree that it makes sense to discuss the general idea of
doing buffered I/O for the temporary files that are being created today
with things like fwrite and have that be independent of encryption.  As
mentioned on that thread, doing our own buffering and having file
accesses done the same way throughout the system is just generally a
good thing and focusing on that will certainly help with any TDE effort
that may or may not happen later.

> It makes sense to me if we first implement the buffering (i.e. writing/reading
> certain amount of data at a time) and make the related functions aware of
> encryption later: as long as we use a block cipher, we also need to read/write
> (suitably sized) chunks rather than individual bytes (or arbitrary amounts of
> data). (In theory, someone might need encryption but reject buffering, but I'm
> not sure if this is a realistic use case.)

Agreed.

> For the buffering, I imagine a "file stream" object that user creates on the
> top of a file descriptor, such as
> 
>     FileStream  *FileStreamCreate(File file, int buffer_size)
> 
>     or
> 
>     FileStream  *FileStreamCreateFD(int fd, int buffer_size)
> 
> and uses functions like
> 
>     int FileStreamWrite(FileStream *stream, char *buffer, int amount)
> 
>     and
> 
>     int FileStreamRead(FileStream *stream, char *buffer, int amount)
> 
> to write and read data respectively.

Yeah, something along these lines was also what I had in mind, in
particular as it would mean fewer changes to places like reorderbuffer.c
(or, at least, very clear changes).

> Besides functions to close the streams explicitly (e.g. FileStreamClose() /
> FileStreamFDClose()), we'd need to ensure automatic closing where that happens
> to the file. For example, if OpenTemporaryFile() was used to obtain the file
> descriptor, the user expects that the file will be closed and deleted on
> transaction boundary, so the corresponding stream should be freed
> automatically as well.

Sure.  We do have some places that want to write using offsets and we'll
want to support that also, I'd think.

> To avoid code duplication, buffile.c should use these streams internally as
> well, as it also performs buffering. (Here we'd also need functions to change
> reading/writing position.)

Yeah... though we should really go through and make sure that we
understand the use-cases for each of the places that decided to use
their own code rather than what already existed, to the extent that we
can figure that out, and make sure that we're solving those cases and
not writing a bunch of new code that won't end up getting used.  We
really want to be sure that all writes are going through these code
paths and make sure there aren't any reasons for them not to be used.

> Once we implement the encryption, we might need add an argument to the
> FileStreamCreate...() functions that helps to generate an unique IV, but the
> ...Read() / ...Write() functions would stay intact. And possibly one more
> argument to specify the kind of cipher, in case we support more than one.

As long as we're only needing to pass these into the Create function,
that seems reasonable to me.  I wouldn't want to go through the effort
of this and then still have to change every single place we read/write
using this system.

Thanks,

Stephen