AU: Sign delta payloads

AU: Sign delta payloads

- Change .proto to have explicit offset/length of signature. I was
  hoping the length could be kept out of the proto, but it needs to go
  in. The way we cheat and keep the signature in the file is to have a
  dummer install operation at the end that will cause old clients to
  write the signature data to nowhere.

- Change delta generator to take an optional private key, which if
  present will cause the payload to be signed

- Cleanup Omaha hash calculator, which should be renamed to SHA1 hash
  calculator, and allow export of the non-base64 encoded SHA1 result.

- Note: signatures are not yet checked. That will come in a future CL.

BUG=5662
TEST=unittests

[petkov, 2010-08-22 06:06:05]

LGTM w/ some nits and comments.

http://codereview.chromium.org/3132033/diff/1/2
File delta_diff_generator.cc (right):

http://codereview.chromium.org/3132033/diff/1/2#newcode886
delta_diff_generator.cc:886: //op->set_data_offset(next_blob_offset);
remove comment?

http://codereview.chromium.org/3132033/diff/1/6
File omaha_hash_calculator.cc (right):

http://codereview.chromium.org/3132033/diff/1/6#newcode20
omaha_hash_calculator.cc:20: LOG(ERROR) << "SHA1_Init failed";
You could use LOG_IF(ERROR, !valid_) << ...

http://codereview.chromium.org/3132033/diff/1/6#newcode53
omaha_hash_calculator.cc:53: success = (BIO_flush(b64) == 1);
This success assignment is unused as is. Also, it seems that the updated code
may return true even if the base64 encoding failed.

http://codereview.chromium.org/3132033/diff/1/6#newcode73
omaha_hash_calculator.cc:73: LOG(ERROR) << "SHA1_Final failed.";
Shouldn't you be returning an error status if SHA1_Final fails so that the
caller can also error out?

http://codereview.chromium.org/3132033/diff/1/7
File omaha_hash_calculator.h (right):

http://codereview.chromium.org/3132033/diff/1/7#newcode11
omaha_hash_calculator.h:11: #include "base/logging.h"
sort

http://codereview.chromium.org/3132033/diff/1/8
File payload_signer.cc (right):

http://codereview.chromium.org/3132033/diff/1/8#newcode48
payload_signer.cc:48: SplitString("/usr/bin/openssl rsautl -pkcs -sign -inkey x
-in x -out x",
Argh, this is a bit ugly. I guess you can't use StringPrintf because the paths
may contain spaces...

[adlr, 2010-08-24 00:37:43]

Thanks. fixed and will submit when tree opens

http://codereview.chromium.org/3132033/diff/1/2
File delta_diff_generator.cc (right):

http://codereview.chromium.org/3132033/diff/1/2#newcode886
delta_diff_generator.cc:886: //op->set_data_offset(next_blob_offset);
On 2010/08/22 06:06:05, petkov wrote:
> remove comment?
> 

Done.

http://codereview.chromium.org/3132033/diff/1/6
File omaha_hash_calculator.cc (right):

http://codereview.chromium.org/3132033/diff/1/6#newcode20
omaha_hash_calculator.cc:20: LOG(ERROR) << "SHA1_Init failed";
On 2010/08/22 06:06:05, petkov wrote:
> You could use LOG_IF(ERROR, !valid_) << ...
> 

Done.

http://codereview.chromium.org/3132033/diff/1/6#newcode53
omaha_hash_calculator.cc:53: success = (BIO_flush(b64) == 1);
On 2010/08/22 06:06:05, petkov wrote:
> This success assignment is unused as is. Also, it seems that the updated code
> may return true even if the base64 encoding failed.
> 

Good catch! I think it's good now

http://codereview.chromium.org/3132033/diff/1/6#newcode73
omaha_hash_calculator.cc:73: LOG(ERROR) << "SHA1_Final failed.";
On 2010/08/22 06:06:05, petkov wrote:
> Shouldn't you be returning an error status if SHA1_Final fails so that the
> caller can also error out?
> 

Done.

http://codereview.chromium.org/3132033/diff/1/7
File omaha_hash_calculator.h (right):

http://codereview.chromium.org/3132033/diff/1/7#newcode11
omaha_hash_calculator.h:11: #include "base/logging.h"
On 2010/08/22 06:06:05, petkov wrote:
> sort
> 

Done.

http://codereview.chromium.org/3132033/diff/1/8
File payload_signer.cc (right):

http://codereview.chromium.org/3132033/diff/1/8#newcode48
payload_signer.cc:48: SplitString("/usr/bin/openssl rsautl -pkcs -sign -inkey x
-in x -out x",
On 2010/08/22 06:06:05, petkov wrote:
> Argh, this is a bit ugly. I guess you can't use StringPrintf because the paths
> may contain spaces...
> 

Exactly. It would be awesome if you could initialize a vector with something
like {"/usr/bin/openssl", "rsautl", "-pkcs", ..., some_string, "-out",
other_string};