Skip to content

Commit

Permalink
Checkpoint
Browse files Browse the repository at this point in the history
  • Loading branch information
pepone committed Apr 25, 2024
1 parent 3588bd4 commit 512c596
Show file tree
Hide file tree
Showing 18 changed files with 42 additions and 96 deletions.
17 changes: 7 additions & 10 deletions cpp/include/Ice/ClientAuthenticationOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,19 @@ namespace Ice::SSL
{
#if defined(_WIN32)
/**
* A callback that allows selecting the client credentials based on the target server host name.
* A callback that allows selecting the client's credentials based on the target server host name.
*
* @param host The target server host name.
* @return The server credentials. The credentials must remain valid for the duration of the connection.
* @return The client credentials. The credentials must remain valid for the duration of the connection.
*
* [See Detailed Schannel documentation on Schannel credentials](
* https://learn.microsoft.com/en-us/windows/win32/secauthn/acquirecredentialshandle--schannel)
*/
std::function<CredHandle(const std::string& host)> clientCredentialsSelectionCallback;

/**
* A callback that allows manual validation of the client server's certificate chain during the SSL handshake.
* This callback allows for implementing custom verification logic. When the verification callback returns
* false, the connection will be aborted with an Ice::SecurityException.
* A callback that allows manually validating the server certificate chain. When the verification callback
* returns false, the connection will be aborted with an Ice::SecurityException.
*
* @param context A CtxtHandle representing the security context associated with the current connection. This
* context contains security data relevant for validation, such as the server's certificate chain and cipher
Expand All @@ -83,7 +82,7 @@ namespace Ice::SSL
* @return The client's certificate chain. The certificate chain must remain valid for the duration of the
* connection.
*
* The requirements for the Secure Transport certificates are documented in
* The requirements for the Secure Transport certificate chain are documented in
* https://developer.apple.com/documentation/security/1392400-sslsetcertificate?changes=_3&language=objc
*/
std::function<CFArrayRef(const std::string& host)> clientCertificateSelectionCallback;
Expand Down Expand Up @@ -143,10 +142,8 @@ namespace Ice::SSL
std::function<void(::SSL* ssl, const std::string& host)> sslNewSessionCallback;

/**
* A callback that allows manual validation of the server certificate chain during the SSL handshake. This
* callback is called from the SSL_verify_cb function in OpenSSL and provides an interface for custom
* verification logic beyond the standard certificate checking process. When the verification callback returns
* false, the connection will be aborted with an Ice::SecurityException.
* A callback that allows manually validating the server certificate chain. When the verification callback
* returns false, the connection will be aborted with an Ice::SecurityException.
*
* @param verified A boolean indicating whether the preliminary certificate verification done by OpenSSL's
* built-in mechanisms succeeded or failed. True if the preliminary checks passed, false otherwise.
Expand Down
9 changes: 2 additions & 7 deletions cpp/include/Ice/SSLConnectionInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,15 @@ namespace IceSSL
* @param adapterName The name of the adapter associated with the connection.
* @param connectionId The connection id.
* @param certs The certificate chain.
* @param host The host name.
*/
ConnectionInfo(
const Ice::ConnectionInfoPtr& underlying,
bool incoming,
const std::string& adapterName,
const std::string& connectionId,
const std::vector<CertificatePtr>& certs,
const std::string& host)
const std::vector<CertificatePtr>& certs)
: Ice::ConnectionInfo(underlying, incoming, adapterName, connectionId),
certs(certs),
host(host)
certs(certs)
{
}

Expand All @@ -56,8 +53,6 @@ namespace IceSSL
* The certificate chain.
*/
std::vector<CertificatePtr> certs;
std::string host;
std::string desc;
};
}

Expand Down
12 changes: 4 additions & 8 deletions cpp/include/Ice/ServerAuthenticationOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,8 @@ namespace Ice::SSL
bool clientCertificateRequired;

