Skip to content
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.

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

Use the platform native APIs with C++ SSL transport #2063

Merged
merged 52 commits into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
e299cf6
Use ssl native API
pepone Apr 17, 2024
6b9286f
Checkpoint
pepone Apr 19, 2024
ddb1fb4
Checkpoint
pepone Apr 19, 2024
c36953a
SChannel fixes
pepone Apr 21, 2024
f176d7d
SChannel certificate validation fixes
pepone Apr 22, 2024
b18f2f5
SecureTransport fixes
pepone Apr 23, 2024
be82bae
Add TLS 1.2 protocol name
pepone Apr 23, 2024
c877a81
OpenSSL fixes
pepone Apr 24, 2024
ed9711b
Cleanup
pepone Apr 24, 2024
319ac21
Merge remote-tracking branch 'origin/main' into ssl-native
pepone Apr 24, 2024
ae94cd4
Cleanup
pepone Apr 24, 2024
9d13c39
Merge remote-tracking branch 'origin/main' into ssl-native
pepone Apr 25, 2024
2dfc388
Checkpoint
pepone Apr 25, 2024
1e48e73
Additional fixes
pepone Apr 25, 2024
02ac138
Fix clang format
pepone Apr 25, 2024
d61d27f
Doc comment fixes
pepone Apr 25, 2024
3588bd4
Script language fixes
pepone Apr 25, 2024
512c596
Checkpoint
pepone Apr 25, 2024
fbc6712
Fix Swift tests
pepone Apr 25, 2024
f4e9f30
Restore EOF_WHILE_READING
pepone Apr 26, 2024
c9ff80c
SChannel fixes
pepone Apr 26, 2024
f48f3cf
Retrieve OpenSSL peer certificate chain
pepone Apr 26, 2024
78c2921
OpenSSL fixes
pepone Apr 26, 2024
42bc483
Fix clang-format
pepone Apr 25, 2024
5030a1e
macOS fixes
pepone Apr 26, 2024
ae67d2f
Add default OpenSSL certificate verification callback
pepone Apr 26, 2024
4b2a152
Merge remote-tracking branch 'origin/main' into ssl-native
pepone Apr 26, 2024
f84b577
SecureTransport fixes
pepone Apr 26, 2024
54c4b49
Schannel fixes
pepone Apr 29, 2024
4d7a2f6
SecureTransport fixes
pepone Apr 29, 2024
dbc3234
Cleanup
pepone Apr 29, 2024
b58840b
SecureTransport & OpenSSL fixes
pepone Apr 30, 2024
460129a
Checkpoint
pepone Apr 30, 2024
0bec18e
Merge remote-tracking branch 'origin/main' into ssl-native
pepone Apr 30, 2024
89d2d1b
SecureTransport updates
pepone May 1, 2024
aec11f5
Improve OpenSSL usage doc comments
pepone May 1, 2024
1607c42
Retain/Release SecureTransport trusted roots
pepone May 1, 2024
bd70794
OpenSSL bug with bogus _sslCtx reference
pepone May 1, 2024
7c8290f
SChannel updates
pepone May 2, 2024
ab5d208
Update cpp/src/IceSSL/OpenSSLTransceiverI.cpp
pepone May 3, 2024
2a6c820
Update cpp/src/IceSSL/SecureTransportTransceiverI.cpp
pepone May 3, 2024
4326856
Update cpp/src/IceSSL/SecureTransportTransceiverI.cpp
pepone May 3, 2024
063dbc8
Update cpp/src/IceSSL/SChannelEngine.cpp
pepone May 3, 2024
d8851b4
Update cpp/src/IceSSL/OpenSSLTransceiverI.cpp
pepone May 3, 2024
79426bd
Update cpp/src/IceSSL/SChannelTransceiverI.cpp
pepone May 3, 2024
ed26e4e
Review fixes
pepone May 3, 2024
97c3dc6
More review fixes
pepone May 3, 2024
e0bf553
Fix typo
pepone May 3, 2024
f406d54
Cleanup
pepone May 3, 2024
3f0b970
Merge remote-tracking branch 'origin/main' into ssl-native
pepone May 3, 2024
456022a
Build fixes
pepone May 3, 2024
01f506f
Additional review fixes
pepone May 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 0 additions & 52 deletions cpp/include/Ice/Certificate.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,58 +18,6 @@

