Re: [PATCH 3/8] Add support for a generic wal reading facility dubbed XLogReader

On Tuesday, October 30, 2012 03:20:03 PM Alvaro Herrera wrote:
> Andres Freund escribió:
> > On Monday, October 29, 2012 08:58:53 PM Alvaro Herrera wrote:
> > > Heikki Linnakangas escribió:
> > >
> > > Andres commented elsewhere about reading xlog records, processing them
> > > as they came in, and do a running CRC while we're still reading it. I
> > > think this is a mistake; we shouldn't do anything with a record until
> > > the CRC has been verified. Otherwise we risk reading arbitrarily
> > > corrupt data.
> >
> > Uhm. xlog.c does just the same. It reads the header and if it looks valid
> > it uses its length information to read the full record and only computes
> > the CRC at the end.
>
> Uh. Correct.
>
> Am I the only one who finds this rather bizarre? Maybe this was okay
> when xlog data would only come from WAL files stored in the data
> directory at recovery, but if we're now receiving these from a remote
> sender over the network I wonder if we should be protecting against
> malicious senders. (This is not related to this patch anyway.)

How should this work otherwise? The CRC is over the whole data so we obviously
need to read the whole data to compute the CRC? Would you prefer protecting
the header with a separate CRC?
You can't use a CRC against malicous users anyway, its not cryptographically
secure in any meaning of the word, its trivial to generate different content
resulting in the same CRC. The biggest user of the CRC checking code we have
is making sure were not reading beyond the end of the WAL...

Greetings,

Andres
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services