/**
* A callback that allows manual validation of the client certificate chain during the SSL handshake. Unlike
* other implementations, Schannel does not automatically validate the client certificate chain. This callback
* allows for implementing custom verification logic. When the verification callback returns false, the
* connection will be aborted with an Ice::SecurityException.
* A callback that allows manually validating the client certificate chain. When the verification callback
* returns false, the connection will be aborted with an Ice::SecurityException.
*
* @param context A CtxtHandle representing the security context associated with the current connection. This
* context contains security data relevant for validation, such as the client's certificate chain and cipher
Expand Down Expand Up @@ -156,10 +154,8 @@ namespace Ice::SSL
std::function<void(::SSL* ssl, const std::string& adapterName)> sslNewSessionCallback;

/**
* A callback that allows manual validation of the client certificate chain during the SSL handshake. This
* callback is called from the SSL_verify_cb function in OpenSSL and provides an interface for custom
* verification logic beyond the standard certificate checking process. When the verification callback returns
* false, the connection will be aborted with an Ice::SecurityException.
* A callback that allows manually validating the client certificate chain. When the verification callback
* returns false, the connection will be aborted with an Ice::SecurityException.
*
* @param verified A boolean indicating whether the preliminary certificate verification done by OpenSSL's
* built-in mechanisms succeeded or failed. True if the preliminary checks passed, false otherwise.
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/Ice/ObjectAdapterFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ IceInternal::ObjectAdapterFactory::updateObservers(void (ObjectAdapterI::*fn)())

ObjectAdapterPtr
IceInternal::ObjectAdapterFactory::createObjectAdapter(
const std::string& name,
const std::optional<Ice::RouterPrx>& router,
const string& name,
const optional<Ice::RouterPrx>& router,
const optional<SSL::ServerAuthenticationOptions>& serverAuthenticationOptions)
{
shared_ptr<ObjectAdapterI> adapter;
Expand Down
14 changes: 7 additions & 7 deletions cpp/src/Ice/ObjectAdapterI.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,23 +99,23 @@ namespace Ice
void decDirectCount();

IceInternal::ThreadPoolPtr getThreadPool() const;
void setAdapterOnConnection(const Ice::ConnectionIPtr&);
void setAdapterOnConnection(const ConnectionIPtr&);
size_t messageSizeMax() const { return _messageSizeMax; }

// The dispatch pipeline is the dispatcher plus the logger and observer middleware. They are installed in the
// dispatch pipeline only when the communicator configuration enables them.
const Ice::ObjectPtr& dispatchPipeline() const noexcept { return _dispatchPipeline; }
const ObjectPtr& dispatchPipeline() const noexcept { return _dispatchPipeline; }

ObjectAdapterI(
const IceInternal::InstancePtr&,
const CommunicatorPtr&,
const IceInternal::ObjectAdapterFactoryPtr&,
const std::string&,
bool,
const std::optional<Ice::SSL::ServerAuthenticationOptions>&);
const std::optional<SSL::ServerAuthenticationOptions>&);
virtual ~ObjectAdapterI();

std::optional<Ice::SSL::ServerAuthenticationOptions> getServerAuthenticationOptions() const
std::optional<SSL::ServerAuthenticationOptions> getServerAuthenticationOptions() const
{
return _serverAuthenticationOptions;
}
Expand All @@ -130,8 +130,8 @@ namespace Ice
void checkForDeactivation() const;
std::vector<IceInternal::EndpointIPtr> parseEndpoints(const std::string&, bool) const;
std::vector<IceInternal::EndpointIPtr> computePublishedEndpoints();
void updateLocatorRegistry(const IceInternal::LocatorInfoPtr&, const std::optional<Ice::ObjectPrx>&);
bool filterProperties(Ice::StringSeq&);
void updateLocatorRegistry(const IceInternal::LocatorInfoPtr&, const std::optional<ObjectPrx>&);
bool filterProperties(StringSeq&);

enum State
{
Expand Down Expand Up @@ -168,7 +168,7 @@ namespace Ice
size_t _messageSizeMax;
mutable std::recursive_mutex _mutex;
std::condition_variable_any _conditionVariable;
const std::optional<Ice::SSL::ServerAuthenticationOptions> _serverAuthenticationOptions;
const std::optional<SSL::ServerAuthenticationOptions> _serverAuthenticationOptions;
};
}

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/Ice/WSEndpoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ IceInternal::WSEndpoint::connectorsAsync(
AcceptorPtr
IceInternal::WSEndpoint::acceptor(
const string& adapterName,
const optional<Ice::SSL::ServerAuthenticationOptions>& serverAuthenticationOptions) const
const optional<SSL::ServerAuthenticationOptions>& serverAuthenticationOptions) const
{
AcceptorPtr acceptor = _delegate->acceptor(adapterName, serverAuthenticationOptions);
return make_shared<WSAcceptor>(const_cast<WSEndpoint*>(this)->shared_from_this(), _instance, acceptor);
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/IceBT/EndpointI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ IceBT::EndpointI::connectorsAsync(
}

IceInternal::AcceptorPtr
IceBT::EndpointI::acceptor(const string& adapterName, const std::optional<Ice::SSL::ServerAuthenticationOptions>&) const
IceBT::EndpointI::acceptor(const string& adapterName, const std::optional<SSL::ServerAuthenticationOptions>&) const
{
return make_shared<AcceptorI>(
const_cast<EndpointI*>(this)->shared_from_this(),
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/IceSSL/SSLEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ IceSSL::SSLEngine::verifyPeer(const ConnectionInfoPtr& info) const
string msg = string(info->incoming ? "incoming" : "outgoing") + " connection rejected by trust manager";
if (_securityTraceLevel >= 1)
{
getLogger()->trace(_securityTraceCategory, msg + "\n" + info->desc);
getLogger()->trace(_securityTraceCategory, msg);
}
throw SecurityException(__FILE__, __LINE__, msg);
}
Expand Down
1 change: 0 additions & 1 deletion cpp/src/IceSSL/SecureTransportTransceiverI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,6 @@ IceSSL::SecureTransport::TransceiverI::getInfo() const
info->incoming = _incoming;
info->adapterName = _adapterName;
info->certs = _certs;
info->host = _host;
return info;
}

Expand Down
1 change: 0 additions & 1 deletion cpp/src/IceSSL/TrustManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ TrustManager::verify(const ConnectionInfoPtr& info) const
trace << "trust manager evaluating server:\n"
<< "subject = " << string(subject) << '\n';
}
trace << info->desc;
}

// Fail if we match anything in the reject set.
Expand Down
15 changes: 4 additions & 11 deletions cpp/test/IceSSL/configuration/AllTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -803,8 +803,7 @@ allTests(Test::TestHelper* helper, const string& /*testDir*/, bool p12)
d = createServerProps(props, p12, "s_rsa_ca1_cn1", "cacert1");
server = fact->createServer(d);

