EVP_PKEY_assign_RSA, at least in C, is more type-safe. It also is
compatible with an upcoming BoringSSL change to make the RSA struct
opaque. For some Swift reasons I don't fully understand (but relating to
the OpaquePointer mess), when RSA becomes opaque, EVP_PKEY_assign no
longer works.
I assume it could be made to work with some appropriate cast, but
since EVP_PKEY_assign_RSA already exists (and will, in the future, be
more binary-size-friendly), just use that.
Motivation:
A few of cert the verification tests are failing on macOS.
Modifications:
Fetched a newer leaf cert by running 'openssl s_client -connect apple.com:443 -showcerts'
Result:
Tests pass.
After your change, what will change.
* Drop Swift 5.5
This patch drops Swift 5.5 support. It removes docker-compose files and
deletes any conditional compilation guards that are no longer necessary.
* Remove test generation script
Motivation:
#fileID introduced in Swift 5.3, so no longer need to use #file anywhere
Modifications:
Changed #file to #filePath or #fileID depending on situation
Motivation:
Lock has been deprecated in favour of NIOLock. Warnings aren't great.
Modifications:
- Replace Lock with NIOLock
- Update the minimum required NIO version to 2.42.0.
Result:
Everything builds cleanly
Motivation:
The giant write test requires an enormous commitment of RAM to run it.
This has caused problems for both our own CI and a number of downstream
CI systems, leading to long execution times or outright failures.
Unfortunately, we can't defend against the original failure using this
test suite, but we can at least exercise the new machinery we added and
trust that the configuration is right.
Modifications:
- Make the maximum write size configurable internally
- Test the maximum write size at a different value.
Result:
We feel confident that the write splitting continues to work correctly.
* Adopt `Sendable`
* `@preconcurrency typealias` doesn’t work
* `NIOPSKClient/ServerIdentityCallback`
* `NIOSSLKeyLogCallback`
* `NIOSSLClientTLSProvider`
* Fix warnings in tests
* `@preconcurrency` is broken https://github.com/apple/swift/issues/60508
I think we don’t actually need it in that case and still don’t break API
* Fix warnigns in tets
* fix nightly
* Add file path and line number info to BoringSSL error wrappers
* Rename filename to filepath
* Renamed boringSSLErrorCode to boringSSLErrorInfo
* Store line number as Uint
* Change Int32 to Int
Motivation:
The CA name tests have some complex certificate requirements, so they
have hardcoded certs. Those have expired, making the unit tests fail.
Modifications:
- Replace the certs with new ones.
Result:
The tests pass again.
* Invoke `NIOSSLContext.AdditionalCertificateChainVerificationCallback` with peer certificate and make it public without guaranteeing API stability
* move function typealias to the module namespace and improve documentation
* Move `additionalPeerCertificateVerificationCallback` from `NIOSSLContext` to `NIOSSHandler`
* make peer certificate non-optional in user callback
* Update Sources/NIOSSL/NIOSSLHandler.swift
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
Add TLS-PSK Support to address #230.
Modifications:
Modified SSLContext to take a passed in PSK callback to perform key and identity operations during the handshake.
Result:
TLS-PSK support for TLS 1.2. TLS 1.3 is not addressed due to API support in BoringSSL.
Motivation:
Unnecessary empty writes should be avoided, and we should avoid
completing promises too often.
Modifications:
- Avoid sending a split promise on every write when writes are too big.
- Avoid sending a zero-length write when writes are too big.
Result:
Slightly better write behaviour.
Note that this is fundamentally un-testable: there's no place to put a
test hook here, and we coalesce writes on the far side, so we can't do
much here.
Motivation:
TLSConfiguration cannot be made equatable due to its use of closures, so
we have our own best-effort functions. Unfortunately, I forgot to make
sure everything was added to them, so I'm doing so now.
Modifications:
- Add missing fields to bestEffort functions.
Result:
Better bestEffort functions.
Motivation:
In rare cases, users may attempt to write more data than can be
expressed in 2**31 bytes. This should not crash our program.
Modifications:
- Break up extremely long writes into smaller chunks
- Make sure to clamp on the read path too
Result:
Users who send hilariously long writes can do so.
# Motivation
We missed unbuffering reads after the handshake completed. This could result in reads that were buffered while the handshake was in progress to not unbuffer.
# Modification
This PR unbuffers any reads after the handshake is completed.
# Result
We are now unbuffering reads correctly.
* Include file path in `IOError`s thrown by `fopen`
* change error reason formatting and also include mode
* use explicit `String` to `UnsafePointer<CChar>` transformation
* Port SecureBytes from Swift Crypto to use with TLS-PSK
Motivation:
Porting SecureBytes to NIOSSLSecureBytes for using in (Add TLS-PSK Support #360).
Modifications:
Addition and modification of SecureBytes to work within SwiftNIO SSL.
Added additional test cases.
Result:
NIOSSLSecureBytes will be ported for usage in a later PR with TLS-PSK.
* secure-bytes-transfer: Added LinuxMain tests
* secure-bytes-transfer: Removed case from NIOSSLError
* secure-bytes-transfer: Addressed initial feedback
* secure-bytes-transfer: Removed NIOSSLSecureBytesError from SSLErrors
* secure-bytes-transfer: Remove public access from NIOSSLSecureBytes
* Remove remaining public declaration.
Co-authored-by: meaton2 <meaton3@apple.com>
Co-authored-by: Cory Benfield <lukasa@apple.com>
Motivation:
Address the issue the race condition brought up in #367
Modifications:
Created directories based on the test name so that each file would be isolated.
Result:
Avoid race condition in parallel test.
* support additional certificate validation
* run `scripts/generate_linux_tests.rb`
* fix review comments
- rename `CertificateChainVerificationCallback` to `AdditionalCertificateChainVerificationCallback`
- switch over `result` instead of try/catch
- add `. eofDuringAdditionalCertficiateChainValidation` error and throw it if channel becomes inactive while in `.additionalVerification` state
* add `Channel` parameter to `AdditionalCertificateChainVerificationCallback`
* get `context` from self instead of captering it
* save progress
* Only flush after handshake
* run `./scripts/generate_linux_tests.rb`
`
* Add NIOSSLCertficiateExtensions
* adopt naming of ObjectIdentifier
* address review comments
- namespace top level types with `NIO`
- add `withUnsafeBytes(_:)` that gives access to the buffer through an untyped buffer pointer
- fix indentation
* use real optional for `reference` in `_NIOSSLCertificateExtensions`
* fix review comments part 2
- nest `_Extensions` and `_Extension` under `NIOSSLCertificate`
- explain optional reference
- remove empty and copy constructor
- replace fancy force-unwraps with `!`
* use channel from argument
* fix review comments
- buffer the flush instead of ignoring it
- do not automatically flush after a succesfull handshake
* Add NIOSSLCertficiateExtensions
* adopt naming of ObjectIdentifier
* address review comments
- namespace top level types with `NIO`
- add `withUnsafeBytes(_:)` that gives access to the buffer through an untyped buffer pointer
- fix indentation
* use real optional for `reference` in `_NIOSSLCertificateExtensions`
* fix review comments part 2
- nest `_Extensions` and `_Extension` under `NIOSSLCertificate`
- explain optional reference
- remove empty and copy constructor
- replace fancy force-unwraps with `!`
* support additional certificate validation
* run `scripts/generate_linux_tests.rb`
* fix review comments
- rename `CertificateChainVerificationCallback` to `AdditionalCertificateChainVerificationCallback`
- switch over `result` instead of try/catch
- add `. eofDuringAdditionalCertficiateChainValidation` error and throw it if channel becomes inactive while in `.additionalVerification` state
* add `Channel` parameter to `AdditionalCertificateChainVerificationCallback`
* get `context` from self instead of captering it
Co-authored-by: Cory Benfield <lukasa@apple.com>
* make `_SubjectAlternativeNames` and related types public
and add `RandomAccessCollection` conformance to `_SubjectAlternativeNames`
* move type conversion out of collection
* add `_SubjectAlternativeName`, `. NameType` and `. Contents`
* remove new error case
* add email as a supported name type
* fix review comments and move _SubjectAlternativeNames and related types to a separate file
* remove unused error code
* fix review comments
* add missing Glibc import for `in_addr` and related functionality
* add bounds check and use load instead of memoryRebound
* Enhancement to address issue-337 to query the final `TLSVersion`
Motivation:
To provide an enhancment to resolve issue-337 to query the final `TSLVersion`.
This allows a client channel to grab the final TLSVersion used for the connection.
Modifications:
Added extension points to allow querying the `TLSVersion` from the `SSLConnection` based on the `Channel` and `ChannelPipeline`.
Added an `unknown` type to `TLSVersion` to account for a default case where the `TLSVersion` cannot be derived from the conenction.
Added a unit test `testObtainingTLSVersionOnClientChannel` to query the final `TLSVersion` when the client channel is closed.
Result:
2 APIs added to query the `TLSVersion` when on a channel.
Additional Notes:
Note that I have 2 concerns about this PR that I would like some additional feedback on:
1. First is the `unknown` case for `TLSVersion`. This may not be needed as BoringSSL may just return `tlsv12` each time the version cannot be derived from the connection. Let me know your thoughts.
2. I am not wild about the way this is being unit-tested, but I wanted to do it this way because it provides the `TLSVersion` on the `Channel` after it is closed. Let me know your thoughts.
* Removed unknown case as it cases an API breakage.
Motivation:
Removed unknown case as it cases an API breakage.
Modifications:
Defaulted to tlsv12 as this is what BoringSSL will default to.
Result:
Wiped out unknown and switched to tlsv12 as the default
* issue--337: Fix failure on soundness.sh for Linux test
* issue-337: Addressed feedback from review
* issue-337: Updated PR to address feedback
* issue-337: Updated the EventLoopFuture for the Channel extension
* issue-337: Refactored test case
Co-authored-by: meaton2 <meaton3@apple.com>
Motivation:
When we get channelInactive during a handshake, we currently produce a
fairly useless error:
.handshakeFailed(NIOSSL.BoringSSLError.sslError([])). This error isn't
helpful to anyone, and it doesn't really communicate what's happened.
Sadly, due to the tightly constrained types it's really hard for us to
add new errors to handshakeFailed. Regardless, it would be good for us
to give as much diagnostic information as we can here.
Modifications:
- Changed the representation of BoringSSLInternalError to allow
"synthetic" errors that don't come from BoringSSL at all.
- Added a new synthetic error for eof during handshake.
- Sent that error instead of a BoringSSL error stack that will always be
empty.
Result:
- Easier to diagnose the error.
Motivation:
When we added additional trust roots on Darwin, we didn't add much
testing to cover that use-case because testing the default validator is
hard. This meant a bug slipped through: we called a pair of APIs in the
wrong order. As a result, users using additional trust roots on Darwin
_only_ trusted the additional ones: the defaults were unavailable.
Modifications:
- Fix the call so that we trust both the defaults and the additional
roots.
- Refactor the security framework validator to make it possible to test
it directly.
- Add unit tests for the security framework validator.
Result:
We'll correctly trust the additional roots and the defaults on Darwin.
* issue-316: Support c_rehash Directory Format
* issue-316: Address Feedback from PR
* issue-316: Addressed feedback
* issue-316: Addressed latest feedback
* issue-316: Refactor rehash check to be String.UTF8View:
* issue-316: Added split back into the String.UTF8View computational process
* issue-316: Added another split back to the computation of the rehash check
* issue-316: Added additional checks to rehash format
Co-authored-by: meaton2 <meaton3@apple.com>
Motivation:
In some rare cases we've seen production applications crash due to us
getting a zeroReturnError in either closed or perhaps unwrapped. This is
pretty hard to get to trigger in realistic situations, but I've found a
contrived-but-possible way to hit it in tests. In both of these cases we
are able to simply ignore the error and move on.
Modifications:
- Added a test to replicate the issue
- Tolerate .unwrapped and .closed on zeroReturnError.
Result:
Fewer weird crashes
Motivation:
NIOSSL traditionally handles private keys be requiring that they can be
decoded into a BoringSSL EVP_PKEY structure. This requires the key to
have an in-memory representation that BoringSSL understands, and
ultimately forces the bytes of the private key into memory.
In many cases, however, this is either undesirable or not possible.
For security reasons it is common to want to offload a private key into
secure storage, such as the iPhone's Secure Enclave or a portable smart
card. In other circumstances it may be useful to put the private key on
an entirely different machine and have the signing operations occur over
an RPC mechanism.
In all of these cases it is necessary to have a programmatic interface
to the private key that can be implemented in an essentially arbitrary
manner.
Modifications:
- Defined a new protocol, `NIOSSLCustomPrivateKey`, that allows users to
implement the private key operations needed for TLS.
- Plumbed support for this interface through `NIOSSLPrivateKey`.
- Added support for the BoringSSL interface to `SSLConnection`.
Result:
Custom TLS private keys backed by function calls can now be provided to
NIOSSL.
Motivation:
The recently-added CA name tests consistently fail on Darwin machines.
This is becuase they expect the handshake to fail due to certificate
verification errors, but that doesn't happen synchronously on Darwin.
This also introduces thread-unsafety into the tests. Neither is good.
Modifications:
- Modify the test configuration to use custom trust roots, which
disables the Security.framework callbacks
- Modify the test configuration to disable hostname verification.
Result:
These tests now successfully perform their handshake on all platforms.
Motivation:
When we added implementationOnly imports a while back, we supported
Swift 5.0, and so we needed to guard their availability behind a
compiler check. We only support 5.2 and later now, so that check is
redundant and can go away.
Modifications:
- Removed compiler checks around imports of our C libraries.
Result:
Cleaner code
Motivation:
With NIO 2.32.0 we broke the core NIO module up into modules that split
apart the POSIX layer and the core abstractions. As a result, this
package no longer needs to express a hard dependency on the POSIX layer.
Modifications:
- Rewrote imports of NIO to NIOCore.
- Added NIOEmbedded and NIOPosix imports where necessary in tests.
- Extended soundness script to detect NIO imports.
- Note that the main modules still depend on NIO, which is necessary
for backwards-compatibility reasons. This dependency is unused.
Result:
No need to use NIOPosix.
Motivation:
Resolving build failures for the nightly build suite on each commit.
Modifications:
Modification to ClientSNITests.swift.
Result:
Surpressing the error on the nightly build.
Co-authored-by: meaton2 <meaton3@apple.com>
* Update BoringSSL to 25773430c07075a368416c3646fa4b07daf4968a
* Clean up the use of the X509 pointer
Motivation:
BoringSSL has begun opaquifying their X.509 library. This unfortunately
manifests as source code changes in Swift due to the way OpaquePointer
behaves. As a result, we need to change the places that held an X.509
pointer directly.
While we're here, we've been playing a bit fast-and-loose with the
lifetime of owned pointers. We should take this opportunity to clean
things up and be a bit better behaved.
Modifications:
- Replace the X509 pointers with OpaquePointer
- Use scoped access to the X.509 access outside the NIOSSLCertificate
class.
Result:
Code compiles with better pointer hygeine.
Motivation:
This is an enhancement to add support for send CA names during the handshake for #231.
Modifications:
Changes to:
TLSConfiguration.swift: Added the `sendCANameList` flag to indicate loading the CA names on `.fullVerification`.
SSLContext.swift: Added the CA Names derived from the `trustRoots`.
TLSConfigurationTests.swift: Added the test case for `testFullVerificationWithCANames`.
NIOSSLTestHelpers.swift: Added a new set of PEMs for TLS and client authentication testing.
Result:
Addition for #231.
Motivation:
Run the tests on Android.
Modifications:
- Enable the tests on Android.
- Replace a few uses of /tmp with FileManager.default.temporaryDirectory in the
Swift tests.
Result:
All the tests pass on Android AArch64.
Motivation:
Currently whenever we add new config to TLSConfiguration, we add a new
static factory function that needs to provide the field. This is because
we currently allow configuration primarily by way of initializers. This
is increasingly not scaling, leading to a proliferation of almost
identical static factory functions.
We can replace this proliferation by having only "default" constructions
that default basically everything, and then having users use
getters/setters to initialize things. This also works well when we have
multiple ways of interacting with config, e.g. with the various cipher
suite operations.
Modifications:
- Deprecate all existing factory functions.
- Replace with two: forClient() and
forServer(certificateChain:privateKey:). These configure only the
mandatory things for each mode.
- Replace all uses of the deprecated initializers with the new ones,
configuring fields manually as needed.
Result:
More consistent configuration objects.
Motivation:
The new hashable implementation was missing some fields. I've added
them, and added testing. It also needed to raise the minimum NIO
version.
Modifications:
- Raised the minimum NIO version.
- Fixed the comparison.
- Added fields.
Result:
The code is right.
* Rebase: Added API for setting cipher suites in TLSConfiguration
Motivation:
Issue-207 for adding an API for setting a cipher suite while also preserving the string based declaration.
Additions to TLSConfiguration and SSLContext to allow safe access to ciphers on a context.
Modifications:
Additions to TLSConfiguration and SSLContext to allow safe access to ciphers on a context.
Added test cases in TLSConfigurationTest.
Result:
Additional API for Issue-207.
Simplified and safer access to STACK_OF(SSL_CIPHER) on a context.
* Addressed all feedback expect for memberwise init.
Motivation:
Addressed all feedback from previous feedback except for memberwise init.
Modifications:
Changes to SSLContext and TLS Configuration.
Result:
Resolved all feedback except for memberwise init.
* Addressed duplicating the memberwise init.
Motivation:
Addressing feedback for memberwise init.
Modifications:
Changes to TLSConfiguration init.
Result:
Reduced code surface.
* Added additional cipher suite tests
Motivation:
Added additional cipher suites tests per feedback.
Modifications:
Added tests to TLSConfigurationTests.
Result:
Additional cipher suite tests.
Co-authored-by: Johannes Weiss <johannesweiss@apple.com>
Motivation:
To address #253
Modifications:
When channelInactive is called during a handshake failure it currently defaults to uncleanShutdown.
This bugfix and test is to report handshakeFailed instead of uncleanShutdown.
Added the handshaking case to the channelInactive function to report handshakeFailed.
Added a new test case, testChannelInactiveDuringHandshake, in UnwrappingTests to test this fix.
Result:
Fix of #253
David Benjamin