Temporary file access API
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]/messages/by-id/CA+TgmoYGjN_f=FCErX49bzjhNG+GoctY+a+XhNRWCVvDY8U74w@mail.gmail.com (and the posts
upthread), I've got another idea, so let's try to discuss it first.
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.)
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.
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.
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.)
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.
I think that's enough to start the discussion. Thanks for feedback in advance.
[1]: /messages/by-id/CA+TgmoYGjN_f=FCErX49bzjhNG+GoctY+a+XhNRWCVvDY8U74w@mail.gmail.com
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
Greetings,
* Antonin Houska (ah@cybertec.at) 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 asFileStream *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
Stephen Frost <sfrost@snowman.net> wrote:
* Antonin Houska (ah@cybertec.at) 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.
Thanks for your comments, the initial version is attached here.
The buffer size is not configurable yet. Besides the new API itself, the patch
series shows how it can be used to store logical replication data changes as
well as statistics. Further comments are welcome, thanks in advance.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
Attachments:
temp_file_api_v1.tgzapplication/gzipDownload+1-0
On Tue, Mar 8, 2022 at 6:12 AM Antonin Houska <ah@cybertec.at> wrote:
Thanks for your comments, the initial version is attached here.
I've been meaning to look at this thread for some time but have not
found enough time to do that until just now. And now I have
questions...
1. Supposing we accepted this, how widely do you think that we could
adopt it? I see that this patch set adapts a few places to use it and
that's nice, but I have a feeling there's a lot more places that are
making use of system calls directly, or through some other
abstraction, than just this. I'm not sure that we actually want to use
something like this everywhere, but what would be the rule for
deciding where to use it and where not to use
it? If the plan for this facility is just to adapt these two
particular places to use it, that doesn't feel like enough to be
worthwhile.
2. What about frontend code? Most frontend code won't examine data
files directly, but at least pg_controldata, pg_checksums, and
pg_resetwal are exceptions.
3. How would we extend this to support encryption? Add an extra
argument to BufFileStreamInit(V)FD passing down the encryption
details?
There are some smaller things about the patch with which I'm not 100%
comfortable, but I'd like to start by understanding the big picture.
Thanks,
--
Robert Haas
EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Mar 8, 2022 at 6:12 AM Antonin Houska <ah@cybertec.at> wrote:
Thanks for your comments, the initial version is attached here.
I've been meaning to look at this thread for some time but have not
found enough time to do that until just now. And now I have
questions...1. Supposing we accepted this, how widely do you think that we could
adopt it? I see that this patch set adapts a few places to use it and
that's nice, but I have a feeling there's a lot more places that are
making use of system calls directly, or through some other
abstraction, than just this. I'm not sure that we actually want to use
something like this everywhere, but what would be the rule for
deciding where to use it and where not to use
it? If the plan for this facility is just to adapt these two
particular places to use it, that doesn't feel like enough to be
worthwhile.
Admittedly I viewed the problem from the perspective of the TDE, so I haven't
spent much time looking for other opportunities. Now, with the stats collector
using shared memory, even one of the use cases implemented here no longer
exists. I need to do more research.
Do you think that the use of a system call is a problem itself (e.g. because
the code looks less simple if read/write is used somewhere and fread/fwrite
elsewhere; of course of read/write is mandatory in special cases like WAL,
heap pages, etc.) or is the problem that the system calls are used too
frequently? I suppose only the latter.
Anyway, I'm not sure there are *many* places where system calls are used too
frequently. Instead, the coding uses to be such that the information is first
assembled in memory and then written to file at once. So the value of the
(buffered) stream is that it makes the code simpler (eliminates the need to
prepare the data in memory). That's what I tried to do for reorderbuffer.c and
pgstat.c in my patch.
Related question is whether we should try to replace some uses of the libc
stream (FILE *) at some places. You seem to suggest that in [1]/messages/by-id/CA+TgmoYGjN_f=FCErX49bzjhNG+GoctY+a+XhNRWCVvDY8U74w@mail.gmail.com. One example
is snapmgr.c:ExportSnapshot(), if we also implement output formatting. Of
course there are places where (FILE *) cannot be replaced because, besides
regular file, the code needs to work with stdin/stdout in general. (Parsing of
configuration files falls into this category, but that doesn't matter because
bison-generated parser seems to implement buffering anyway.)
2. What about frontend code? Most frontend code won't examine data
files directly, but at least pg_controldata, pg_checksums, and
pg_resetwal are exceptions.
If the frequency of using system calls is the problem, then I wouldn't change
these because ControlFileData structure needs to be initialized in memory
anyway and then written at once. And pg_checksums reads whole blocks
anyway. I'll take a closer look.
3. How would we extend this to support encryption? Add an extra
argument to BufFileStreamInit(V)FD passing down the encryption
details?
Yes.
There are some smaller things about the patch with which I'm not 100%
comfortable, but I'd like to start by understanding the big picture.
Thanks!
[1]: /messages/by-id/CA+TgmoYGjN_f=FCErX49bzjhNG+GoctY+a+XhNRWCVvDY8U74w@mail.gmail.com
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
On Fri, Apr 8, 2022 at 4:54 AM Antonin Houska <ah@cybertec.at> wrote:
Do you think that the use of a system call is a problem itself (e.g. because
the code looks less simple if read/write is used somewhere and fread/fwrite
elsewhere; of course of read/write is mandatory in special cases like WAL,
heap pages, etc.) or is the problem that the system calls are used too
frequently? I suppose only the latter.
I'm not really super-interested in reducing the number of system
calls. It's not a dumb thing in which to be interested and I know that
for example Thomas Munro is very interested in it and has done a bunch
of work in that direction just to improve performance. But for me the
attraction of this is mostly whether it gets us closer to TDE.
And that's why I'm asking these questions about adopting it in
different places. I kind of thought that your previous patches needed
to encrypt, I don't know, 10 or 20 different kinds of files. So I was
surprised to see this patch touching the handling of only 2 kinds of
files. If we consolidate the handling of let's say 15 of 20 cases into
a single mechanism, we've really moved the needle in the right
direction -- but consolidating the handling of 2 of 20 cases, or
whatever the real numbers are, isn't very exciting.
--
Robert Haas
EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Apr 8, 2022 at 4:54 AM Antonin Houska <ah@cybertec.at> wrote:
Do you think that the use of a system call is a problem itself (e.g. because
the code looks less simple if read/write is used somewhere and fread/fwrite
elsewhere; of course of read/write is mandatory in special cases like WAL,
heap pages, etc.) or is the problem that the system calls are used too
frequently? I suppose only the latter.I'm not really super-interested in reducing the number of system
calls. It's not a dumb thing in which to be interested and I know that
for example Thomas Munro is very interested in it and has done a bunch
of work in that direction just to improve performance. But for me the
attraction of this is mostly whether it gets us closer to TDE.And that's why I'm asking these questions about adopting it in
different places. I kind of thought that your previous patches needed
to encrypt, I don't know, 10 or 20 different kinds of files. So I was
surprised to see this patch touching the handling of only 2 kinds of
files. If we consolidate the handling of let's say 15 of 20 cases into
a single mechanism, we've really moved the needle in the right
direction -- but consolidating the handling of 2 of 20 cases, or
whatever the real numbers are, isn't very exciting.
There are't really that many kinds of files to encrypt:
(And pg_stat/* files should be removed from the list.)
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
On Mon, Apr 11, 2022 at 4:05 AM Antonin Houska <ah@cybertec.at> wrote:
There are't really that many kinds of files to encrypt:
(And pg_stat/* files should be removed from the list.)
This kind of gets into some theoretical questions. Like, do we think
that it's an information leak if people can look at how many
transactions are committing and aborting in pg_xact_status? In theory
it could be, but I know it's been argued that that's too much of a
side channel. I'm not sure I believe that, but it's arguable.
Similarly, the argument that global/pg_internal.init doesn't contain
user data relies on the theory that the only table data that will make
its way into the file is for system catalogs. I guess that's not user
data *exactly* but ... are we sure that's how we want to roll here?
I really don't know how you can argue that pg_dynshmem/mmap.NNNNNNN
doesn't contain user data - surely it can.
--
Robert Haas
EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Apr 11, 2022 at 4:05 AM Antonin Houska <ah@cybertec.at> wrote:
There are't really that many kinds of files to encrypt:
(And pg_stat/* files should be removed from the list.)
This kind of gets into some theoretical questions. Like, do we think
that it's an information leak if people can look at how many
transactions are committing and aborting in pg_xact_status? In theory
it could be, but I know it's been argued that that's too much of a
side channel. I'm not sure I believe that, but it's arguable.
I was referring to the fact that the statistics are no longer stored in files:
Similarly, the argument that global/pg_internal.init doesn't contain
user data relies on the theory that the only table data that will make
its way into the file is for system catalogs. I guess that's not user
data *exactly* but ... are we sure that's how we want to roll here?
Yes, this is worth attention.
I really don't know how you can argue that pg_dynshmem/mmap.NNNNNNN
doesn't contain user data - surely it can.
Good point. Since postgres does not control writing into this file, it's a
special case though. (Maybe TDE will have to reject to start if
dynamic_shared_memory_type is set to mmap and the instance is encrypted.)
Thanks.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
On Tue, Apr 12, 2022 at 5:30 AM Antonin Houska <ah@cybertec.at> wrote:
Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Apr 11, 2022 at 4:05 AM Antonin Houska <ah@cybertec.at> wrote:
There are't really that many kinds of files to encrypt:
(And pg_stat/* files should be removed from the list.)
This kind of gets into some theoretical questions. Like, do we think
that it's an information leak if people can look at how many
transactions are committing and aborting in pg_xact_status? In theory
it could be, but I know it's been argued that that's too much of a
side channel. I'm not sure I believe that, but it's arguable.I was referring to the fact that the statistics are no longer stored in files:
Oh, yeah, I agree with that. I was saying that I'm not altogether a
believer in the idea that we need not encrypt SLRU files.
TBH, I think that no matter what we do there are going to be side
channel leaks, where some security researcher demonstrates that they
can infer something based on file length or file creation rate or
something. It seems inevitable. But the more stuff we don't encrypt,
the easier those attacks are going to be. On the other hand,
encrypting more stuff also makes the project harder. It's hard for me
to judge what the right balance is here. Maybe it's OK to exclude that
stuff for an initial version and just disclaim the heck out of it, but
I don't think that should be our long term strategy.
I really don't know how you can argue that pg_dynshmem/mmap.NNNNNNN
doesn't contain user data - surely it can.Good point. Since postgres does not control writing into this file, it's a
special case though. (Maybe TDE will have to reject to start if
dynamic_shared_memory_type is set to mmap and the instance is encrypted.)
Interesting point.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Mon, 11 Apr 2022 at 10:05, Antonin Houska <ah@cybertec.at> wrote:
Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Apr 8, 2022 at 4:54 AM Antonin Houska <ah@cybertec.at> wrote:
Do you think that the use of a system call is a problem itself (e.g. because
the code looks less simple if read/write is used somewhere and fread/fwrite
elsewhere; of course of read/write is mandatory in special cases like WAL,
heap pages, etc.) or is the problem that the system calls are used too
frequently? I suppose only the latter.I'm not really super-interested in reducing the number of system
calls. It's not a dumb thing in which to be interested and I know that
for example Thomas Munro is very interested in it and has done a bunch
of work in that direction just to improve performance. But for me the
attraction of this is mostly whether it gets us closer to TDE.And that's why I'm asking these questions about adopting it in
different places. I kind of thought that your previous patches needed
to encrypt, I don't know, 10 or 20 different kinds of files. So I was
surprised to see this patch touching the handling of only 2 kinds of
files. If we consolidate the handling of let's say 15 of 20 cases into
a single mechanism, we've really moved the needle in the right
direction -- but consolidating the handling of 2 of 20 cases, or
whatever the real numbers are, isn't very exciting.There are't really that many kinds of files to encrypt:
(And pg_stat/* files should be removed from the list.)
I was looking at that list of files that contain user data, and
noticed that all relation forks except the main fork were marked as
'does not contain user data'. To me this seems not necessarily true:
AMs do have access to forks for user data storage as well (without any
real issues or breaking the abstraction), and the init-fork is
expected to store user data (specifically in the form of unlogged
sequences). Shouldn't those forks thus also be encrypted-by-default,
or should we provide some other method to ensure that non-main forks
with user data are encrypted?
- Matthias
Matthias van de Meent <boekewurm+postgres@gmail.com> wrote:
On Mon, 11 Apr 2022 at 10:05, Antonin Houska <ah@cybertec.at> wrote:
There are't really that many kinds of files to encrypt:
(And pg_stat/* files should be removed from the list.)
I was looking at that list of files that contain user data, and
noticed that all relation forks except the main fork were marked as
'does not contain user data'. To me this seems not necessarily true:
AMs do have access to forks for user data storage as well (without any
real issues or breaking the abstraction), and the init-fork is
expected to store user data (specifically in the form of unlogged
sequences). Shouldn't those forks thus also be encrypted-by-default,
or should we provide some other method to ensure that non-main forks
with user data are encrypted?
Thanks. I've updated the wiki page (also included Robert's comments).
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
Greetings,
* Robert Haas (robertmhaas@gmail.com) wrote:
On Tue, Apr 12, 2022 at 5:30 AM Antonin Houska <ah@cybertec.at> wrote:
Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Apr 11, 2022 at 4:05 AM Antonin Houska <ah@cybertec.at> wrote:
There are't really that many kinds of files to encrypt:
(And pg_stat/* files should be removed from the list.)
This kind of gets into some theoretical questions. Like, do we think
that it's an information leak if people can look at how many
transactions are committing and aborting in pg_xact_status? In theory
it could be, but I know it's been argued that that's too much of a
side channel. I'm not sure I believe that, but it's arguable.I was referring to the fact that the statistics are no longer stored in files:
Oh, yeah, I agree with that. I was saying that I'm not altogether a
believer in the idea that we need not encrypt SLRU files.TBH, I think that no matter what we do there are going to be side
channel leaks, where some security researcher demonstrates that they
can infer something based on file length or file creation rate or
something. It seems inevitable. But the more stuff we don't encrypt,
the easier those attacks are going to be. On the other hand,
encrypting more stuff also makes the project harder. It's hard for me
to judge what the right balance is here. Maybe it's OK to exclude that
stuff for an initial version and just disclaim the heck out of it, but
I don't think that should be our long term strategy.
I agree that there's undoubtably going to be side-channel attacks, some
of which we may never be able to address, but we should be looking to
try and do as much as we can while also disclaiming that which we can't.
I'd lay this out in steps along these lines:
- Primary data is encrypted (and, ideally, optionally authenticated)
This is basically what the current state of things are with the
patches that have been posted in the past and would be a fantastic first
step that would get us past a lot of the auditors and others who are
unhappy that they can simply 'grep' a PG data directory for user data.
This also generally addresses off-line attacks, backups, etc. This also
puts us on a similar level as other vendors who offer TDE, as I
understand it.
- Encryption / authentication of metadata (SLRUs, maybe more)
This would be a great next step and would move us in the direction of
having a system that's resiliant against online storage-level attacks.
As for how to get there, I would think we'd start with coming up with a
way to at least have good checksums for SLRUs that are just generally
available and users are encouraged to enable, and then have that done in
a way that we could store an authentication tag instead in the future.
I really don't know how you can argue that pg_dynshmem/mmap.NNNNNNN
doesn't contain user data - surely it can.Good point. Since postgres does not control writing into this file, it's a
special case though. (Maybe TDE will have to reject to start if
dynamic_shared_memory_type is set to mmap and the instance is encrypted.)Interesting point.
Yeah, this is an interesting case and it really depends on what attack
vector is being considered and how the system is configured. I don't
think it's too much of an issue to say that you shouldn't use TDE with
mmap (I'm not huge on the idea of explicitly preventing someone from
doing it if they really want though..).
Thanks,
Stephen
On Mon, Apr 11, 2022 at 04:34:18PM -0400, Robert Haas wrote:
On Mon, Apr 11, 2022 at 4:05 AM Antonin Houska <ah@cybertec.at> wrote:
There are't really that many kinds of files to encrypt:
(And pg_stat/* files should be removed from the list.)
This kind of gets into some theoretical questions. Like, do we think
that it's an information leak if people can look at how many
transactions are committing and aborting in pg_xact_status? In theory
it could be, but I know it's been argued that that's too much of a
side channel. I'm not sure I believe that, but it's arguable.
Similarly, the argument that global/pg_internal.init doesn't contain
user data relies on the theory that the only table data that will make
its way into the file is for system catalogs. I guess that's not user
data *exactly* but ... are we sure that's how we want to roll here?
I don't think we want to be encrypting pg_xact/, so they can get the
transaction commit rate from there.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Indecision is a decision. Inaction is an action. Mark Batterson
On Wed, Apr 13, 2022 at 6:25 PM Bruce Momjian <bruce@momjian.us> wrote:
I don't think we want to be encrypting pg_xact/, so they can get the
transaction commit rate from there.
I think it would be a good idea to eventually encrypt SLRU data,
including pg_xact. Maybe not in the first version.
--
Robert Haas
EDB: http://www.enterprisedb.com
Greetings,
On Wed, Apr 13, 2022 at 18:54 Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Apr 13, 2022 at 6:25 PM Bruce Momjian <bruce@momjian.us> wrote:
I don't think we want to be encrypting pg_xact/, so they can get the
transaction commit rate from there.I think it would be a good idea to eventually encrypt SLRU data,
including pg_xact. Maybe not in the first version.
I agree with Robert, on both parts.
I do think getting checksums for that data may be the first sensible step.
Thanks,
Stephen
Show quoted text
On Wed, Apr 13, 2022 at 06:54:13PM -0400, Robert Haas wrote:
On Wed, Apr 13, 2022 at 6:25 PM Bruce Momjian <bruce@momjian.us> wrote:
I don't think we want to be encrypting pg_xact/, so they can get the
transaction commit rate from there.I think it would be a good idea to eventually encrypt SLRU data,
including pg_xact. Maybe not in the first version.
I assume that would be very hard or slow, and of limited value.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Indecision is a decision. Inaction is an action. Mark Batterson
Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Apr 12, 2022 at 5:30 AM Antonin Houska <ah@cybertec.at> wrote:
Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Apr 11, 2022 at 4:05 AM Antonin Houska <ah@cybertec.at> wrote:
There are't really that many kinds of files to encrypt:
(And pg_stat/* files should be removed from the list.)
This kind of gets into some theoretical questions. Like, do we think
that it's an information leak if people can look at how many
transactions are committing and aborting in pg_xact_status? In theory
it could be, but I know it's been argued that that's too much of a
side channel. I'm not sure I believe that, but it's arguable.I was referring to the fact that the statistics are no longer stored in files:
Oh, yeah, I agree with that.
I see now that the statistics are yet saved to a file on server shutdown. I've
updated the wiki page.
Attached is a new version of the patch, to evaluate what the API use in the
backend could look like. I haven't touched places where the file is accessed
in a non-trivial way, e.g. lseek() / fseek() or pg_pwrite() / pg_pread() is
called.
Another use case might be copying one file to another via a buffer. Something
like
BufFileCopy(int dstfd, int srcfd, int bufsize)
The obvious call site would be in copydir.c:copy_file(), but I think there are
a few more in the server code.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
Attachments:
Antonin Houska <ah@cybertec.at> wrote:
Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Apr 12, 2022 at 5:30 AM Antonin Houska <ah@cybertec.at> wrote:
Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Apr 11, 2022 at 4:05 AM Antonin Houska <ah@cybertec.at> wrote:
There are't really that many kinds of files to encrypt:
(And pg_stat/* files should be removed from the list.)
This kind of gets into some theoretical questions. Like, do we think
that it's an information leak if people can look at how many
transactions are committing and aborting in pg_xact_status? In theory
it could be, but I know it's been argued that that's too much of a
side channel. I'm not sure I believe that, but it's arguable.I was referring to the fact that the statistics are no longer stored in files:
Oh, yeah, I agree with that.
I see now that the statistics are yet saved to a file on server shutdown. I've
updated the wiki page.Attached is a new version of the patch, to evaluate what the API use in the
backend could look like. I haven't touched places where the file is accessed
in a non-trivial way, e.g. lseek() / fseek() or pg_pwrite() / pg_pread() is
called.
Rebased patch set is attached here, which applies to the current master.
(A few more opportunities for the new API implemented here.)
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
Attachments:
On 04.07.22 18:35, Antonin Houska wrote:
Attached is a new version of the patch, to evaluate what the API use in the
backend could look like. I haven't touched places where the file is accessed
in a non-trivial way, e.g. lseek() / fseek() or pg_pwrite() / pg_pread() is
called.Rebased patch set is attached here, which applies to the current master.
(A few more opportunities for the new API implemented here.)
I don't understand what this patch set is supposed to do. AFAICT, the
thread originally forked off from a TDE discussion and, considering the
thread subject, was possibly once an attempt to refactor temporary file
access to make integrating encryption easier? The discussion then
talked about things like saving on system calls and excising all
OS-level file access API use, which I found confusing, and the thread
then just became a general TDE-related mini-discussion.
The patches at hand extend some internal file access APIs and then
sprinkle them around various places in the backend code, but there is no
explanation why or how this is better. I don't see any real structural
savings one might expect from a refactoring patch. No information has
been presented how this might help encryption.
I also suspect that changing around the use of various file access APIs
needs to be carefully evaluated in each individual case. Various code
has subtle expectations about atomic write behavior, syncing, flushing,
error recovery, etc. I don't know if this has been considered here.
