GitLab

Corresponding Chrome CL:
https://codereview.chromium.org/2899403004/

BUG=chromium:723069

Change-Id: I0ab9c104263d12e8251adf60d89ad19cf0436785
Reviewed-on: https://boringssl-review.googlesource.com/16664

Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

Windows Clang needs this in the stack case too, but it doesn't define
__GNUC__ since it's emulating MSVC.

Change-Id: I646550ca95240e80822adddc2b53c3b58c2ec4a6
Reviewed-on: https://boringssl-review.googlesource.com/16644


Commit-Queue: Steven Valdez <[email protected]>
Reviewed-by: Steven Valdez <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

Chromium's tooling is unhappy.

Change-Id: Iefa70ceca5b62cc65f19ee46a33068139b79b3fc
Reviewed-on: https://boringssl-review.googlesource.com/16624


Commit-Queue: David Benjamin <[email protected]>
Commit-Queue: Steven Valdez <[email protected]>
Reviewed-by: Steven Valdez <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

This change replace the cmovq scheme with slightly faster SSE2 code.
The SSE2 code was first introduced in Go's curve25519 implementation.
See: https://go-review.googlesource.com/c/39693/

The implementation is basicly copied from the Go assembly.

Change-Id: I25931a421ba141ce33809875699f048b0941c061
Reviewed-on: https://boringssl-review.googlesource.com/16564

Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

