Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rebased from upstream 0.16.19 #6

Merged
merged 392 commits into from
Dec 9, 2020

Conversation

trevor-crypto
Copy link
Collaborator

I had to apply the changes from #5 in order to get the tests to actually pass.

From my research into a few issues (fortanix/rust-sgx#30, briansmith#775) it doesn't seem that sgx will be supported on ring

agl and others added 30 commits October 16, 2020 21:36
A recent change broke this but I didn't notice. (Which suggests that the
test isn't very useful, which is true, but I'm not ready to pull the
trigger on deleting it just yet.)

Change-Id: If120a553c095fa0be9f8e85fc05ee996a486621f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43484
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Use empty() over size() == 0, and don't export the IterateAES*
functions. (They return private types.)

Change-Id: I8a8f33a64e28cc2eab789563c6ba91afa6df87f9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43544
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
With -Wundef one could get warnings of undefined symbols.
This patch tries to fix this issue.

Furthermore, the case where there is BTI but no Pointer Authentication
now uses GNU_PROPERTY_AARCH64_BTI in the check which should correctly
reflect that feature's enabled state.

Change-Id: I14902a64e5f403c2b6a117bc9f5fb1a4f4611ebf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43524
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Change-Id: I9cb1ed08fff71d94c28f16efa4d356bdfdebeebe
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43564
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Change-Id: I1cb16d926a58de5345de462c857774775c865c2f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43565
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The constructor parameter vs. method name one is a little unfortunate
given Google C++ style, but I think we've done this elsewhere in libssl,
so let's run with it for now.

Bug: 378
Change-Id: I31fb6b4b16e3248369dae6f47cc150de0e4f04fe
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43545
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Change-Id: I4ffa2572acce1fdccdf4d3c33680e6d0114bd42b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43405
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: I2e6cc7367b5ca6631329be298fbed7424221a06b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43406
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
…ures.""

This is a looser reland of
https://boringssl-review.googlesource.com/c/boringssl/+/41804, which was
reverted in
https://boringssl-review.googlesource.com/c/boringssl/+/42804.

Enforcing that the ECDSA parameters were omitted rather than NULL hit
some compatibility issues, so instead allow either forms for now. To
align with the Chromium verifier, we'll probably want to later be
stricter with a quirks flag to allow the invalid form, and then add a
similar flag to Chromium. For now, at least try to reject the completely
invalid parameter values.

Update-Note: Some invalid certificates will now be rejected at
verification time. Parsing of certificates is unchanged.

Bug: b/167375496,342
Change-Id: I1cba44fd164660e82a7a27e26368609e2bf59955
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43664
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Change-Id: I9aa66109bd0f6acc0c30a505eef6d85b6972132d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43624
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: I40bbf6d10fcfd1e0fb506bef44f4cd6e9d2daac5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43644
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: Ibc794a66ea9b04e2d48c2124d52234a0bed10aff
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43625
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Change-Id: I8697230d4feb3bc5308905aa8981087b0f080555
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43626
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
These booleans are a little hard to understand in context and adding
any more makes things even more complicated. Thus make them flags so
that the meaning is articulated locally.

Change-Id: I8cdb7fd5657bb12f28a73d7c6818d400c987ad3b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43684
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This change is expected to be a non-functional change.

Original request:
https://boringssl-review.googlesource.com/c/boringssl/+/42084

Change-Id: Ifbf85eb6cafebabf0cf063b7dd147417d01c280c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43584
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
@trevor-crypto trevor-crypto changed the title Rebased onto upstream 0.16.19 Merged from upstream 0.16.19 Dec 8, 2020
@trevor-crypto trevor-crypto changed the title Merged from upstream 0.16.19 Rebased from upstream 0.16.19 Dec 8, 2020
src/aead/gcm.rs Outdated
@@ -65,7 +65,7 @@ impl Key {
}
}

#[cfg(all(not(target_arch = "aarch64"), not(target_env = "sgx")))]
#[cfg(all(not(target_arch = "aarch64")))]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#5 (review) -- no need for "all" + why not to use the HW implementation?

Copy link
Collaborator Author

@trevor-crypto trevor-crypto Dec 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for "all"

Will change this.

HW implementation

I'm probably missing something, cause I'm new to the sgx stuff, but I thought we were trying to avoid the changes from this patch briansmith@7c29308 and the panic! state

@@ -52,97 +51,97 @@ pub(crate) fn features() -> Features {
let [l1edx, l1ecx, l7ebx, l7ecx] = unsafe { &mut GFp_ia32cap_P };
Copy link

@tomtau tomtau Dec 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw later in this detection that it requires feature = "sgx" -- maybe just target_env = "sgx" is enough?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah most likely is enough, since we're using the sgx feature by default. I will change it.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, perhaps no need for sgx feature, if it can be guarded with target_env

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.