third_party/openssl: add ChaCha20+Poly1305 support.

third_party/openssl: add ChaCha20+Poly1305 support.

This change is not as scary as it appears. Most of the code has already been
reviewed and is running in production without issues. The only new code is the
ARM support.

ARM now builds both the NEON and generic versions of the code and can enable
the NEON code at runtime by calling CRYPTO_set_NEON_capable(1). This patch does
not contain the code to call that, however.

The addition openssl/patches/channelidchromium.patch and
fix_lhash_iteration.patch is noise from the import script.

BUG=310768

[digit1, 2013-11-06 15:52:17]

rubberstamp lgtm!

Not commenting on the actual patch content itself, but it is really nice that
you added a .s file for the ARM case, to allow the issues related to rebuilding
with -mfpu=neon. Thanks :)

It looks like this patch is not totally well-formed, i.e. if I only copy
patches.chromium/0007-chacha.patch to my local checkout and re-run
./import-from-android.sh, this fails with two rejects in openssl/openssl.config,
this is not supposed to happen. But maybe that works if all changes are applied
at once.

Can you send me the whole patchset privately, since chromiumcodereview doesn't
allow to download it? I'd like to verify that it applies cleanly. Also, I'm
currently trying to simplify our patch management process by making small
changes to import_from_android.sh and openssl/import_openssl.sh, so I could
check that this fits in.

Can you update README.chromium to describe 0007-chacha.patch too?

[agl, 2013-11-06 16:07:43]

On Wed, Nov 6, 2013 at 10:52 AM,  <digit@chromium.org> wrote:
> It looks like this patch is not totally well-formed, i.e. if I only copy
> patches.chromium/0007-chacha.patch to my local checkout and re-run
> ./import-from-android.sh, this fails with two rejects in
> openssl/openssl.config,

Hmm. Are you are r233061? I landed a couple of changes to add a patch
file for the lhash change in SVN and if you don't have that, the
ChaCha patch may fail to apply.

(First change worked, but it patched the files directly, which is
incorrect. The second change corrected it to be a patch file that adds
a patch file that, in turn, patches the source. Have I mentioned how
many hours I spent getting the patches correct yesterday? :( )


Cheers

AGL

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[digit, 2013-11-06 16:13:36]

On Wed, Nov 6, 2013 at 5:06 PM, Adam Langley <agl@chromium.org> wrote:

> On Wed, Nov 6, 2013 at 10:52 AM,  <digit@chromium.org> wrote:
> > It looks like this patch is not totally well-formed, i.e. if I only copy
> > patches.chromium/0007-chacha.patch to my local checkout and re-run
> > ./import-from-android.sh, this fails with two rejects in
> > openssl/openssl.config,
>
> Hmm. Are you are r233061? I landed a couple of changes to add a patch
> file for the lhash change in SVN and if you don't have that, the
> ChaCha patch may fail to apply.
>
>
Ah, that might explain it, might DEPS is at 231572
But that also means that my DEPS roll to 23307 [1] is not needed anymore.
Weird that it is failing so bad on the linux_aura bot.

I'll check again, thanks

(First change worked, but it patched the files directly, which is
> incorrect. The second change corrected it to be a patch file that adds
> a patch file that, in turn, patches the source. Have I mentioned how
> many hours I spent getting the patches correct yesterday? :( )
>
> Yes, that's exactly the point I'd like to address :) I've been trying to
rebase the current sources on top of a newer AOSP commit, and it's still
too difficult. I'll keep you posted when I have something I'm happy with.


>
> Cheers
>
> AGL
>

[1] https://codereview.chromium.org/60483002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[agl, 2013-11-06 16:18:14]

On Wed, Nov 6, 2013 at 11:13 AM, David Turner <digit@google.com> wrote:
> Ah, that might explain it, might DEPS is at 231572
> But that also means that my DEPS roll to 23307 [1] is not needed anymore.
> Weird that it is failing so bad on the linux_aura bot.

The changes that I made were just adding the patch files to match up
with the existing code - so the DEPS roll to at least 233017 is needed
in order to bring in the effective change. I wouldn't want that crash
fix to have to wait for the ChaCha change because I expect issues with
the ARM build with the ChaCha code.

Also, sometimes the bots are just bust and it's ok to manually submit.

> Yes, that's exactly the point I'd like to address :) I've been trying to
> rebase the current sources on top of a newer AOSP commit, and it's still too
> difficult. I'll keep you posted when I have something I'm happy with.

