8000 Implements an additional fork detection mechanism: pthread_atfork by torben-hansen · Pull Request #142 · aws/aws-lc · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Implements an additional fork detection mechanism: pthread_atfork #142

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
May 8, 2021
Merged

Implements an additional fork detection mechanism: pthread_atfork #142

merged 7 commits into from
May 8, 2021

Conversation

torben-hansen
Copy link
Contributor

Issues:

CryptoAlg-727

Description of changes:

AWS-LC currently has one fork detection mechanisms: WIPEONFORK. This PR implements an additional fork detection mechanism using the pthread library function pthread_atfork. Using this function, we can register a handler that is executed in the child after a fork call fork() that is used to flip a sentinel fork detection value. This change is intended to add defense-in-depth to the fork detection.

The code flow is roughly as follows:

  • The fork detection is lazily initialised
  • On first call: init_fork_detect is only called by the first thread entering it
  • mmap a page that is acting as the sentinel informing whether a fork has occurred or not
  • Initialise the two fork detection mechanisms
  • If initialisation is successful, set the sentinel value to 1 (indicating no fork has occurred). The value 0 indicates that a fork has occurred.

Call-outs:

Emphasis has been given to keeping the generation number logic in the function CRYPTO_get_fork_generation as is.

Had to move things around to be able to inject ignore flags into the code path for testing. This probably makes merging harder, but beneficial for possible future extensions.

Testing:

Added extra dimensions that test each fork detection mechanism in isolation. This ensures that each mechanism is correctly initialised and can properly detect a fork. Existing tests cover the full combination of the mechanisms.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

nebeid
nebeid reviewed May 4, 2021
View reviewed changes
@torben-hansen
Copy link
Contributor Author

Diagram that might aid code flow understanding.

fork_detection_awslc

