AU: Switch to 2048 bit RSA keys; Pad SHA256 hashes appropriately.

AU: Switch to 2048 bit RSA keys; Pad SHA256 hashes appropriately.

Manually pad hashes according to how SHA256 hashes should be padded
for use in RSA 2048 bit encryption.

Also, remove the public key from the repository, as it's generated by
scons.

In an upcoming CL, I will test via an actual update.

BUG=chromium-os:13341
TEST=unittests

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=bdfaaf0

[petkov, 2011-03-29 22:05:11]

I think sosa's AU harness might be using the public key.

[sosa, 2011-03-29 22:47:45]

I do :(

On Tue, Mar 29, 2011 at 3:05 PM,  <petkov@chromium.org> wrote:
> I think sosa's AU harness might be using the public key.
>
>
> http://codereview.chromium.org/6771024/
>

[sosa, 2011-03-29 22:48:07]

s/I/it

On Tue, Mar 29, 2011 at 3:47 PM, Chris Sosa <sosa@chromium.org> wrote:
> I do :(
>
> On Tue, Mar 29, 2011 at 3:05 PM,  <petkov@chromium.org> wrote:
>> I think sosa's AU harness might be using the public key.
>>
>>
>> http://codereview.chromium.org/6771024/
>>
>

[adlr, 2011-03-29 22:59:38]

Is it too much of a hassle to generate it? It would be nice not to have a
build product checked into version control.

/usr/bin/openssl rsa -in *private_key.pem* -pubout -out *public_key.pem*

On Tue, Mar 29, 2011 at 3:48 PM, Chris Sosa <sosa@chromium.org> wrote:

> s/I/it
>
> On Tue, Mar 29, 2011 at 3:47 PM, Chris Sosa <sosa@chromium.org> wrote:
> > I do :(
> >
> > On Tue, Mar 29, 2011 at 3:05 PM,  <petkov@chromium.org> wrote:
> >> I think sosa's AU harness might be using the public key.
> >>
> >>
> >> http://codereview.chromium.org/6771024/
> >>
> >
>

[gauravsh, 2011-03-29 23:04:05]

So, overall this looks fine but are you ok with keeping the key size fixed to
2048 bits? Parts of the code assume that, so it'd probably be nice to throw in
some extra bit of checks that the key size is always 2048 bits. We can avoid
problems like Sosa's test using a 1024-bit key then (although I guess with the
new padding that should be ok).

http://codereview.chromium.org/6771024/diff/9/payload_signer.h
File payload_signer.h (right):

http://codereview.chromium.org/6771024/diff/9/payload_signer.h#newcode70
payload_signer.h:70: // Pads a SHA256 hash so that it may be encrypted/signed
with RSA2048.
just a add a comment that this pads using the PKCS#1 v1.5 scheme. (there are
multiple types of padding schemes)

http://codereview.chromium.org/6771024/diff/9/payload_signer.h#newcode72
payload_signer.h:72: static bool PadRSA2048SHA256Hash(std::vector<char>* hash);
comment on what |hash| is? Also that in-place modifies the hash.

[gauravsh, 2011-03-29 23:04:50]

http://codereview.chromium.org/6771024/diff/9/payload_signer.cc
File payload_signer.cc (right):

http://codereview.chromium.org/6771024/diff/9/payload_signer.cc#newcode139
payload_signer.cc:139: vector<char> padded_hash(hash);
should you check here that this is indeed the size that you expect?

[sosa, 2011-03-29 23:15:22]

Very well.

http://codereview.chromium.org/6775011/

On Tue, Mar 29, 2011 at 4:04 PM,  <gauravsh@chromium.org> wrote:
>
> http://codereview.chromium.org/6771024/diff/9/payload_signer.cc
> File payload_signer.cc (right):
>
> http://codereview.chromium.org/6771024/diff/9/payload_signer.cc#newcode139
> payload_signer.cc:139: vector<char> padded_hash(hash);
> should you check here that this is indeed the size that you expect?
>
> http://codereview.chromium.org/6771024/
>

[adlr, 2011-03-29 23:24:57]

Awesome, thanks, Chris!

-andrew

On Tue, Mar 29, 2011 at 4:15 PM, Chris Sosa <sosa@chromium.org> wrote:

> Very well.
>
> http://codereview.chromium.org/6775011/
>
> On Tue, Mar 29, 2011 at 4:04 PM,  <gauravsh@chromium.org> wrote:
> >
> > http://codereview.chromium.org/6771024/diff/9/payload_signer.cc
> > File payload_signer.cc (right):
> >
> >
> http://codereview.chromium.org/6771024/diff/9/payload_signer.cc#newcode139
> > payload_signer.cc:139: vector<char> padded_hash(hash);
> > should you check here that this is indeed the size that you expect?
> >
> > http://codereview.chromium.org/6771024/
> >
>

[sosa, 2011-03-29 23:43:06]

Pushed my CL.  Ctest no longer has the dependency :)

