Backup Central

On Friday 15 June 2007 15:00, Josh Fisher wrote:
Kern Sibbald wrote:
Hello,

I am now working on bug #807, where decrypting files gets signature digest
errors on each file restored. As far as I can tell, these are *false*
error
messages, most likely due to the fact that Microsoft BackupWrite() does
not
restore exactly the same bits as BackupRead() gave.

Anyway, in looking at the encryption code in detail, which I have not
totally
finished, I come to realize that there are some important problems that
need
fixing.

1. The code is rather inefficient from several stand points -- see items
below.

2. It does a number of malloc() and free()s for each file backed up
which is something that Bacula does not normally do. Over time,
I will fix this.

3. In computing the signature digest, during a restore, it first writes
the
file to disk, then re-reads the whole file computing the digest that
it
compares to the original. This is very inefficient, and I am planning
to change it in version 2.2.0 to compute the signature digest as it
reads the data from the SD, and thus to skip reading the file after it
is written.

Upside -- *much* more efficient; the current signature errors
will be eliminated.

Downside -- it is probably better to compute the signature from the
actual file written in case something goes wrong in writing it.
However,
this doesn't work.



I don't think there is a downside. If something goes wrong with writing
the file, then the file is corrupt and it might as well get a digest
error on restore as a checksum error, or worse, no error at all.

Yes, good point. Thanks.

Kern


4. The current signature digest algorithm (as far as I can tell) is not
saved
on the Volume. The signature digest algorithm is compiled into the
code,
and it is a different algorithm depending on whether or not you have
only SHA1 or you have SHA2 algorithms available. This is *very* bad.
It means that if you rebuild your Bacula FD, it may not be able to
compute
a signature digest for a restore in the same way it did for the backup.

5. While computing the signature digest for a restore as noted above, the
code is also computing a digest, which is used for the Bacula file
hash
code -- i.e. the old MD5 or SHA1. This means that two digests are
during the restore, which is unnecessary (as mentioned in the author's
comments in the files).

6. There are a good number of places where the error code returned is
simply ignored.

7. The code for the most part causes any OpenSSL errors to be lost.
This is now fixed in version 2.1.14

I'm proposing the following:

Item 2. Eliminate the unnecessary malloc() calls over time (some gone in
version 2.1.14

Item 3. Do the signature digest as the data is received from the SD
eliminating the need to re-read the file. I hope to do this before
version
2.2.0 is released.

Items 4-5. Compute only a single digest and use it for the signature
record
as well as the hash code that goes into the catalog. I doubt this can
be
done for version 2.2.0 without delaying it another month, because this
is not so simple (new streams for the Volume) and unfortunately, not
100% upward transparent -- you will need to modify your Dir conf file
to
have the same level of signature protection as today.

Item 6. Fix over time.

Item 7. Already fixed.

If anyone has any comments or objections, particularly on item 3, now is
the
time to speak up.

Best regards,

Kern

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bacula-users mailing list
Bacula-users < at > li...
https://lists.sourceforge.net/lists/listinfo/bacula-users


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bacula-devel mailing list
Bacula-devel < at > li...
https://lists.sourceforge.net/lists/listinfo/bacula-devel

Small request: can we start to clip the extra text in responses? It's
getting very difficult to find the actual content in these
discussions...

Downside -- it is probably better to compute the signature from
the
actual file written in case something goes wrong in writing it.
However,
this doesn't work.
=20
I don't think there is a downside. If something goes wrong with
writing
the file, then the file is corrupt and it might as well get a digest
error on restore as a checksum error, or worse, no error at all.

On Fri, 15 Jun 2007 14:18:48 +0200, Kern Sibbald said:

Hello,

I am now working on bug #807, where decrypting files gets signature digest
errors on each file restored. As far as I can tell, these are *false* error
messages, most likely due to the fact that Microsoft BackupWrite() does not
restore exactly the same bits as BackupRead() gave.

Anyway, in looking at the encryption code in detail, which I have not totally
finished, I come to realize that there are some important problems that need
fixing.

...

3. In computing the signature digest, during a restore, it first writes the
file to disk, then re-reads the whole file computing the digest that it
compares to the original. This is very inefficient, and I am planning
to change it in version 2.2.0 to compute the signature digest as it
reads the data from the SD, and thus to skip reading the file after it
is written.

Upside -- *much* more efficient; the current signature errors
will be eliminated.

Downside -- it is probably better to compute the signature from the
actual file written in case something goes wrong in writing it. However,
this doesn't work.

I would be wary about changing this without knowing exactly which bits are
different between BackupWrite and BackupRead. It really depends on the
purpose of the signature -- some users might expect it to be a check that the
restored filesystem is the same as saved one.

__Martin

Attachments: PGP.sig

On Jun 15, 2007, at 5:18 AM, Kern Sibbald wrote:

Hello,

I am now working on bug #807, where decrypting files gets signature
digest
errors on each file restored. As far as I can tell, these are
*false* error
messages, most likely due to the fact that Microsoft BackupWrite()
does not
restore exactly the same bits as BackupRead() gave.

Anyway, in looking at the encryption code in detail, which I have
not totally
finished, I come to realize that there are some important problems
that need
fixing.

Thank you for spending the time on this code.

1. The code is rather inefficient from several stand points -- see
items
below.

2. It does a number of malloc() and free()s for each file backed up
which is something that Bacula does not normally do. Over time,
I will fix this.

Given performance profiling, is this a demonstrable issue? Compared
to the cost of the public-key operations, calling the OS' malloc() is
a drop in the bucket, and even if the malloc() calls are somehow
reduced/removed, OpenSSL is still going to make extensive dynamic
allocations.

Moreover, by encapsulating data structures (rather than trying to re-
use buffers from one big loop), I was able to avoid significant
implementation complexity.

Ultimately my performance profiling has shown that the greatest gains
would be made in adding support for HMAC validation as an alternative
to per-file RSA signatures.

4. The current signature digest algorithm (as far as I can tell) is
not saved
on the Volume. The signature digest algorithm is compiled into
the code,
and it is a different algorithm depending on whether or not you
have
only SHA1 or you have SHA2 algorithms available. This is *very*
bad.
It means that if you rebuild your Bacula FD, it may not be able
to compute
a signature digest for a restore in the same way it did for the
backup.

This isn't the case -- both the signature and digest algorithm are
stored in the SignatureData->SignerInfo record and used on restoration.
At no point does any part of the code make any assumptions regarding
the algorithms of on-disk data -- I was very careful with this.

See crypto_sign_add_signer()

6. There are a good number of places where the error code returned is
simply ignored.

OpenSSL errors?

-landonf