namespace IceSSL
{
/**
bernardnormier marked this conversation as resolved.
Show resolved Hide resolved
* The reason for an IceSSL certificate verification failure.
*/
enum class TrustError : std::uint8_t
{
/** The certification verification succeed */
NoError = 0,
/** The certificate chain length is greater than the specified maximum depth **/
ChainTooLong,
/** The X509 chain is invalid because a certificate has excluded a name constraint **/
HasExcludedNameConstraint,
/** The certificate has an undefined name constraint **/
HasNonDefinedNameConstraint,
/** The certificate has a non permitted name constraint **/
HasNonPermittedNameConstraint,
/** The certificate does not support a critical extension **/
HasNonSupportedCriticalExtension,
/** The certificate does not have a supported name constraint or has a name constraint that is unsupported **/
HasNonSupportedNameConstraint,
/** A host name mismatch has occurred **/
HostNameMismatch,
/** The X509 chain is invalid due to invalid basic constraints **/
InvalidBasicConstraints,
/** The X509 chain is invalid due to an invalid extension **/
InvalidExtension,
/** The X509 chain is invalid due to invalid name constraints **/
InvalidNameConstraints,
/** The X509 chain is invalid due to invalid policy constraints **/
InvalidPolicyConstraints,
/** The supplied certificate cannot be used for the specified purpose **/
InvalidPurpose,
/** The X509 chain is invalid due to an invalid certificate signature **/
InvalidSignature,
/** The X509 chain is not valid due to an invalid time value, such as a value that indicates an expired
certificate **/
InvalidTime,
/** The certificate is explicitly not trusted **/
NotTrusted,
/** The X509 chain could not be built up to the root certificate **/
PartialChain,
/** It is not possible to determine whether the certificate has been revoked **/
RevocationStatusUnknown,
/** The X509 chain is invalid due to a revoked certificate **/
Revoked,
/** The X509 chain is invalid due to an untrusted root certificate **/
UntrustedRoot,
/** The X509 chain is invalid due to other unknown failure **/
UnknownTrustFailure,
};

ICE_API std::string getTrustErrorDescription(TrustError);

/**
* The key usage "digitalSignature" bit is set
*/
Expand Down
106 changes: 106 additions & 0 deletions cpp/include/Ice/ClientAuthenticationOptions.h
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice API!

Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
//
// Copyright (c) ZeroC, Inc. All rights reserved.
//

#ifndef ICE_CLIENT_AUTHENTICATION_OPTIONS_H
#define ICE_CLIENT_AUTHENTICATION_OPTIONS_H

#include "SSLConnectionInfo.h"

#include <functional>

#if defined(_WIN32)
// We need to include windows.h before wincrypt.h.
// clang-format off
# ifndef NOMINMAX
# define NOMINMAX
# endif
# include <windows.h>
# include <wincrypt.h>
// clang-format on
// SECURITY_WIN32 or SECURITY_KERNEL, must be defined before including security.h indicating who is compiling the code.
pepone marked this conversation as resolved.
Show resolved Hide resolved
# ifdef SECURITY_WIN32
# undef SECURITY_WIN32
# endif
# ifdef SECURITY_KERNEL
# undef SECURITY_KERNEL
# endif
# define SECURITY_WIN32 1
# include <schannel.h>
# include <security.h>
# include <sspi.h>
# undef SECURITY_WIN32
#elif defined(__APPLE__)
# include <Security/SecureTransport.h>
# include <Security/Security.h>
#else
# include <openssl/ssl.h>
#endif

namespace Ice::SSL
{
/**
* The SSL configuration properties for client connections.
*/
struct ClientAuthenticationOptions
{
#if defined(_WIN32)
/**
* The credentials handler to configure SSL client connections on Windows. When set the SSL transport would
* ignore all the IceSSL configuration properties and use the provided credentials handle.
*
* [See Schannel
* Credentials](https://learn.microsoft.com/en-us/windows/win32/api/schannel/ns-schannel-sch_credentials).
*/
CredHandle clientCredentials;

/**
* A callback that allows selecting the credentials based on the target server's host name. When the
* callback is set it has preference over the credentials set in clientCredentials.
*/
std::function<CredHandle(const std::string& host)> clientCredentialsSelectionCallback;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume it's invalid for the returned CredHandle to be nullptr?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, We should throw in this case, which we currently don't do.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use std::string_view over const std::string& in new APIs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept the string reference because it is more convenient to call the native APIs that require a NULL-terminated string.


/**
* A callback that allows to manually validate the server certificate during SSL handshake on Windows. When the
* callback is not provided the server certificate will be validated using the platform default validation
* mechanism.
*
* @param context A security context is an opaque data structure that contains security data relevant to the
* current connection.
pepone marked this conversation as resolved.
Show resolved Hide resolved
* @return true if the certificate is valid, false otherwise.
*
* [See Manually Validating Schannel
* Credentials](https://learn.microsoft.com/en-us/windows/win32/secauthn/manually-validating-schannel-credentials).
*/
std::function<bool(CtxtHandle context, const IceSSL::ConnectionInfoPtr& info)>
serverCertificateValidationCallback;
#elif defined(__APPLE__)
// The client's certificate chain.
CFArrayRef clientCertificateChain;

// A callback that allows selecting a certificate chain based on the target server's host name. When the
// callback is set it has preference over a certificate chain set in clientCertificateChain.
std::function<CFArrayRef(const std::string& host)> clientCertificateSelectionCallback;
externl marked this conversation as resolved.
Show resolved Hide resolved

// The trusted root certificates. If set, the server's certificate chain is validated against these
// certificates. If not set the system's root certificates are used.
CFArrayRef trustedRootCertificates;
pepone marked this conversation as resolved.
Show resolved Hide resolved

// A callback that allows validating the server certificate chain. When set trustedRootCertificates are
// not used.
std::function<bool(SecTrustRef trust, const IceSSL::ConnectionInfoPtr& info)>
serverCertificateValidationCallback;

// A callback that is called before ssl handshake is started. The callback can be used to set additional SSL
// context parameters.
std::function<void(SSLContextRef)> sslContextSetup;
#else
SSL_CTX* sslContext;
std::function<int(int, X509_STORE_CTX*, const IceSSL::ConnectionInfoPtr& info)>
serverCertificateVerificationCallback;
std::function<void(::SSL* ssl, const std::string& host)> sslNewSessionCallback;
#endif
};
}

#endif
12 changes: 10 additions & 2 deletions cpp/include/Ice/Communicator.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "Plugin.h"
#include "Properties.h"
#include "Proxy.h"
#include "ServerAuthenticationOptions.h"

#ifdef ICE_SWIFT
# include <dispatch/dispatch.h>
Expand Down Expand Up @@ -141,12 +142,15 @@ namespace Ice
* communicator as is used by the adapter. Attempts to create a named object adapter for which no configuration
* can be found raise InitializationException.
* @param name The object adapter name.
* @param serverAuthenticationOptions The SSL configuration properties for server connections.
* @return The new object adapter.
* @see #createObjectAdapterWithEndpoints
* @see ObjectAdapter
* @see Properties
*/
ObjectAdapterPtr createObjectAdapter(const std::string& name);
ObjectAdapterPtr createObjectAdapter(
const std::string& name,
const std::optional<SSL::ServerAuthenticationOptions>& serverAuthenticationOptions = std::nullopt);

/**
* Create a new object adapter with endpoints. This operation sets the property
Expand All @@ -155,12 +159,16 @@ namespace Ice
* name.
* @param name The object adapter name.
* @param endpoints The endpoints for the object adapter.
* @param serverAuthenticationOptions The SSL configuration properties for server connections.
* @return The new object adapter.
* @see #createObjectAdapter
* @see ObjectAdapter
* @see Properties
*/
ObjectAdapterPtr createObjectAdapterWithEndpoints(const std::string& name, const std::string& endpoints);
ObjectAdapterPtr createObjectAdapterWithEndpoints(
const std::string& name,
const std::string& endpoints,
const std::optional<SSL::ServerAuthenticationOptions>& serverAuthenticationOptions = std::nullopt);

/**
* Create a new object adapter with a router. This operation creates a routed object adapter.
Expand Down
2 changes: 2 additions & 0 deletions cpp/include/Ice/Ice.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
// We don't need to see the following headers when building the generated code.

# include "Certificate.h"
# include "ClientAuthenticationOptions.h"
# include "Communicator.h"
# include "Connection.h"
# include "IconvStringConverter.h"
Expand All @@ -38,6 +39,7 @@
# include "SSLConnectionInfo.h"
# include "SSLEndpointInfo.h"
# include "ServantLocator.h"
# include "ServerAuthenticationOptions.h"
# include "SlicedData.h"
# include "StringConverter.h"
# include "UUID.h"
Expand Down
7 changes: 7 additions & 0 deletions cpp/include/Ice/Initialize.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define ICE_INITIALIZE_H

#include "BatchRequest.h"
#include "ClientAuthenticationOptions.h"
#include "CommunicatorF.h"
#include "Connection.h"
#include "Ice/BuiltinSequences.h"
Expand Down Expand Up @@ -330,6 +331,12 @@ namespace Ice
* The value factory manager.
*/
ValueFactoryManagerPtr valueFactoryManager;

/**
* The authentication options for SSL client connections. When set, the SSL transport ignores all IceSSL
* configuration properties and uses the provided options.
*/
std::optional<SSL::ClientAuthenticationOptions> clientAuthenticationOptions;
};

