Now that the naming for Apple platforms is aligned, we can select the
variable uniformly.
Change-Id: Id547d1a4ba0585d5f9e4ea0b5d8b255b2ab2ec38
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51345
Reviewed-by: Adam Langley <agl@google.com>
The current names are confusing because (ios, aarch64) is also used on
macOS. The various Apple platforms all use the same ABI, and aarch64 is
no longer limited to iOS. (For that matter, the iOS simulator on x86
Macs is x86 iOS.) Just call it 'apple'.
Update-Note: References to 'mac' or 'ios' source lists in downstream
builds should be renamed to 'apple'.
Change-Id: Id1a0627f8ce3241f34ffa6bb245ee5783adc7c15
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51307
Reviewed-by: Adam Langley <agl@google.com>
https://boringssl-review.googlesource.com/c/boringssl/+/51265 broke
Linux and macOS platforms on architectures where we don't build
assembly. OPENSSL_NO_ASM needs to match the assembly selector.
While I'm here, remove the OPENSSL_C11_ATOMIC comment. We autodetect
atomics these days, so the flag is only useful if you want to force it.
Change-Id: I1f20f7577b5ca7e208dc90fb46a93b20da864ec5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51305
Reviewed-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-by: Adam Langley <agl@google.com>
In testing out the ECH bits on the Chromium side, it is much harder to
tell what's going on without some indication that we sent a
ClientHelloInner. This CL routes it into the callback. A corresponding
CL in Chromium will add it to NetLog.
Bug: 275
Change-Id: I945ab2679614583e875a0ba90d6cf1481ed315d9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51205
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Both call sites end up calling them in succession. This saves a little
bit of code.
Bug: 275
Change-Id: Ib87bd9be446c368f77beb3b329deaa84ef43ac95
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51186
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The ECH extension is not covered in the AAD and so should not be
referenced in ech_outer_extensions. We end up rejecting this anyway when
checking for valid ClientHelloInners, but better to reject this
explicitly, as the spec suggests.
As part of this, use the more specific error in the various tests, so we
can distinguish the two cases. (DECODE_ERROR is coming from an extra,
probably unnecessary, error in ssl_decode_client_hello_inner's caller.)
Bug: 275
Change-Id: Ibeff55e5e1b7646ce9c68c5847cd1b40a47e6480
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51185
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Description:
Mark Wooden and Franck Rondepierre noted that the square-root-mod-p
operations used in the EdDSA RFC (RFC 8032) can be simplified. For
Ed25519, instead of computing u*v^3 * (u * v^7)^((p-5)/8), we can
compute u * (u*v)^((p-5)/8). This saves 3 multiplications and 2
squarings. For more details (including a proof), see the following
message from the CFRG mailing list:
https://mailarchive.ietf.org/arch/msg/cfrg/qlKpMBqxXZYmDpXXIx6LO3Oznv4/
Testing:
Build and run the Ed25519 tests:
mkdir build
cd build
cmake -GNinja ..
ninja && ./crypto/crypto_test --gtest_filter="Ed25519Test*"
Numerical testing of the square-root computation can be done using the
following sage script:
def legendre(x,p):
return kronecker(x,p)
# Ed25519
p = 2**255-19
# -1 is a square
if legendre(-1,p)==1:
print("-1 is a square")
# 2 is a non-square
if legendre(2,p)==-1:
print("2 is a non-square")
# 2 is a generator
# this can be checked by factoring p-1
# and then showing 2**((p-1)/q) != 1 (mod p)
# for all primes q dividing p-1.
# suppose u/v is a square.
# to compute one of its square roots, find x such that
# x**4 == (u/v)**2 .
# this implies
# x**2 == u/v, or
# x**2 == -(u/v) ,
# which implies either x or i*x is a square-root of u/v (where i is a square root of -1).
# we can take x equal to u * (u*v)**((p-5)/8).
g = 2
s = p>>2 # s = (p-1)/4
i = power_mod(g, s, p)
t = p>>3 # t = (p-5)/8
COUNT = 1<<18
while COUNT > 0:
COUNT -= 1
r = randint(0,p-1) # r = u/v
v = randint(1,p-1)
u = mod(r*v,p)
# compute x = u * (u*v)**((p-5)/8)
w = mod(u*v,p)
x = mod(u*power_mod(w, t, p), p)
# check that x**2 == r, or (i*x)**2 == r, or r is not a square
rr = power_mod(x, 2, p)
if rr==r:
continue
rr = power_mod(mod(i*x,p), 2, p)
if rr==r:
continue
if legendre(r,p) != 1:
continue
print("failure!")
exit()
print("passed!")
Change-Id: Iaa284d3365dd8c9fa18a4584121013f05a3f4cc6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50965
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
This matches the source, which only builds support for these tests on
Linux. Note Android sets CMAKE_SYSTEM_NAME to "Android", so this covers
the previous ANDROID check.
Bug: 476
Change-Id: I41ca408706d0d0c5bb22006f4c31d51fc1267f69
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51165
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Upstream used the macro originally, but later added a bespoke
implementation to transparently support X9.42 DH parameter serialization
(DHXPARAMS in OpenSSL) in 2ca873e8d898e8a232ea707227400213980059a4.
We don't support DHXPARAMS, so revert to the macro.
Change-Id: Ib17902c6c640bb88ee55881945ce57a032f7061b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51105
Reviewed-by: Adam Langley <agl@google.com>
POSIX feature macros are a mess. Reportedly, FreeBSD also breaks with
_XOPEN_SOURCE, so try leaving it unset by default.
Update-Note: It's possible this will break yet another obscure UNIX.
Hopefully we can eventually find a combination that works?
Bug: 471
Change-Id: I103f8093110d343789b9c5a22eb056ab78d9cd14
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51145
Reviewed-by: Adam Langley <agl@google.com>
u8 strings in C++20 are char8_t instead of char; in order to compile on
both C++17 and C++20 we need to remove the prefix.
Change-Id: I85d1a9d72d24e8fa96ca22b1d99be9982fee8fb5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51065
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This hash table, in applications that use pooling, can dedup received
certificates in memory and thus should use a keyed hash.
Change-Id: Idc40dc8f7463025183121642b30ea0de43ebac0e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51125
Reviewed-by: Adam Langley <agl@google.com>
The check finds implicit conversions of integer literals to bools:
bool b1 = 1;
bool b2 = static_cast<bool>(1);
and transforms them to:
bool b1 = true;
bool b2 = true;
Bug: chromium:1290142
Change-Id: I15579e28f544d07b331a230b70a8278e0651150d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51085
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
See https://fuchsia-review.googlesource.com/c/fuchsia/+/624684. Also
pick up the new, more specific, name for ZX_ARM64_FEATURE_ISA_SHA2.
Update-Note: This CL is written assuming we can just rely on the SDK
changes. Per go/fuchsia-sdk-age, this seems fairly safe. If this file
fails to build due to missing symbols, update your project's Fuchsia
SDK. If this blocks something, let us know.
Change-Id: I28b0c234b577cc0de90e7ef096c15bb75a4ba501
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50926
Reviewed-by: Adam Langley <agl@google.com>
This is designed to be the minimal infrastructure required to support
using BoringSSL in the Rust ecosystem without fear of ABI drift. Bindgen
is used to generate Rust bindings in lockstep with the rest of the
build. `rust-openssl` can consume these generated bindings with minimal
changes.
Change-Id: I1dacd36a4131e22a930ebb01da00407e8465ad7e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49645
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Description:
Creating just a Gerrit password isn't enough. Before you can push a
change to Gerrit, you must also create/associate a Gerrit account with
the google account used to create the password.
This avoids "git push ..." rejections like this:
remote: PERMISSION_DENIED: The caller does not have permission
remote: [type.googleapis.com/google.rpc.LocalizedMessage]
remote: locale: "en-US"
remote: message: "\'git push\' requires a Gerrit user account."
Change-Id: Id02c1a69ccb0c2b8bf4c63b77ed3064125966eb3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50985
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This matches our other free functions.
Fixed: 473
Change-Id: Ie147995c2f5b429f78e95cfc9a08ed54181af94e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51005
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
140-3 says
> the zeroisation of protected and unprotected SSPs
> shall be performed in the following scenarios:
> ...
> For temporary value(s) generated during the integrity test of the
> module’s software or firmware upon completion of the integrity test.
(IG 9.7.B)
Change-Id: I911f294860bf33b13b2c997fc633c9bda777fc48
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50945
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Change-Id: If28138bbda4111b4a62f48cd30c7a71a675e44f7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50885
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Change-Id: Ic3305debe9c5d85b1c47be4ebcdfcbd0660f49af
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50865
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
OpenSSL's assembly files have a few places where we condition code on
__ARM_ARCH__, the minimum target ARM revision. It currently only
controls some pre-ARMv7 code. This symbol has, from what I can tell, the
same semantics as __ARM_ARCH, defined in Arm C Language Extensions, and
added in GCC 4.8 and Clang 3.2:
https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9e94a7fc5ab770928b9e6a2b74e292d35b4c94da;hp=25bab91e017eb1d6d93117f3da96fa9b43703190e98c4dbd1e
Those are over nine years old, so drop all the fallback code. Also fix
arm_arch.h to be includable on non-ARM platforms. Some tools expect all
public headers to be cleanly includable and arm_arch.h being "public"
was getting in the way (see cl/416881417).
Interestingly, arm_arch.h previously only computed __ARM_ARCH__ for
__GNUC__ and Clang doesn't define __GNUC__ on Windows. That means we
actually weren't defining __ARM_ARCH__ for Windows. But none of the
aarch64 assembly has __ARM_ARCH__-gated code, so it works out. If it
ever does, that CL smooths that over. I've gated the
__ARM_(MAX_)_ARCH__ bits on __ASSEMBLER__ to avoid breaking no-asm
Windows/aarch64 builds on MSVC. There aren't any uses in C.
Update-Note: ARM assembly now requires the compiler define __ARM_ARCH.
This is not expected to break Clang or GCC from the last 8 or 9 years.
Change-Id: Id45e95406edeecf8dda11dce9e82418516e9de1f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50849
Reviewed-by: Adam Langley <agl@google.com>
GCC's __ARMEL__ and __ARMEB__ defines denote little- and big-endian arm,
respectively. They are not defined on aarch64, which instead use
__AARCH64EL__ and __AARCH64EB__.
However, OpenSSL's assembly originally used the 32-bit defines on both
platforms and even define __ARMEL__ and __ARMEB__ in arm_arch.h. This is
less portable and can even interfere with other headers, which use
__ARMEL__ to detect little-endian arm. (Our own base.h believes
__ARMEL__ implies 32-bit arm. We just happen to check __AARCH64EL__
first. base.h is probably also always included before arm_arch.h.)
Over time, the aarch64 assembly has switched to the correct defines,
such as in 32bbb62ea634239e7cb91d6450ba23517082bab6. This commit
finishes the job.
(There is an even more official endianness detector, __ARM_BIG_ENDIAN in
the Arm C Language Extensions. But I've stuck with the GCC ones here as
that would be a larger change.)
See also https://github.com/openssl/openssl/pull/17373
Change-Id: Ic04ff85782e6599cdeaeb33d12c2fa8edc882224
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50848
Reviewed-by: Adam Langley <agl@google.com>
The latest version of ACLE splits __ARM_FEATURE_CRYPTO into two defines
to reflect that, starting ARMv8.2, the cryptography extension can
include {AES,PMULL} and {SHA1,SHA256} separately.
Also standardize on __ARM_NEON, which is the recommended symbol from
ACLE, and the only one defined on non-Apple aarch64 targets. Digging
through GCC history, __ARM_NEON__ is a bit older. __ARM_NEON was added
in GCC's 9e94a7fc5ab770928b9e6a2b74e292d35b4c94da from 2012, part of GCC
4.8.0.
I suspect we can stop paying attention to __ARM_NEON__ at this point,
but I've left both working for now. __ARM_FEATURE_{AES,SHA2} is definite
too new to fully replace __ARM_FEATURE_CRYPTO.
Tested on Linux that -march=armv8-a+aes now also drops the fallback AES
code. Previously, we would pick up -march=armv8-a+crypto, but not
-march=armv8-a+aes. Also tested that, on an OPENSSL_STATIC_ARMCAP build,
-march=armv8-a+sha2 sets the SHA-1 and SHA-256 features.
Change-Id: I749bdbc501ba2da23177ddb823547efcd77e5c98
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50847
Reviewed-by: Adam Langley <agl@google.com>
These symbols were not marked OPENSSL_EXPORT, so they weren't really
usable externally anyway. They're also very sensitive to various build
configuration toggles, which don't always get reflected into projects
that include our headers. Move them to crypto/internal.h.
Change-Id: I79a1fcf0b24e398d75a9cc6473bae28ec85cb835
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50846
Reviewed-by: Adam Langley <agl@google.com>
cpu.h contains almost entirely private symbols, which aren't reliably
usable outside the library because they lack OPENSSL_EXPORT. (And can't
have OPENSSL_EXPORT. The linker wants references to exported symbols to
go through the GOT, and our assembly doesn't do that.) In preparation
for unexporting them, move the few public APIs to crypto.h. They seem
similar in spirit to functions like CRYPTO_has_asm.
Update-Note: As part of this, I conditioned cpu-arm-linux.c on
OPENSSL_LINUX, so that the header files can have accurate conditions.
This means unrecognized ARM platforms that do not set
OPENSSL_STATIC_ARMCAP will fail to build, where previously we defaulted
to the Linux mechanisms. This matches cpu-aarch64-linux.c, which is
already gated on OPENSSL_LINUX. (And the file is quite Linux-specific.
Even if a non-Linux ELF target used getauxval for ARM capabilities, it's
unlikely that our hardcoded constants and /proc behavior applies
anyway.)
Change-Id: I1ee9eb72097be619d3f28a51b1ea058b3c37d05a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50845
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
We use underscores everywhere except these files, which use hyphens.
Switch them to be consistent.
Change-Id: I67eddbdae7caaf8405bdb4a0c1b65e6f3ca43916
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50808
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This imports the changes to sha512-armv8.pl from
upstream's af0fcf7b4668218b24d9250b95e0b96939ccb4d1.
Tweaks needed:
- Add an explicit .text because we put .LK$BITS in .rodata for XOM
- .LK$bits and code are in separate sections, so use adrp/add instead of
plain adr
- Where glibc needs feature flags to *enable* pthread_rwlock, Apple
interprets _XOPEN_SOURCE as a request to *disable* Apple extensions.
Tighten the condition on the _XOPEN_SOURCE check.
Added support for macOS and Linux, tested manually on an ARM Mac and a
VM, respectively. Fuchsia and Windows do not currently have APIs to
expose this bit, so I've left in TODOs. Benchmarks from an Apple M1 Max:
Before:
Did 4647000 SHA-512 (16 bytes) operations in 1000103us (74.3 MB/sec)
Did 1614000 SHA-512 (256 bytes) operations in 1000379us (413.0 MB/sec)
Did 439000 SHA-512 (1350 bytes) operations in 1001694us (591.6 MB/sec)
Did 76000 SHA-512 (8192 bytes) operations in 1011821us (615.3 MB/sec)
Did 39000 SHA-512 (16384 bytes) operations in 1024311us (623.8 MB/sec)
After:
Did 10369000 SHA-512 (16 bytes) operations in 1000088us (165.9 MB/sec) [+123.1%]
Did 3650000 SHA-512 (256 bytes) operations in 1000079us (934.3 MB/sec) [+126.2%]
Did 1029000 SHA-512 (1350 bytes) operations in 1000521us (1388.4 MB/sec) [+134.7%]
Did 175000 SHA-512 (8192 bytes) operations in 1001874us (1430.9 MB/sec) [+132.5%]
Did 89000 SHA-512 (16384 bytes) operations in 1010314us (1443.3 MB/sec) [+131.4%]
(This doesn't seem to change the overall SHA-256 vs SHA-512 performance
question on ARM, when hashing perf matters. SHA-256 on the same chip
gets up to 2454.6 MB/s.)
In terms of build coverage, for now, we'll have build coverage
everywhere and test coverage on Chromium, which runs this code on macOS
CI. We should request a macOS ARM64 bot for our standalone CI. Longer
term, we need a QEMU-based builder to test various features. QEMU seems
to have pretty good coverage of all this, which will at least give us
Linux.
I haven't added an OPENSSL_STATIC_ARMCAP_SHA512 for now. Instead, we
just look at the standard __ARM_FEATURE_SHA512 define. Strangely, the
corresponding -march tag is not sha512. Neither GCC and nor Clang have
-march=armv8-a+sha512. Instead, -march=armv8-a+sha3 implies both
__ARM_FEATURE_SHA3 and __ARM_FEATURE_SHA512! Yet everything else seems
to describe the SHA512 extension as separate from SHA3.
https://developer.arm.com/architectures/system-architectures/software-standards/acle
Update-Note: Consumers with a different build setup may need to
limit -D_XOPEN_SOURCE=700 to Linux or non-Apple platforms. Otherwise,
<sys/types.h> won't define some typedef needed by <sys/sysctl.h>. If you
see a build error about u_char, etc., being undefined in some system
header, that is probably the cause.
Change-Id: Ia213d3796b84c71b7966bb68e0aec92e5d7d26f0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50807
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This imports 753316232243ccbf86b96c1c51ffcb41651d9ad5,
46f4e1bec51dc96fa275c168752aa34359d9ee51, and
32bbb62ea634239e7cb91d6450ba23517082bab6.
The last commit fixes a detection of big-endian aarch64 in the kernel,
which we do not support at all, but is imported to reduce the upstream
diff. Though it points out a messy part of arm_arch.h: __ARMEL__ and
__ARMEB__ are specific to 32-bit ARM. __AARCH64EB__ and __AARCH64EL__
are the 64-bit ones. But OpenSSL's arm_arch.h defines __ARME[LB]__ for
aarch64 and uses it in perlasm. We should fix the files upstream to
look at the aarch64 ones. (Indeed our own base.h assumes __ARMEL__
implies 32-bit ARM.)
Change-Id: I6c2241e103a97e8c3599cdfa43dcc6f30d4a2581
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50806
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We currently have two aarch64 SHA-256 implementations: one using
general-purpose registers and one using the SHA-256 extensions.
Upstream's 866e505e0d663158b0fe63a7fb7455eebacc6470 added a NEON
version.
This CL syncs the transforms at the bottom of the file, to avoid
potential mistranslations in future imports. It doesn't change the
output for our current assembly.
Skips the NEON implementation itself for now. It only helps
processors without SHA-256 instructions. While Android does not
actually mandate the cryptography extensions on ARMv8, most devices
have it.
Additionally, this file does CPU dispatch in assembly, without taking
advantage of static information. We'd end up shipping both fallback
SHA-256 implementations. This is particularly silly because NEON is
mandatory in ARMv8-A anyway. (Does anyone build us on -R or -M? Probably
not?)
(If we later have a reason to import it, the binary size cost isn't that
significant. Moreover, the NEON fallback is actually slightly smaller
than the non-NEON fallback, so if we move CPU dispatch to C, importing
may even be worthwhile.)
Change-Id: I3c8ca6e77e4e6d1299f975c407cbcf4c9c240523
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50805
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This silences a pile of -Wformat-signedness warnings. We still need
casts in a few places where the API gives int but really wanted
uint16_t. There I cast to unsigned instead of uint16_t for the sake of
not losing information.
With that, we should be -Wformat-signedness-clean on GCC, so enable the
warning.
Bug: 450
Change-Id: I3ab10348bb47d398b8b9b39acf360284a8ab04d7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50771
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Whether the order makes sense is another matter, but keep them aligned
so future flags have an easier time with it.
Change-Id: I3c3912039b593a55af86078b2e9768c76ee2ee14
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50770
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The command-line parser is slightly showing its age: first, it is hard
to add new integral types, such as uint16_t, which is getting in the way
of fixing some of the -Wformat-signedness errors. Second, the parameter
extraction logic and skipping logic is duplicated in every type.
While I'm here, use a binary search to look up the flag, since we have
rather a lot of them. With more C++ template tricks, we could avoid the
std::function, but that seemed more trouble than was worth it,
especially since, prior to C++17, it's a little hard to convince
template argument deduction to infer one of the parameters.
Change-Id: I208f89d46371b31fc8b44487725296bcd9d7c8e7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50769
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This fix isn't ideal, given the current space of possible version
values. But rather than make the printing code complicated, we should
make invalid versions impossible. I've left TODOs where that would be
needed.
Bug: 467, 450
Change-Id: I6c9ae97b8454182b0c1ab6ba2e070dc6d7d8b3f4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50767
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
X509_print_ex tries to print negative serial numbers in decimal. In
doing so, it ends up passing a signed long to %lx and trips
-Wformat-signed.
A minimal fix would be to cast to unsigned long, but this unsigned long
is the absolute value of a signed long (l = -l). This is tricky because
-LONG_MIN does not fit in long. It all works because the length check
only allows one bit short of sizeof(long)*8 bits (ASN1_INTEGER is
sign-and-magnitude).
Still, this is a whole lot of subtlety to account for an invalid case.
Instead, send negative serial numbers down the generic path.
Bug: 450
Change-Id: Ib215fd23863de27e01f7ededf95578f9c800da37
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50766
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
GCC has a warning that complains about even more type mismatches in
printf. Some of these are a bit messy and will be fixed in separate CLs.
This covers the easy ones.
The .*s stuff is unfortunate, but printf has no size_t-clean string
printer. ALPN protocol lengths are bound by uint8_t, so it doesn't
really matter.
The IPv6 printing one is obnoxious and arguably a false positive. It's
really a C language flaw: all types smaller than int get converted to
int when you do arithmetic. So something like this first doesn't
overflow the shift because it computes over int, but then the result
overall is stored as an int.
uint8_t a, b;
(a << 8) | b
On the one hand, this fixes a "missing" cast to uint16_t before the
shift. At the same time, the incorrect final type means passing it to
%x, which expects unsigned int. The compiler has forgotten this value
actually fits in uint16_t and flags a warning. Mitigate this by storing
in a uint16_t first.
The story doesn't quite end here. Arguments passed to variadic functions
go through integer promotion[0], so the argument is still passed to
snprintf as an int! But then va_arg allows for a signedness mismatch[1],
provided the value is representable in both types. The combination means
that %x, though actually paired with unsigned, also accept uint8_t and
uint16_t, because those are guaranteed to promote to an int that meets
[1]. GCC recognizes [1] applies here.
(There's also PRI16x, but that's a bit tedious to use and, in glibc, is
defined as plain "x" anyway.)
[0] https://en.cppreference.com/w/c/language/conversion#Default_argument_promotions
[1] https://en.cppreference.com/w/c/variadic/va_arg
Bug: 450
Change-Id: Ic1d41356755a18ab922956dd2e07b560470341f4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50765
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Update-Note: ".example.com" as an input DNS name will no longer match
"www.example.com" in a certificate. (Note this does not impact wildcard
certificates. Rather, it removes a non-standard "reverse wildcard" that
OpenSSL implemented.)
Fixed: 463
Change-Id: I627e1bd00b8e4b810e9bb756f424f6230a99496e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50726
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This replaces v3name_test.cc which is rather difficult to follow.
v3name_test.cc ran all pairs of names against each other, used a
default case-insensitivity rule, and then had a list of string
exceptions to that rule. This is hopefully easier for us to adjust
later. It also fixes a testing bug we wouldn't notice if an expected
"exception" didn't fire.
Sadly, we cannot use designated initializers in C++ yet. MSVC does not
support them until MSVC 2019.
Change-Id: Ia8e3bf5f57d33a9bf1fc929ba1e8cd2a270a8a24
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50725
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Always enable X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS and never enable
X509_CHECK_FLAG_MULTI_LABEL_WILDCARDS.
Update-Note: BoringSSL will no longer accept wildcard patterns like
*www.example.com or www*.example.com. (It already did not accept
ww*w.example.com.) X509_CHECK_FLAG_MULTI_LABEL_WILDCARDS will also be
ignored and can no longer be used to allow foo.bar.example.com to match
*.example.com.
Fixes: 462
Change-Id: I004e087bf70f4c3f249235cd864d9e19cc9a5102
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50705
Reviewed-by: Adam Langley <agl@google.com>
I believe, with this, we have aligned with OpenSSL 1.1.x on the
crypto/x509 and crypto/asn1 types that are now opaque. Strangely,
OpenSSL kept X509_ALGOR public. We may wish to hide that one too later,
but we can leave it for now.
Update-Note: Use X509_REVOKED accessors rather than reaching into the
struct.
Bug: 425
Change-Id: Ib47944648a8693ed7078ffe94f7b557022debe30
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50685
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I9ba12ad7b3cfc9a6d1015da728cec45e4b71dcc9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50665
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
rust-openssl, rather than using Rust's existing error types, exposes the
OpenSSL error queue as the error type in its public callback types.
Supporting a simplified version of ERR_set_error_data is simple enough,
so it's easiest just to add this function.
Unlike OpenSSL's, we don't attempt to support non-string error data. We
also don't try to retain borrowed pointers. If the caller did not pass
ownership, make a copy internally.
Change-Id: I909eebc2867ab1f3b9975546a106ee1f762bf516
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50625
Reviewed-by: Adam Langley <agl@google.com>
David Benjamin
Piotr Sikora
James Muir
Mark Schott
Anton Bikineev
Benjamin Brittain
Adam Langley