info = dynamic_pointer_cast<IceSSL::ConnectionInfo>(server->ice_getConnection()->getInfo());
test(info->host == "localhost");
server->ice_ping();

fact->destroyServer(server);
comm->destroy();
Expand Down Expand Up @@ -854,8 +853,7 @@ allTests(Test::TestHelper* helper, const string& /*testDir*/, bool p12)
// Expected.
}
#else
info = dynamic_pointer_cast<IceSSL::ConnectionInfo>(server->ice_getConnection()->getInfo());
test(info->host == "localhost");
server->ice_ping();
#endif

fact->destroyServer(server);
Expand Down Expand Up @@ -922,8 +920,7 @@ allTests(Test::TestHelper* helper, const string& /*testDir*/, bool p12)
d = createServerProps(defaultProps, p12, "s_rsa_ca1_cn6", "cacert1");
server = fact->createServer(d);

info = dynamic_pointer_cast<IceSSL::ConnectionInfo>(server->ice_getConnection()->getInfo());
test(info->host == "127.0.0.1");
server->ice_ping();

fact->destroyServer(server);
comm->destroy();
Expand Down Expand Up @@ -2917,11 +2914,7 @@ allTests(Test::TestHelper* helper, const string& /*testDir*/, bool p12)
{
try
{
Ice::WSConnectionInfoPtr wsinfo =
dynamic_pointer_cast<Ice::WSConnectionInfo>(p->ice_getConnection()->getInfo());
IceSSL::ConnectionInfoPtr sslInfo =
dynamic_pointer_cast<IceSSL::ConnectionInfo>(wsinfo->underlying);
test(sslInfo->host == "zeroc.com");
p->ice_ping();
break;
}
catch (const Ice::LocalException& ex)
Expand Down
13 changes: 3 additions & 10 deletions swift/src/Ice/ConnectionInfoFactory.swift
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,12 @@ private class WSConnectionInfoI: ConnectionInfoI, WSConnectionInfo {
}

private class SSLConnectionInfoI: ConnectionInfoI, SSLConnectionInfo {
var cipher: String
var certs: [SecCertificate]
var verified: Bool

init(
underlying: ConnectionInfo?, incoming: Bool, adapterName: String, connectionId: String,
cipher: String, certs: StringSeq, verified: Bool
certs: StringSeq
) {
self.cipher = cipher
self.certs = []
let beginPrefix = "-----BEGIN CERTIFICATE-----\n"
let endPrefix = "\n-----END CERTIFICATE-----\n"
Expand All @@ -122,7 +119,6 @@ private class SSLConnectionInfoI: ConnectionInfoI, SSLConnectionInfo {
}
}
}
self.verified = verified
super.init(
underlying: underlying, incoming: incoming, adapterName: adapterName,
connectionId: connectionId)
Expand Down Expand Up @@ -249,17 +245,14 @@ class ConnectionInfoFactory: ICEConnectionInfoFactory {
incoming: Bool,
adapterName: String,
connectionId: String,
cipher: String,
certs: [String], verified: Bool
certs: [String]
) -> Any {
return SSLConnectionInfoI(
underlying: getUnderlying(underlying),
incoming: incoming,
adapterName: adapterName,
connectionId: connectionId,
cipher: cipher,
certs: certs,
verified: verified)
certs: certs)
}