The only place it is used is EC_KEY_{dup,copy} and no one calls that
function on an EC_KEY with ex_data. This aligns with functions like
RSAPublicKey_dup which do not copy ex_data. The logic is also somewhat
subtle in the face of malloc errors (upstream's PR 3323).

In fact, we'd even changed the function pointer signature from upstream,
so BoringSSL-only code is needed to pass this pointer in anyway. (I
haven't switched it to CRYPTO_EX_unused because there are some callers
which pass in an implementation anyway.)

Note, in upstream, the dup hook is also used for SSL_SESSIONs when those
are duplicated (for TLS 1.2 ticket renewal or TLS 1.3 resumption). Our
interpretation is that callers should treat those SSL_SESSIONs
equivalently to newly-established ones. This avoids every consumer
providing a dup hook and simplifies the interface.

(I've gone ahead and removed the TODO(fork). I don't think we'll be able
to change this API. Maybe introduce a new one, but it may not be worth
it? Then again, this API is atrocious... I've never seen anyone use argl
and argp even.)

BUG=21

Change-Id: I6c9e9d5a02347cb229d4c084c1e85125bd741d2b
Reviewed-on: https://boringssl-review.googlesource.com/16344

Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

This is for demonstrating an integrity check failure.

Change-Id: I4b52b1aa5450f5dec024f381863aeed92b5e9ce0
Reviewed-on: https://boringssl-review.googlesource.com/16465

Reviewed-by: Martin Kreichgauer <[email protected]>
Reviewed-by: Adam Langley <[email protected]>

It shouldn't have been defined for variable-length nonces at all, but so
it goes. EVP_CIPHER rejected this by way of EVP_CTRL_GCM_SET_IVLEN
comparing <= 0, but the EVP_AEAD API did not.

I've done the test in a separate file on the assumption that aead_test
will become GTest shortly, at which point it will be easy to stick extra
tests into the same file as the FileTest ones.

Thanks to Daniel Bleichenbacher and Thanh Bui of Project Wycheproof for
the report.

Change-Id: Ic4616b39a1d7fe74a1f14fb58cccec2ce7c4f2f3
Reviewed-on: https://boringssl-review.googlesource.com/16544

Reviewed-by: Adam Langley <[email protected]>

BUG=129

Change-Id: I4a501fa620b7b5b4c585731ec1ece94407e9b727
Reviewed-on: https://boringssl-review.googlesource.com/16510

Reviewed-by: Adam Langley <[email protected]>

BUG=129

Change-Id: I784a745f102d9d09d35b27542d94b2b85dfb332e
Reviewed-on: https://boringssl-review.googlesource.com/16508

Reviewed-by: Adam Langley <[email protected]>

This introduces machinery to start embedding the test data files into
the crypto_test binary. Figuring out every CI's test data story is more
trouble than is worth it. The GTest FileTest runner is considerably
different from the old one:

- It returns void and expects failures to use the GTest EXPECT_* and
  ASSERT_* macros, rather than ExpectBytesEqual. This is more monkey
  work to convert, but ultimately less work to add new tests. I think
  it's also valuable for our FileTest and normal test patterns to align
  as much as possible. The line number is emitted via SCOPED_TRACE.

- I've intentionally omitted the Error attribute handling, since that
  doesn't work very well with the new callback. This means evp_test.cc
  will take a little more work to convert, but this is again to keep our
  two test patterns aligned.

- The callback takes a std::function rather than a C-style void pointer.
  This means we can go nuts with lambdas. It also places the path first
  so clang-format doesn't go nuts.

BUG=129

Change-Id: I0d1920a342b00e64043e3ea05f5f5af57bfe77b3
Reviewed-on: https://boringssl-review.googlesource.com/16507

Reviewed-by: Adam Langley <[email protected]>

Change-Id: I6914691628c5ece4564be2bf60e17484081188fe
Reviewed-on: https://boringssl-review.googlesource.com/16509

Reviewed-by: Adam Langley <[email protected]>

In GTest, we'll just burn the files into the binary and not worry about
this. Apparently test files is a one of computer science's great
unsolved problems and everyone has their own special-snowflake way of
doing it. Burning them into the executable is easier.

BUG=129

Change-Id: Ib39759ed4dba6eb9ba97f0282f000739ddf931fe
Reviewed-on: https://boringssl-review.googlesource.com/16506

Reviewed-by: Adam Langley <[email protected]>

This is no longer necessary as of
7ed2e82e.

Change-Id: I02d4df0bc92bf3c5826e9e4f27ce153ae419c258
Reviewed-on: https://boringssl-review.googlesource.com/16505

Reviewed-by: Adam Langley <[email protected]>

This is no longer used.

Change-Id: I74bf2ffff5260cdbf1356e8628fa4f9a667217e8
Reviewed-on: https://boringssl-review.googlesource.com/16504

Reviewed-by: Adam Langley <[email protected]>

Instead of a script which generates macros, emit static inlines in
individual header (or C files). This solves a few issues with the
original setup:

- The documentation was off. We match the documentation now.

- The stack macros did not check constness; see some of the fixes in
  crypto/x509.

- Type errors did not look like usual type errors.

- Any type which participated in STACK_OF had to be made partially
  public. This allows stack types to be defined an internal header or
  even an individual file.

- One could not pass sk_FOO_free into something which expects a function
  pointer.

Thanks to upstream's 411abf2dd37974a5baa54859c1abcd287b3c1181 for the
idea.

Change-Id: Ie5431390ccad761c17596b0e93941b0d7a68f904
Reviewed-on: https://boringssl-review.googlesource.com/16087

Reviewed-by: Adam Langley <[email protected]>

Change-Id: I7bf485a9bfe0d7b7a3dc3081f86278fee87b8c74
Reviewed-on: https://boringssl-review.googlesource.com/16485

Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

Change-Id: Iab7a738a8981de7c56d1585050e78699cb876dab
Reviewed-on: https://boringssl-review.googlesource.com/16467


Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

This change causes FIPS mode to use RDRAND in preference to the kernel's
entropy pool. This prevents issues where the ioctl that we have to do
when getrandom isn't supported transiently reports that the pool is
“empty” and causes us to block.

Change-Id: Iad50e443d88b168bf0b85fe1e91e153d79ab3703
Reviewed-on: https://boringssl-review.googlesource.com/16466


Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

We returned the wrong type, but with a typedef which made it void*. In
C++, void* to T* doesn't implicitly convert, so it doesn't quite work
right. Notably, Node passes it into sk_SSL_COMP_zero. The sk_* macros
only weakly typecheck right now, but a pending CL converts them to
proper functions.

Change-Id: I635d1e39e4f4f11b2b7bf350115a7f1b1be30e4f
Reviewed-on: https://boringssl-review.googlesource.com/16447

Reviewed-by: Adam Langley <[email protected]>

BUG=129

Change-Id: I7ce961f136b9c63cd02a364d6a78fd875c240d25
Reviewed-on: https://boringssl-review.googlesource.com/16446


Commit-Queue: Steven Valdez <[email protected]>
Reviewed-by: Steven Valdez <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

Also allow breaking ECDSA/RSA pair-wise consistency tests and ECDSA
self-test.

Change-Id: I1c7723f6082568ebf93158cfaa184cbdeb7480a0
Reviewed-on: https://boringssl-review.googlesource.com/16305

Reviewed-by: Adam Langley <[email protected]>

Rather than comparing against both endpoints, subtract the minimum and
rely on unsigned wraparound to do both comparisons at once. This seems
to be slightly faster.

In addition, constant_time_lt_8 becomes much simpler if it can assume
that |a| and |b| have the same MSB. But we can arrange that by casting
up to |crypto_word_t| (which is otherwise happening anyway).

Change-Id: I82bd676e487eb7bb079ba7286df724c1c380bbb4
Reviewed-on: https://boringssl-review.googlesource.com/16445

Reviewed-by: Adam Langley <[email protected]>

With the constant-time base64 decode, base64_ascii_to_bin is a bit more
expensive. This check is redundant with the one in base64_decode_quad,
though it does mean syntax error reporting will be slightly deferred by
four bytes.

Change-Id: I71f23ea23feba2ee5b41df79ce09026fb56996d3
Reviewed-on: https://boringssl-review.googlesource.com/16444

Reviewed-by: Adam Langley <[email protected]>

At some point we'll need to run this against an actual run, where FAX
files are unavailable.

Change-Id: I244bdb6608faf78f321d7016416bbad0486fd0b6
Reviewed-on: https://boringssl-review.googlesource.com/16424

Reviewed-by: Adam Langley <[email protected]>

Change-Id: Ie466b7b55bdd679c5baf2127bd8de4a5058fc3b7
Reviewed-on: https://boringssl-review.googlesource.com/16346


Commit-Queue: Steven Valdez <[email protected]>
Reviewed-by: Steven Valdez <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

Since SSLv3 is disabled by default now this is not needed anymore, but
it makes enabling SSLv3 using -min-version impossible.

At some point this should be removed anyway (when SSLv3 support is
removed), so might as well do it now and fix this tiny problem.

Change-Id: Ie3f7453b5b5198f33fcc4d4294102f116b8843ae
Reviewed-on: https://boringssl-review.googlesource.com/16404

Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

Saves having it in several places.

Change-Id: I329e1bf4dd4a7f51396e36e2604280fcca32b58c
Reviewed-on: https://boringssl-review.googlesource.com/16026

Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

5c38c05b caused [email protected] for
external foo to resolve to bcm_redirector_foo. This is morally
equivalent to using [email protected] when a pointer to foo is needed. But this
does not work if foo is data. Notably, this ended up mangling
OPENSSL_ia32cap_P because it failed to recognize it as an symbol in the
library (but external to the module). It also mangles some things that
ASan emits.

(It also breaks non-NULL function pointer comparisons, but those are
silly.)

Instead, apply a variation of the OPENSSL_ia32cap_addr_delta trick that
works for the GOT. "addr_delta" is really weird, so I'm calling this an
"external relocation". This causes fprintf(stderr) to work and also
seems to keep ASan compiling. I was unable to reproduce the case that
5c38c05b added the bcm_redirector_foo
transform for.

Also tighten up the pattern. No need to reference a bit of memory twice
since we just loaded it into a register.

Change-Id: If5520fc0887e83e23a08828e40fbbed9e47d912e
Reviewed-on: https://boringssl-review.googlesource.com/16345


Commit-Queue: David Benjamin <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

Let's see if the SDE builders work now.

Change-Id: Iad555b5e9ed4b65ee65ac6d276282c110007f362

Change-Id: I7d8f9098038a82b29ab0eff8a3258975d8804a68
Reviewed-on: https://boringssl-review.googlesource.com/16264

Reviewed-by: David Benjamin <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

AES-GCM-SIV specifies that the counter is a 32-bit, unsigned number.
These test vectors are crafted to trigger a wrap-around and ensure that
corner of the spec is implemented correctly.

Change-Id: I911482ca0b6465a7623ee1b74a6cb1d5e54ddbea
Reviewed-on: https://boringssl-review.googlesource.com/16324


Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: David Benjamin <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

Change-Id: I358b5da2463597e6b3d5841a4faa974c604e5df5
Reviewed-on: https://boringssl-review.googlesource.com/16284

Reviewed-by: Matt Braithwaite <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
CQ-Verified: CQ bot account: commit-b[email protected]rg <[email protected]>

Change-Id: I3d1ddc8cca9fb1da5d0b6a68ba2125c89e5bc0ce
Reviewed-on: https://boringssl-review.googlesource.com/16304


Commit-Queue: Martin Kreichgauer <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Reviewed-by: Adam Langley <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

This makes things a little easier for some of our tooling.

Change-Id: Ia7e73daf0a5150b106cf9b03b10cae194cb8fc5a
Reviewed-on: https://boringssl-review.googlesource.com/15104

Reviewed-by: Matt Braithwaite <[email protected]>
Reviewed-by: Adam Langley <[email protected]>

43e5a26b removed the .file directive
from x86asm.pl. This removes the parameter from asm_init altogether. See
also upstream's e195c8a2562baef0fdcae330556ed60b1e922b0e.

Change-Id: I65761bc962d09f9210661a38ecf6df23eae8743d
Reviewed-on: https://boringssl-review.googlesource.com/16247

Reviewed-by: Steven Valdez <[email protected]>
Commit-Queue: Steven Valdez <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

Once the ACL issue is sorted out, this will mean we don't need to
re-download it each run.

BUG=180

Change-Id: I589d3944f8b6ab93d8fd279966484621ab825daa
Reviewed-on: https://boringssl-review.googlesource.com/16248


Commit-Queue: Steven Valdez <[email protected]>
Reviewed-by: Steven Valdez <[email protected]>
CQ-Verified: CQ bot account: [email protected] <[email protected]>

Tweaked the SDE ACLs slightly. Unclear if that fixed it.

Change-Id: I29f185b0bc36aee4481e7dfa1fb8d3e48b75612a

This reverts commit fee85591.

Oops. Didn't mean to push that one without review.

Once the ACL issue is sorted out, this will mean we don't need to
re-download it each run.

BUG=180

Change-Id: Iddcceafa3b359f8d5c7875887ecbaf21671c93f9

Change-Id: If69184003f00123c5b1d1c81fc5b0fe757360687