On Tue, Mar 29, 2011 at 4:24 PM, Andrew de los Reyes <adlr@chromium.org> wrote:
> Awesome, thanks, Chris!
> -andrew
>
> On Tue, Mar 29, 2011 at 4:15 PM, Chris Sosa <sosa@chromium.org> wrote:
>>
>> Very well.
>>
>> http://codereview.chromium.org/6775011/
>>
>> On Tue, Mar 29, 2011 at 4:04 PM,  <gauravsh@chromium.org> wrote:
>> >
>> > http://codereview.chromium.org/6771024/diff/9/payload_signer.cc
>> > File payload_signer.cc (right):
>> >
>> >
>> > http://codereview.chromium.org/6771024/diff/9/payload_signer.cc#newcode139
>> > payload_signer.cc:139: vector<char> padded_hash(hash);
>> > should you check here that this is indeed the size that you expect?
>> >
>> > http://codereview.chromium.org/6771024/
>> >
>
>

[adlr, 2011-03-30 19:40:14]

Hey, PTAL.

I pushed b/c I mistakenly thought you had LGTMed, but I now see you haven't. I'm
sorry.

If you see any issues, I'll fix them immediately, or I can rollback if
necessary.

-andrew

http://codereview.chromium.org/6771024/diff/9/payload_signer.cc
File payload_signer.cc (right):

http://codereview.chromium.org/6771024/diff/9/payload_signer.cc#newcode139
payload_signer.cc:139: vector<char> padded_hash(hash);
On 2011/03/29 23:04:50, gauravsh wrote:
> should you check here that this is indeed the size that you expect?

Done.

http://codereview.chromium.org/6771024/diff/9/payload_signer.h
File payload_signer.h (right):

http://codereview.chromium.org/6771024/diff/9/payload_signer.h#newcode70
payload_signer.h:70: // Pads a SHA256 hash so that it may be encrypted/signed
with RSA2048.
On 2011/03/29 23:04:05, gauravsh wrote:
> just a add a comment that this pads using the PKCS#1 v1.5 scheme. (there are
> multiple types of padding schemes) 

Done.

http://codereview.chromium.org/6771024/diff/9/payload_signer.h#newcode72
payload_signer.h:72: static bool PadRSA2048SHA256Hash(std::vector<char>* hash);
On 2011/03/29 23:04:05, gauravsh wrote:
> comment on what |hash| is? Also that in-place modifies the hash.

Done.

[gauravsh, 2011-03-31 17:48:02]

Can you upload the new patchset?

What about the key size issue? I can easily see the 2K-bit key
restriction becoming a problem in the future. In any case, if 2K bit
keys are the only ones supported, there should be checks for that to
avoid problems like Sosa's test had.

On Wed, Mar 30, 2011 at 12:40 PM,  <adlr@chromium.org> wrote:
> Hey, PTAL.
>
> I pushed b/c I mistakenly thought you had LGTMed, but I now see you haven't.
> I'm
> sorry.
>
> If you see any issues, I'll fix them immediately, or I can rollback if
> necessary.
>
> -andrew
>
>
> http://codereview.chromium.org/6771024/diff/9/payload_signer.cc
> File payload_signer.cc (right):
>
> http://codereview.chromium.org/6771024/diff/9/payload_signer.cc#newcode139
> payload_signer.cc:139: vector<char> padded_hash(hash);
> On 2011/03/29 23:04:50, gauravsh wrote:
>>
>> should you check here that this is indeed the size that you expect?
>
> Done.
>
> http://codereview.chromium.org/6771024/diff/9/payload_signer.h
> File payload_signer.h (right):
>
> http://codereview.chromium.org/6771024/diff/9/payload_signer.h#newcode70
> payload_signer.h:70: // Pads a SHA256 hash so that it may be
> encrypted/signed with RSA2048.
> On 2011/03/29 23:04:05, gauravsh wrote:
>>
>> just a add a comment that this pads using the PKCS#1 v1.5 scheme.
>
> (there are
>>
>> multiple types of padding schemes)
>
> Done.
>
> http://codereview.chromium.org/6771024/diff/9/payload_signer.h#newcode72
> payload_signer.h:72: static bool PadRSA2048SHA256Hash(std::vector<char>*
> hash);
> On 2011/03/29 23:04:05, gauravsh wrote:
>>
>> comment on what |hash| is? Also that in-place modifies the hash.
>
> Done.
>
> http://codereview.chromium.org/6771024/
>



-- 
-g