#if os(iOS) || os(watchOS) || os(tvOS)
Expand Down
15 changes: 0 additions & 15 deletions swift/src/Ice/SSLConnectionInfo.swift
Original file line number Diff line number Diff line change
@@ -1,26 +1,11 @@
//
// Copyright (c) ZeroC, Inc. All rights reserved.
//
//
// Ice version 3.7.10
//
// <auto-generated>
//
// Generated from file `ConnectionInfo.ice'
//
// Warning: do not edit this file.
//
// </auto-generated>
//

import Foundation

/// Provides access to the connection details of an SSL connection
public protocol SSLConnectionInfo: ConnectionInfo {
/// The negotiated cipher suite.
var cipher: Swift.String { get set }
/// The certificate chain.
var certs: [SecCertificate] { get set }
/// The certificate chain verification status.
var verified: Swift.Bool { get set }
}
4 changes: 1 addition & 3 deletions swift/src/IceImpl/Connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,7 @@ ICEIMPL_API @protocol ICEConnectionInfoFactory
incoming:(BOOL)incoming
adapterName:(NSString*)adapterName
connectionId:(NSString*)connectionId
cipher:(NSString*)cipher
certs:(NSArray<NSString*>*)certs
verified:(BOOL)verified;
certs:(NSArray<NSString*>*)certs;

#if TARGET_OS_IPHONE

Expand Down
4 changes: 1 addition & 3 deletions swift/src/IceImpl/Connection.mm
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,7 @@ - (BOOL)throwException:(NSError**)error
incoming:sslInfo->incoming
adapterName:toNSString(sslInfo->adapterName)
connectionId:toNSString(sslInfo->connectionId)
cipher:toNSString(sslInfo->cipher)
certs:toNSArray(sslInfo->certs)
verified:sslInfo->verified];
certs:toNSArray(sslInfo->certs)];
}

#if TARGET_OS_IPHONE
Expand Down
12 changes: 8 additions & 4 deletions swift/test/IceSSL/configuration/AllTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ public func allTests(_ helper: TestHelper, _ defaultDir: String) throws -> SSLSe
// and doesn't trust the server certificate.
//
let properties = createClientProps(defaultProperties: defaultProperties, cert: "", ca: "")
properties.setProperty(key: "IceSSL.VerifyPeer", value: "0")
let comm = try helper.initialize(properties)
let fact = try checkedCast(
prx: comm.stringToProxy(factoryRef)!, type: SSLServerFactoryPrx.self)!
Expand All @@ -120,8 +119,14 @@ public func allTests(_ helper: TestHelper, _ defaultDir: String) throws -> SSLSe
d["IceSSL.VerifyPeer"] = "0"
let server = try fact.createServer(d)!

try server.noCert()
try test(!(server.ice_getConnection()!.getInfo() as! SSLConnectionInfo).verified)
do {
try server.noCert()
try test(false)
} catch is SecurityException {
// Expected, if reported as an SSL alert by the server.
} catch is ConnectionLostException {
// Expected.
}
try fact.destroyServer(server)
comm.destroy()
}
Expand All @@ -142,7 +147,6 @@ public func allTests(_ helper: TestHelper, _ defaultDir: String) throws -> SSLSe
var server = try fact.createServer(d)!
do {
try server.noCert()
try test((server.ice_getConnection()!.getInfo() as! SSLConnectionInfo).verified)
}
try fact.destroyServer(server)
//
Expand Down
1 change: 0 additions & 1 deletion swift/test/IceSSL/configuration/Test.ice
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ interface Server
{
void noCert();
void checkCert(string subjectDN, string issuerDN);
void checkCipher(string cipher);
}

dictionary<string, string> Properties;
Expand Down
Loading

0 comments on commit 512c596

Please sign in to comment.