nebeid
nebeid reviewed May 5, 2021
View reviewed changes
nebeid
nebeid previously approved these changes May 5, 2021
View reviewed changes
dkostic
dkostic reviewed May 6, 2021
View reviewed changes
dkostic
dkostic previously approved these changes May 6, 2021
View reviewed changes
@torben-hansen torben-hansen dismissed stale reviews from dkostic and nebeid via 442eb09 May 6, 2021 23:45
nebeid
nebeid previously approved these changes May 7, 2021
View reviewed changes
dkostic
dkostic previously approved these changes May 7, 2021
View reviewed changes
@torben-hansen torben-hansen dismissed stale reviews from dkostic and nebeid via 60090a7 May 7, 2021 15:28
@torben-hansen torben-hansen merged commit 5ada8e8 into aws:main May 8, 2021
@torben-hansen torben-hansen mentioned this pull request Nov 4, 2022
Merged
torben-hansen added a commit that referenced this pull request Nov 8, 2022
@torben-hansen
Remove pthread_atfork fork detection mechanism (#673)
8569259 
#142 added pthread_atfork() as a secondary fork detection mechanism. However, we have observed several issues consuming AWS-LC artifacts due to the symbol pthread_atfork being handled differently in distributions (and in a non-portable way). This affects our ability to distribute AWS-LC to our intended places. See internal document for details.

Therefore, completely remove pthread_atfork() from the code-base.
Taffer added a commit that referenced this pull request Nov 10, 2022
@Taffer @billbo-yang
@davidben @osokin @nebeid @dkostic @justsmth @torben-hansen @andrewhop @agl @Biswa96
Update the 2022 FIPS branch to current main (#677)
0d96a03 
* rename tlskdf.go to kdf-components.go to more accurately reflect NIST test vector naming scheme (#666)

* Check for TLS 1.3 in SSL_generate_key_block.

SSL_generate_key_block is specific to TLS 1.2. It will output garbage in
TLS 1.3 (wrong KDF), so fail instead.

Update-Note: SSL_generate_key_block gets a new error case, but callers
that hit this were getting back useless output anyway.

Change-Id: Ib35384f902e03cd4654d25b39ca1808c4d878c3d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54705
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
(cherry picked from commit 19d6ec9c439323c869e28f4e496329e55179f1ee)

* Make CBB_init_fixed infallible and allocationless.

Today, every use of CBB, even CBB_init_fixed, requires a small, fallible
allocation to allocate the top-level CBB's cbb_buffer_st. We could embed
cbb_buffer_st directly in CBB, but then every child CBB wastes that
space, and needs an extra pointer to point back to the cbb_buffer_st.

But top-level and child CBBs have disjoint representations anyway. We
share a cbb_buffer_st pointer, but it's owning in one case and
borrowed in another. Child CBBs have length prefix information, but it's
never filed in for a top-level CBB.

Make this a sum type, with is_child as the discriminator and a union for
the two structures. (Elsewhere I've been trying to get rid of unions,
but this isn't using unions for type-punning, so it should valid even in
C++. We never access inactive arms.)

The implementation gains a few more branches, but now CBB_init_fixed is
infallible and allocation-less. I'm hoping this will let us more freely
convert functions like UTF8_putc into CBB because we don't need to worry
about cleanup or introducing allocations.

Change-Id: If0b28cd9e079418f35d5a614058c0aa73658822e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54645
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
(cherry picked from commit 15ba28f839c828ced30178a86a9c56b104d3b5d7)

* Some CBB_init_fixed simplifications.

CBB_init_fixed callers no longer need to check the return value, or
handle any cleanup. The hpke.c instance was even already (incorrectly at
the time) assuming this.

Change-Id: I2f4cb124454fc7ba7ff6d2075d99f537a58c6c6b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54647
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(cherry picked from commit 254b8e113977aeb84ced559772537cd15bbe6cfc)

* Add support for arm/aarch64 on FreeBSD

Change-Id: Ib3495ddedec533b78884100ff2ff76f7370e7dc6
Bug: 505
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54105
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(cherry picked from commit d66bba912806b83350afec11a3104a7865d783aa)

* Test (and, for CSRs, fix) TBS cache invalidation on signing.

We didn't actually have a test that would have caught
openssl/openssl#19388. This fixes this by
further generalizing the signing tests to run through all combinations
of {new object, reused object} x {X509_sign, X509_set_signature_value}.

In doing so, align X509_REQ_sign and X509_REQ_sign_ctx, which were
missing the TBS invalidation.

Change-Id: I5028aa2a00e71da0ebc7a03b23823b1337a56fca
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54726
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
(cherry picked from commit da96eeb958ee8442f408a5e8fa78cbfb86cc3281)

* Fix comments around TBSCertificate cache.

We don't actually refresh the cache most of the time, just drop it and
live without it. The distinction isn't really visible by callers, but
make the comments accurate.

Change-Id: I7321695337125ca648ab57667564d9578a6fd549
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54727
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
(cherry picked from commit 818c4aafa86fa3922f969220473a27c163214b73)

* Also detect Armv8.2 SHA-512 extensions on FreeBSD.

A small follow-up to
https:/
8000
/boringssl-review.googlesource.com/c/boringssl/+/54105, to bring
it up to feature parity with the other aarch64 backends.
ID_AA64ISAR0_SHA2_512 seems to be present in FreeBSD 12.0, so I don't
believe this needs any compatibility ifdefs.

Bug: 505
Change-Id: I44891cf635adfd2ae26d4113fdc910549cf89193
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54725
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Robert Clausecker <fuzxxl@gmail.com>
Reviewed-by: Bob Beck <bbe@google.com>
(cherry picked from commit 9cd85d0b4c3ea8fc0e1a1883bab3a46ffaa18bd9)

* Further fixups on the new tests.

I messed up the indentation in one place, and Shane Lontis pointed out
that the comment is slightly out of date now that there are two
codepaths involved.

Change-Id: I1be69f3f9a3835fffc4801b4464b9fb8ecb092cc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54745
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
(cherry picked from commit ca6fa61cdb1cd5c6402fd41908479bbbf1b4566b)

* Add EC scalar mul functions to bssl speed (#660)

Adding lower level EC scalar multiplication operations to bssl speed tool: mul, mul_base, mul_public.

* Add SSHKDF to ACVP Tool (#668)

* prepare kdf-components.go for both tls and ssh

* update kdf-components.go to include logic for handling sshkdf

* add sshkdf to acvp tool

* fix bug where no result is produced for TLS-v1.2

* add SSH KDF to acvp test suite

* update spacing in code

* reorganize subprocess.go

* Revert "rename tlskdf.go to kdf-components.go to more accurately reflect NIST test vector naming scheme"

Also updated variable names to resolve conflicts.

This reverts commit ce0e884.

* rename tlskdf.go to kdf-components.go to more accurately reflect NIST test vector naming scheme (#666)

* move magic numbers into variables

* add more elegant error handling

* remove duplicated variables from kdf-components.go

* Rust - rename lib to avoid link conflicts (#669)

* Remove pthread_atfork fork detection mechanism (#673)

#142 added pthread_atfork() as a secondary fork detection mechanism. However, we have observed several issues consuming AWS-LC artifacts due to the symbol pthread_atfork being handled differently in distributions (and in a non-portable way). This affects our ability to distribute AWS-LC to our intended places. See internal document for details.

Therefore, completely remove pthread_atfork() from the code-base.

* Update run_analytics.sh to skip ARM FIPS Static builds with GCC, add tracking reference to run_fips_tests.sh (#678)

* Import the source code of Kyber768 (#679)

Import the source code of Kyber768.

The source code was taken from the primary source of Kyber
at [link](https://github.com/pq-crystals/kyber),
at [commit](https://github.com/pq-crystals/kyber/tree/faf5c3fe33e0b61c7c8a7888dd862bf5def17ad2),
as of September 13th 2021.

* Switch RSA_sign to size_t.

While I'm here, use a fixed-size uint64_t in RSA_generate_key, rather
than unsigned long. This code also assumes unsigned long fits in
BN_ULONG, which is probably true on all platforms we care about, but
unnecessarily fussy.

The RSA_sign -> RSA_METHOD transition does require a cast. Go ahead and
check length/hash_nid consistency so we know it fits in the cast. This
does mean RSA_METHOD-backed keys are restricted to implementing digests
that we support, but that's probably fine. If anything, I think we
should try to shift away from RSA_METHOD as a story for custom keys.

Bug: 516
Change-Id: I3969da67d1daeff882279a534eb48ca831eb16cd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54465
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
(cherry picked from commit 58f728eaecf9d1152df4977e50f90375e05c486d)

* Enable -Wstring-concatenation and silence warning.

Newer versions of Clang have a warning to detect "suspicious" uses of
string concatenation, where they think a comma or so was missing. It
flags a false positive in x509_test.cc, which we can silence with
parentheses. Fuchsia builds with this warning enabled, so enable it to
catch future instances.

I couldn't find official documentation on when this was added, but
empirically it's in my clang-12 but not my clang-11. That's recent
enough that adding a version check seems prudent. Unfortunately,
version-detecting Clang is complex because AppleClang uses completely
different versions. There's a handy table on Wikipedia that maps them.

Change-Id: I503c21d39bb5c68dda9bda6da693c7208f3af561
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54785
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(cherry picked from commit 7d1fc2b014a940e4e9ecd4660146c5cc2accad9b)

* Remove CMake install command for modulewrapper.

I think it never picked up the fix in
https://boringssl-review.googlesource.com/c/boringssl/+/52345 for older
CMakes, but it doesn't have much reason to be part of the install in the
first place.

Bug: 524
Change-Id: Ifbb898b1e4686194c85e9902ee3d59d83b55b78a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54786
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(cherry picked from commit 14aa0de18f638a92be13597bc1b8a95ca8fcf8a4)

* Break FIPS tests by zeroing out the entire value.

Previously the code just flipped one bit. But, empirically, modern Clang
will sometimes produce code that doesn't depend on the first 16 bytes of
the data; they are encoded in the instructions instead. Thus zero out
the full value.

(If Clang ever starts embedding complete values into the instruction
stream then we're going to have to do something more complex. Self tests
are a bit funny: the compiler could reasonably optimise them away
completely given that it sees all the inputs. Perhaps the inputs would
have to be moved into a different object file.)

Change-Id: I7bfb18cb7868def67fc791dcc31c5915c7728ac4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54825
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
(cherry picked from commit 1eea82a92a141416b3f0ee4d52434252cb948b47)

* Fix linking with non-MSVC toolchain in Windows platform

This adds the link libraries in CMakeLists.txt file. If the libraries
are not in CMake files linking failed with undefined reference error.

Change-Id: I8f8352f6149a6332eedc0be51f36634890e3db60
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54805
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
(cherry picked from commit b819f7e9392d25db6705a6bd3c92be3bb91775e2)

* Miscellaneous -Wshorten-64-to-32 fixes.

Bug: 516
Change-Id: Iba2014da414658c08e42e0993912fa73848832d3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54945
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(cherry picked from commit 9d64d8d2373db494d3460102b970f4aaf92ee274)

* Clean up short BIGNUM handling in bn_print.

We shouldn't print different things depending on sizeof(long).

Change-Id: I5f97e17b838f8c9b119421b9ce0e93e95bd33dc0
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54946
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
(cherry picked from commit 7deb8314625b40aea2e0c4fdcf9784d18045e397)

* Add tests for EVP_PKEY_print_*

These are mostly to ensure they don't crash, and that subsequent changes
don't unintentionally change the output. The current output is a little
weird but, for now, I've just captured the current output, bugs and all.

Change-Id: I9f1a4910ccc717764ef44551de9b3e0f9f2a1b40
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54947
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(cherry picked from commit 1ec335da79eaa8341308acd9dc804078b8593c84)

* Simplify crypto/evp/print.c.

First, stop trying to pre-size the buffer and just have bn_print
allocate the buffer internally. That removes the need for all the
algorithms being two-pass.

While I'm here, stop passing the unused ASN1_PCTX parameters in
everywhere.

As a side effect, this fixes a int vs size_t instance that flagged
-Wshorten-64-32, but it ended up being a much more substantial change.

Bug: 516
Change-Id: Ic210604de85539559b1ed88889ca6a08dfb20bde
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54948
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
(cherry picked from commit 3592aa3009eb883f7e5431ad7ce08dc3ae979902)

* Update CMakeLists.txt to support CMake version 3.0. VERSION_GREATER_EQUAL was added in 3.7, negate the expression and use the widely supported VERSION_LESS

* Add support for 8192 bit RSA key in speed tool (#680)

Speed tool has support for measuring various RSA operations using 2048 and 4096 bit RSA keys. In terms of bit-security, these can't be compared to all corresponding elliptic curves we support; e.g. p256 is somewhat equivalent to 4096 bit RSA keys, hence no comparison for p384 and/or p512.

To compare performance of RSA against, for example, ECDSA using p384 a 8192 bit RSA key is required. This also shows how RSA scales past 4096 bit keys (and shows why we don't need to measure performance for larger RSA keys...).

Embed such an 8192 bit RSA key and wire it up in the speed tool.

* Improve Rust build/publish (#681)

* Improve Rust build/publish

* Add comment on crate generation

* Add comment on crate publishing

Co-authored-by: William Bo Yang <coolbillyang@gmail.com>
Co-authored-by: David Benjamin <davidben@google.com>
Co-authored-by: Sergey A. Osokin <osa@FreeBSD.org.ru>
Co-authored-by: Nevine Ebeid <66388554+nebeid@users.noreply.github.com>
Co-authored-by: dkostic <25055813+dkostic@users.noreply.github.com>
Co-authored-by: Justin W Smith <103147162+justsmth@users.noreply.github.com>
Co-authored-by: torben-hansen <50673096+torben-hansen@users.noreply.github.com>
Co-authored-by: Andrew Hopkins <andhop@amazon.com>
Co-authored-by: Adam Langley <agl@imperialviolet.org>
Co-authored-by: Biswapriyo Nath <nathbappai@gmail.com>
torben-hansen pushed a commit to torben-hansen/aws-lc that referenced this pull request Sep 18, 2024
@jargh
Merge pull request aws#142 from aqjune-aws/p384-opt
2456f73 
Adopt the Arm SIMD-optimized p384 fields to point operations
s2n-bignum original commit: awslabs/s2n-bignum@6248d16
torben-hansen pushed a commit to torben-hansen/aws-lc that referenced this pull request Sep 18, 2024
@jargh
Merge pull request aws#142 from aqjune-aws/p384-opt
8ed6003 
Adopt the Arm SIMD-optimized p384 fields to point operations
s2n-bignum original commit: awslabs/s2n-bignum@6248d16

s2n-bignum original commit: awslabs/s2n-bignum@2456f73
torben-hansen pushed a commit to torben-hansen/aws-lc that referenced this pull request Sep 18, 2024
@jargh
Merge pull request aws#142 from aqjune-aws/p384-opt
3713dca 
Adopt the Arm SIMD-optimized p384 fields to point operations
s2n-bignum original commit: awslabs/s2n-bignum@6248d16
torben-hansen pushed a commit to torben-hansen/aws-lc that referenced this pull request Sep 19, 2024
@jargh
Merge pull request aws#142 from aqjune-aws/p384-opt
3703e2b 
Adopt the Arm SIMD-optimized p384 fields to point operations
s2n-bignum original commit: awslabs/s2n-bignum@6248d16
dkostic pushed a commit to dkostic/aws-lc that referenced this pull request Dec 5, 2024
@jargh
Merge pull request aws#142 from aqjune-aws/p384-opt
6248d16 
Adopt the Arm SIMD-optimized p384 fields to point operations
dkostic pushed a commit to dkostic/aws-lc that referenced this pull request Dec 10, 2024
@jargh
Merge pull request aws#142 from aqjune-aws/p384-opt
8963843 
Adopt the Arm SIMD-optimized p384 fields to point operations
s2n-bignum original commit: awslabs/s2n-bignum@6248d16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dkostic dkostic dkostic approved these changes

@nebeid nebeid nebeid approved these changes

@andrewhop andrewhop Awaiting requested review from andrewhop

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@torben-hansen @dkostic @nebeid
0