/**
Expand Down
20 changes: 4 additions & 16 deletions cpp/include/Ice/SSLConnectionInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,43 +33,31 @@ namespace IceSSL
* @param incoming Whether or not the connection is an incoming or outgoing connection.
* @param adapterName The name of the adapter associated with the connection.
* @param connectionId The connection id.
* @param cipher The negotiated cipher suite.
* @param certs The certificate chain.
* @param verified The certificate chain verification status.
* @param host The host name.
pepone marked this conversation as resolved.
Show resolved Hide resolved
*/
ConnectionInfo(
const Ice::ConnectionInfoPtr& underlying,
bool incoming,
const std::string& adapterName,
const std::string& connectionId,
const std::string& cipher,
const std::vector<CertificatePtr>& certs,
bool verified)
const std::string& host)
: Ice::ConnectionInfo(underlying, incoming, adapterName, connectionId),
cipher(cipher),
certs(certs),
verified(verified)
host(host)
{
}

ConnectionInfo(const ConnectionInfo&) = delete;
ConnectionInfo& operator=(const ConnectionInfo&) = delete;

/**
* The negotiated cipher suite.
*/
std::string cipher;
/**
* The certificate chain.
*/
std::vector<CertificatePtr> certs;
/**
* The certificate chain verification status.
*/
bool verified;

TrustError errorCode;
std::string host;
std::string desc;
};
}

Expand Down
Loading
Loading