AOSP is using git, and Chromium can deal with git DEPS. Maybe just
maintain Chromium as a branch of the AOSP tree into which we can merge
AOSP changes periodically?


Cheers

AGL

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[digit, 2013-11-06 16:23:50]

Hum.. a recent checkout to tip-of-tree still puts the DEPS to 231572, did
you DEPS roll your changes?
It looks like the most recent OpenSSL-related DEPS rool is
https://codereview.chromium.org/50783004


On Wed, Nov 6, 2013 at 5:13 PM, David Turner <digit@google.com> wrote:

>
>
>
> On Wed, Nov 6, 2013 at 5:06 PM, Adam Langley <agl@chromium.org> wrote:
>
>> On Wed, Nov 6, 2013 at 10:52 AM,  <digit@chromium.org> wrote:
>> > It looks like this patch is not totally well-formed, i.e. if I only copy
>> > patches.chromium/0007-chacha.patch to my local checkout and re-run
>> > ./import-from-android.sh, this fails with two rejects in
>> > openssl/openssl.config,
>>
>> Hmm. Are you are r233061? I landed a couple of changes to add a patch
>> file for the lhash change in SVN and if you don't have that, the
>> ChaCha patch may fail to apply.
>>
>>
> Ah, that might explain it, might DEPS is at 231572
> But that also means that my DEPS roll to 23307 [1] is not needed anymore.
> Weird that it is failing so bad on the linux_aura bot.
>
> I'll check again, thanks
>
> (First change worked, but it patched the files directly, which is
>> incorrect. The second change corrected it to be a patch file that adds
>> a patch file that, in turn, patches the source. Have I mentioned how
>> many hours I spent getting the patches correct yesterday? :( )
>>
>> Yes, that's exactly the point I'd like to address :) I've been trying to
> rebase the current sources on top of a newer AOSP commit, and it's still
> too difficult. I'll keep you posted when I have something I'm happy with.
>
>
>>
>> Cheers
>>
>> AGL
>>
>
> [1] https://codereview.chromium.org/60483002/
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[agl, 2013-11-06 16:25:55]

On Wed, Nov 6, 2013 at 11:23 AM, David Turner <digit@google.com> wrote:
> Hum.. a recent checkout to tip-of-tree still puts the DEPS to 231572, did
> you DEPS roll your changes?
> It looks like the most recent OpenSSL-related DEPS rool is
> https://codereview.chromium.org/50783004

No, I expected you to do a DEPS roll and so didn't want to collide.
But you can set 0007-chacha.patch from SVN or by git checkout
origin/master after a git fetch.


Cheers

AGL

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[digit, 2013-11-06 16:43:46]

On Wed, Nov 6, 2013 at 5:17 PM, Adam Langley <agl@chromium.org> wrote:

> On Wed, Nov 6, 2013 at 11:13 AM, David Turner <digit@google.com> wrote:
> > Ah, that might explain it, might DEPS is at 231572
> > But that also means that my DEPS roll to 23307 [1] is not needed anymore.
> > Weird that it is failing so bad on the linux_aura bot.
>
> The changes that I made were just adding the patch files to match up
> with the existing code - so the DEPS roll to at least 233017 is needed
> in order to bring in the effective change. I wouldn't want that crash
> fix to have to wait for the ChaCha change because I expect issues with
> the ARM build with the ChaCha code.
>
> I see, this works when checking out the tip-of-tree of deps/openssl/,
indeed. There is a strange warning when running import_openssl.sh though:

Makefile:120: target `.c' given more than once in the same rule.
Makefile:120: target `.c' given more than once in the same rule.

Unfortunately, using the --verbose option fails due to a bug in
import_from_android.sh, I'll look into it.

In case this isn't obvious, I'm ok with the patch :)


> Also, sometimes the bots are just bust and it's ok to manually submit.
>
> I'll do that once the tree is green again.


> > Yes, that's exactly the point I'd like to address :) I've been trying to
> > rebase the current sources on top of a newer AOSP commit, and it's still
> too
> > difficult. I'll keep you posted when I have something I'm happy with.
>
> AOSP is using git, and Chromium can deal with git DEPS. Maybe just
> maintain Chromium as a branch of the AOSP tree into which we can merge
> AOSP changes periodically?
>
> The thing is we want to be able to send our changes to AOSP easily, which
means the ability to create openssl/patches/ files properly. My experience
is that this doesn't work too well if we merge AOSP changes directly into
our (modified) git tree, but I may be wrong. There are specialized tools
like stacked-git or quilt which are designed to manage list of patches
easily too, we could look into that.




>
> Cheers
>
> AGL
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.