This commit also addresses the side effect that additional messages have
an influence on the derivation of the application traffic secrets. Therefore,
key derivation is relocated after the server finished message has been sent,
so the additional messages from the client (Certificate, CertificateVerify)
don't affect the key derivation. Only the outbound key is switched there, the
inbound key remains in use until the client's finished message has been
processed.
This change mainly affects legacy TLS versions because TLS 1.3
connections are terminated by the server once the peer does not send a
CertificateVerify message next to its empty Certificate message.
The cached traffic secrets change once the application traffic secrets
are derived, but we must always use the correct base key to derive the
finished message, which are the handshake traffic secrets (RFC 8446,
section 4.4).
This won't work if the client doesn't send a `signature_algorithms`
extension. But since the default is SHA1/RSA, most will send it to at
least announce stronger hash algorithms if not ECDSA.
strongSwan supports RSA_PSS_RSAE schemes for signing but does not
differentiate between rsaEncryption and rsassaPss encoding. Thus
RSA_PSS_PSS schemes are only used for verifying signatures.
A client can send one or multiple key shares from which the server picks
one it supports (checked in its preferred order). A retry is requested if
none of the key shares are supported.
Adds support to request and handle retries with a different DH group.
Only the first key share extension sent by the client is currently
considered, so this might result in protocol errors if the server requests
a group for which the client already sent a key share.
List building also added an additional length field which is required by
client-side TLS extensions but not for server-side certificate request
extension. Now the method only returns a list of supported signature
algorithms and the implementation is responsible to add additional
length fields.
Fixes: 07f826af67 ("Fixed encoding of TLS extensions (elliptic_curves and signature_algorithms)")
- TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256
- TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256
- TLS_DHE_RSA_WITH_CHACHA20_POLY1305_SHA256
- Encrypt then MAC
- Extended master secret
- Session ticket
and also add missing suites in the unit test.
If we don't do this, we might negotiate a TLS version for which we don't
have any suites configured, so that the cipher suite negotiation
subsequently fails.
Only suggest TLS versions of supported cipher suites. For instance, do not
suggest TLS 1.3 if none of its cipher suites (requiring GCM/CCM or
ChaPoly) are available.
Even though we return from build(), we are not actually sending a response,
so we can't return NEED_MORE (would send an invalid ClientHello message) and
if we return SUCCESS, the EAP layer treats this as failure (there is a comment
in eap_authenticator_t about client methods never returning SUCCESS from
process()). Instead we return INVALID_STATE, which allows tls_t.build() to
exit from the build() loop immediately and send the already generated Finished
message.
We generate material for both MSK and EMSK even though we only need the
former. Because HKDF-Expand-Label(), on which the export functionality
is based, encodes the requested key length, we have to allocate the same
number of bytes as e.g. FreeRADIUS does (i.e. if we only request 64
bytes, those won't be the same as the first 64 bytes after requesting
128 bytes).
Unfortunately, key derivation for TLS-based methods is currently not
standardized for TLS 1.3. There is a draft [1], which defines a scheme
that's different from previous versions (instead of individual label
strings it uses a single one and passes the EAP type/code as context
value to TLS-Export()). The current code is compatible to FreeRADIUS
3.0.x, which doesn't implement it according to that draft yet (there are
unreleased changes for EAP-TLS, not for the other methods, but these only
switch the label, no context value is passed). In a separate draft
for EAP-TLS [2] there is an altogether different scheme defined in the
latest version (label combined with EAP method, no context and separate
derivation for MSK and EMSK).
So this is a mess and we will have to change this later with the inevitable
compatibility issues (we should definitely disable TLS 1.3 by default).
[1] https://tools.ietf.org/html/draft-ietf-emu-tls-eap-types
[2] https://tools.ietf.org/html/draft-ietf-emu-eap-tls13
PKCS#1 v1.5 signatures are not defined for use with TLS 1.3 (they can
only appear in certificates, we now send a signature_algorithms_cert
extension to indicate support for them). So for RSA certificates, we
must support RSA-PSS signatures.
There are two sets of schemes, that are differentiated by the type of
RSA key used for the signature, one is for classic RSA keys (rsaEncryption
OID), which can also be used with PKCS#1 when using TLS 1.2, the other
is for RSA-PSS keys (RSASSA-PSS OID), which are not yet commonly
used (and can't be generated by our pki tool). According to the RFC,
PSS must also be supported for TLS 1.2 if the schemes are included in
the signature_algorithms extension (e.g. OpenSSL does not use PKCS#1 v1.5
anymore if PSS is proposed).
This changes how these schemes are stored and enumerated (they are not
treated as combination of hash algo and key type anymore).
Legacy schemes (MD5/SHA-1) are removed.
With the previous code, there was an issue when replying to TLS 1.3
post-handshake messages. In this case, SUCCESS is eventually returned
from build(), however, no actual data has been received so in_done is 0.
This was interpreted as EOF, plain_eof was set to TRUE and no further data
was read from the socket afterwards.
Returning SUCCESS from build() if the exchange is initiated by
write_(), as is the case with the finished reply, never was a problem
because there the return value of 0 is not interpreted as EOF.
If data is processed that eventually includes a TLS close notify, build()
will fail after a close notify has been sent in turn. However, propagating
that error immediately when reading prevented ever returning the data
already processed before the close notify was received.
Note that this breaks connecting to many TLS 1.3 servers until we support
HelloRetryRequest as we now send a key_share for ECP_256 while still
proposing other groups, so many servers request to use CURVE_25519.
These DH groups don't use the point format prefix (RFC 8422 deprecated
any other format anyway). Since they are enumerated now, they can also
be used by servers for TLS 1.2.
There is no point proposing legacy (or future) cipher suites depending on
the proposed TLS versions. It was actually possible to negotiate and use
cipher suites only defined for TLS 1.2 with earlier TLS versions.
The code is a minimal handshake with the HelloRetryRequest message
implementation missing.
Can be tested with an OpenSSL server running TLS 1.3. The server must
be at least version 1.1.1 (September 2018).
Co-authored-by: ryru <pascal.knecht@hsr.ch>
TLS 1.3 uses HMAC-based Extract-and-Expand Key Derivation Function (HKDF)
as defined in RFC 5869 to compute traffic secrets.
Co-authored-by: bytinbit <meline.sieber@hsr.ch>
The key material, in particular the nonce/IV, is derived differently and
the IV is also generated in a different way. Additionally, the actual
content type is encrypted and there may be optional padding to mask the
actual size of the encrypted data.
If we pass a pointer to NULL, the memory allocated by OpenSSL has to be
freed with OPENSSL_free(). Otherwise, this can lead to random
crashes/freezes for Windows builds as seen on AppVeyor. To not
complicate things for callers of this macro, we allocate our own memory,
which we already do for other i2d_* calls.
If no valid key is configured (e.g. because it's inadvertently uninitialized),
we should not just reuse the previous key.
The `key_set` flag is not necessary anymore because a non-NULL key is set
during initialization since 6b347d5232 ("openssl: Ensure underlying hash
algorithm is available during HMAC init").
If the peer deletes the CHILD_SA, we recreate it due to the close
action. However, if we create a new TUN device, we do so with a new
VpnService.Builder object and on that the DNS servers were never applied.
The latter happened only on the fly in the attribute handler when an
IKE_SA was established. Now we do this explicitly when creating the TUN
device, like the virtual IPs and routes. While we could avoid the
recreation of the TUN device if the CHILD_SA is recreated, there is the
theoretical possibility that the remote traffic selectors change. This
way we also avoid adding stuff to the builder in different places.
Fixes#3637.
This is not an error (as reflected by the returned status code) so we
should not print to stderr as output there might still be considered an
error (or at least an audit-worthy event) by some scripts.
In some cases, the algorithms that have been compiled into a plugin have
to be disabled at runtime. Based on the array returned by the get_features()
function the optionally provided function can strip algorithms or even
callbacks or registrations from a plugin, giving us a handy and powerful way
for runtime feature configuration aside from the plugin list.
Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
This adds helper functions to determine the first or last directory separator
in a string and to check if a given character is a separator.
Paths starting with a separator are now also considered absolute on
Windows as these are rooted at the current drive.
Note that it's fine to use DIRECTORY_SEPARATOR when combining strings as
Windows API calls accept both forward and backward slashes as separators.
Co-authored-by: Michał Skalski <mskalski@enigma.com.pl>
References #3684.
Apparently, we should use OPENSSL_free() to release memory allocated by
OpenSSL. While it generally maps to free() that's apparently not the
case on Windows, where the ECP test vectors caused `ACCESS_VIOLATION
exception` crashes (not always the same vector).
Fixes: 74e02ff5e6 ("openssl: Mainly use EVP interface for ECDH")
Functions like ECDH_compute_key() will be removed with OpenSSL 3 (which
will require additional changes as other functions will be deprecated or
removed too).
ECDH_compute_key() was not used because it only gives x-coordinate of
the result. However, the default setting, as per the errata mentioned,
is to use x-coordinate only.
Use ECDH_compute_key() for this setting as it additionally allows HW
offload of the computation using dynamic engine feature in OpenSSL.
EC_POINT_mul() doesn't allow HW offload.
Signed-off-by: Mahantesh Salimath <mahantesh@nvidia.com>
The previous approach would lead to additional zero prefixes in the
encoding of the serial (which is a positive integer, not an arbitrary
blob).
Fixes#3667.
To align with RFC 4519, section 2.31/32, the abbreviation for surname
is changed to "SN" that was previously used for serialNumber, which does
not have an abbreviation.
This mapping had its origins in the X.509 patch for FreeS/WAN that was
started in 2000. It was aligned with how OpenSSL did this in earlier
versions. However, there it was changed already in March 2002 (commit
ffbe98b7630d604263cfb1118c67ca2617a8e222) to make it compatible with
RFC 2256 (predecessor of RFC 4519).
Co-authored-by: Tobias Brunner <tobias@strongswan.org>
Closesstrongswan/strongswan#179.
Also assign online leases to a peer connecting from the same endpoint
when it requests any virtual IP. This is mainly a workaround for
Windows clients that remember the virtual IPv6 address and re-request it
the next time the connection is initiated (even if it is not a
reauthentication) but don't do the same for virtual IPv4 addresses.
This can result in duplicate policies with different reqids because
these are allocated for unique sets of traffic selectors.
Fixes#3541.
This allows more fine grained control over what's updated and does not
require multiple calls of the method. Plus we'll be able to use it in
the ike-mobike task.
Verify certificate chains starting from the root CA certificate and
moving towards the leaf/user certificate.
Also update TKM-RPC and TKM in testing scripts to version supporting the
reworked CC handling.
Load CA certificate id mapping from config and pass the correct CA ID to
TKM when checking certificate chains. The mapping of CA certificate to
CA ID is done via SHA-1 hash of the CA certificates subjectPublicKey.
Debian, Ubuntu, Fedora et. al. started to apply usrmerge to their
latest Linux distributions, i.e. /bin, /sbin, and /lib are now
symbolical links to /usr/bin, /usr/sbin, and /usr/lib, respectively.
Since executables and libraries are contained only once in Linux
packages (e.g. /bin/cp in coreutils but not /usr/bin/cp) this leads
to missing file measurments due to the symlinks when doing remote
attestation.
The new ita_attr_symlinks PA-TNC attribute fixes this problem by
collecting symbolic links pointing to directories on the client
platform.
Depending on from where bison is called, the file might not end up in
the same directory as the .y file, but the location of the Makefile.
This has been seen on FreeBSD.
There is a conflict between Flex's bison-bridge and Bison's api.prefix
options. Apparently, the former was added without consulting the Bison
devs and requires YYSTYPE, which is not added to the header anymore by
the latter. Instead, we just provide the proper definition of yyflex()
manually (as recommended by the Bison docs), so the option is not
required anymore.
DESTROY_IF() checks if the given value is not NULL, before calling
destroy() on it, which does not work for sub-structs. If
port_filter_attr is NULL, this could crash.
If we are already deleting the old/redundant CHILD_SA, we must not
migrate the child-create task as that would destroy the new CHILD_SA we
already moved to the IKE_SA.
Fixes#3644.
Without threads handling the resolution, there is no point waiting
for a reply. If no subsequent resolution successfully starts a
thread (there might not even be one), we'd wait indefinitely.
Fixes#3634.
The DN is otherwise not parsed until compared/printed. This avoids
false detections as ASN.1 DN if e.g. an email address starts with "0",
which is 0x30 = ASN.1 sequence tag, and the next character denotes
the exact length of the rest of the string (see the unit tests for an
example).
For apps targeting Android 10, where a method to change this was added, the
default changed so that all VPN connections are marked as metered. This means
certain background operations (e.g. syncing data) are not performed anymore
even when connected to a WiFi. By setting this to false, the metered state
of the VPN connection reflects that of the underlying networks.
None of our build environments seem to require these declarations. And
current versions of MinGW-w64 define them as inline functions in stdio.h
so these declarations clashed with that ("static declaration of '...'
follows non-static declaration").
If no callback is specified, terminate_ike_execute() is invoked without the
listener waiting on the IKE state change.
Now, if 'force' is false, then ike_sa->delete() just queues an
IKE_DELETE task, and returns SUCCESS - indicating successful task
manager initiation.
However, terminate_ike_execute() ignored this success and set the
status to FAILED.
This is not ideal, as it will be the overall return code of
terminate_ike(), although no failure did occur. This eventually leads
vici's "terminate" to return "Command failed: terminating SA failed",
as seen in this example:
In [9]: list(session.terminate({'ike-id': 2960, 'timeout': -1}))
---------------------------------------------------------------------------
CommandException Traceback (most recent call last)
<ipython-input-9-5f95b5cea88f> in <module>()
----> 1 list(session.terminate({'ike-id': 2960, 'timeout': -1}))
vici/session.pyc in streamed_request(self, command, event_stream_type, message)
136 raise CommandException(
137 "Command failed: {errmsg}".format(
--> 138 errmsg=command_response["errmsg"]
139 )
140 )
CommandException: Command failed: terminating SA failed
If we consider both queueing the task and actually destroying the IKS_SA
a success, we can just always return SUCCESS if we don't have a
callback. There is also no need to explicitly set the status to FAILED
if a listener is waiting as that's the default anyway.
Co-authored-by: Tobias Brunner <tobias@strongswan.org>
Closesstrongswan/strongswan#185.
In FIPS mode, libgcrypt uses a DRBG, which behaves differently when the
length passed to gcry_create_nonce() or gcry_randomize() is <= 0. It
expects a struct and explicitly checks that the passed pointer is not
NULL.
This ensures that e.g. ike/child-updown messages are sent that were
queued but couldn't be sent (even the job to enable to on_write() callback
requires a worker thread that's not around anymore during shutdown).
References #3602.
The message_t object used for defragmentation was only cleared after
all fragments have been received and the message was delivered. So
if we received only some fragments of a retransmitted message, the
fragments of the next message were not processed (message_t returns
INVALID_ARG if the message ID does not match causing the message to
get ignored). This rendered the IKE_SA unusable as the client
obviously never retransmitted the fragments of that previous message
after it received our response.
The parser is quite picky and e.g. doesn't accept UUIDs without dashes.
Even without a specific error, this at least points the users into the
right direction.
Fixes#3583.
If the activity is not active when the service connection is
established and handleIntent() is called, the activity's state is already
saved and any fragment transaction would result in an illegalStateException
due to state loss. We just ignore this and wait for another initiation
attempt (via onNewIntent()).
With the flag set, we basically ignore the resent intent, which is not
ideal if we have not yet actually started another activity. The information
dialog we show first would disappear when closing and reopening the app
or even just rotating it (we hide all dialogs when receiving an intent),
but since the flag was restored, the dialog was not shown again even
when attempting to start other connections.
Pascal Knecht