Skip to content

Commit

Permalink
Added support for channel binding tokens when using HTTPS connections (
Browse files Browse the repository at this point in the history
…#6)

* Added support for channel binding tokens when using HTTPS connections
  • Loading branch information
jborean93 committed Oct 15, 2020
1 parent 30f2f7f commit 7b3b696
Show file tree
Hide file tree
Showing 17 changed files with 1,174 additions and 55 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,5 @@ __pycache__/
/ci-*.zip
/Unix/build-*
/integration_environment/.vagrant
/integration_environment/cert_setup
/integration_environment/exchange*
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
This is the changelog for this fork of OMI.
It documents the changes in each of the tagged releases

## 1.2.0 - TBD

+ Added support for channel binding tokens to work with `Auth/CbtHardeningLevel = Strict`
+ Improved error messages displayed when dealing with OpenSSL errors

## 1.1.0 - 2020-09-01

+ Added Archlinux as a known distribution
Expand Down
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ The following changes have been made:
+ Increased the password length limit to 8KiB to support modern authentication required by O365 WSMan connections
+ The original limit was 1KiB and I've seen JWT tokens that modern auth in O365 exceed 1.5KiB
+ I've set it to 8KiB as that seems to be a common default of HTTP header sizes, if it does exceed that then the server would return a 413 anyway
+ Added support for sending the channel binding tokens when using GSSAPI on a HTTPS connection
+ This will allow the client to authenticate when the WSMan service has set `Auth/CbtHardeningLevel = Strict`
+ If the client fails to derive the CBT token, further information can be found in the [logs](#troubleshooting)

I am not looking at fixing any underlying problems in this library or work on the server side part of OMI.
This is purely focusing on improving the experience when using WinRM as a client on non-Windows based hosts within PowerShell.
Expand Down Expand Up @@ -277,7 +280,6 @@ Otherwise other features/changes that are in the backlog are:
+ Continue to add more distributions for building/testing
+ Alpine 3.9
+ Alpine 3.10
+ Archlinux
+ OpenSUSE 42.3
+ OpenSUSE Leap 15
+ Add a way to specify the `omicli.conf` file through an env var instead of the hardcoded location
Expand Down
186 changes: 181 additions & 5 deletions Unix/http/httpclient.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,37 @@ typedef void SSL_CTX;
#define SSL_accept(c) 0
#define SSL_connect(c) 0

#endif

// JBOREAN CHANGE: New OpenSSL definitions that are being used. This should matter as we only build for POSIX
// compliant systems but best to be safe than sorry.
typedef void EVP_MD;
typedef void X509;
#define EVP_sha256() 0
#define EVP_sha384() 0
#define EVP_sha512() 0
#define OBJ_find_sigid_algs(s, p, r) 0
#define SSL_get_peer_certificate(s) 0
#define X509_digest(d, t, m, l) 0
#define X509_free(a) 0
#define X509_get_signature_nid(c) 0
#define EVP_MAX_MD_SIZE 4
#define NID_md5 4
#define NID_sha1 64
#define NID_sha224 675
#define NID_sha256 672
#define NID_sha384 674
#define NID_sha512 674

#else

// X509_get_signature_nid() was added in SSL 1.0.2, define the fallback for older versions.
#if OPENSSL_VERSION_NUMBER < 0x1000200fL // Old than SSL 1.0.2
int X509_get_signature_nid(const X509 *x)
{
return OBJ_obj2nid(x->sig_alg->algorithm);
}
#endif
#endif

#ifdef ENABLE_TRACING
# define TRACING_LEVEL 4
Expand Down Expand Up @@ -143,6 +172,15 @@ static const char* sslstrerror(unsigned long SslError)

#include "httpclient_private.h"

// JBOREAN CHANGE: Used for creating channel binding data structure.
#if AUTHORIZATION
#if defined(is_macos)
#include <GSS/GSS.h>
#else
#include <gssapi/gssapi.h>
#endif
#endif

/*
NOTE: Initialize the session map
*/
Expand Down Expand Up @@ -454,6 +492,8 @@ static MI_Result _Sock_Write(
{
int res;
int sslError;
// JBOREAN CHANGE: Required for SSL_ERROR_SSL as we use the error char* twice.
char* sslErrorString = NULL;

if (!handler->ssl)
{
Expand All @@ -470,6 +510,8 @@ static MI_Result _Sock_Write(

*sizeWritten = 0;

// JBOREAN CHANGE: SSL_connect is done when creating the socket now.
/*
if (handler->connectDone)
{
res = SSL_write(handler->ssl, buf, buf_size);
Expand All @@ -481,13 +523,15 @@ static MI_Result _Sock_Write(
LOGD2((ZT("_Sock_Write - SSL connect using socket %d returned result: %d, errno: %d (%s)"), handler->base.sock, res, errno, strerror(errno)));
if (res > 0)
{
/* we are done with accept */
// we are done with accept
handler->connectDone = MI_TRUE;
return _Sock_Write(handler,buf,buf_size,sizeWritten);
}
/* perform regular error checking */
//perform regular error checking
}

*/
res = SSL_write(handler->ssl, buf, buf_size);
LOGD2((ZT("_Sock_Write - SSL_write using socket %d returned %d (< 0 for error) / %u bytes written, errno: %d (%s)"), handler->base.sock, res, (unsigned int)buf_size, errno, strerror(errno)));

if (res == 0)
{
Expand Down Expand Up @@ -530,7 +574,11 @@ static MI_Result _Sock_Write(
break;

case SSL_ERROR_SSL:
LOGE2((ZT("_Sock_Write - SSL_write/connect returned OpenSSL error %d (%s)"), sslError, ERR_error_string(sslError, NULL)));
// JBOREAN CHANGE: Use ERR_error_string to get the actual problem not just SSL_ERROR_SSL. Also report the
// error string back to PowerShell so end users can understand what went wrong.
sslErrorString = ERR_error_string(ERR_get_error(), NULL);
handler->errMsg = (MI_Char*)sslErrorString;
LOGE2((ZT("_Sock_Write - SSL_write/connect returned OpenSSL error %d (%s)"), sslError, sslErrorString));
break;

default:
Expand Down Expand Up @@ -1615,6 +1663,13 @@ static MI_Boolean _RequestCallback(

Sock_Close(handler->base.sock);

// JBOREAN CHANGE: Added new field that needs to be freed.
if (handler->channelBindingData)
{
PAL_Free(handler->channelBindingData);
handler->channelBindingData = NULL;
}

if (handler->recvPage)
PAL_Free(handler->recvPage);

Expand Down Expand Up @@ -1776,6 +1831,103 @@ static MI_Result _CreateSocketAndConnect(
return MI_RESULT_WOULD_BLOCK;
}

// JBOREAN CHANGE: Used by _CreateConnectorSocket to create the channel binding token data for GSSAPI.
#if AUTHORIZATION
static MI_Result _CreateChannelBindingToken(
HttpClient_SR_SocketData* handler)
{
MI_Result res = MI_RESULT_OK;
X509* certificate = NULL;
int algoNID;
const EVP_MD* algoType = NULL;
unsigned char* certHash = NULL;
unsigned int certHashLength;

const char* bindingPrefix = "tls-server-end-point:";
int bindingPrefixLength = strlen(bindingPrefix);
int bindingStructLength = sizeof(struct gss_channel_bindings_struct);
gss_channel_bindings_t bindings = NULL;

// TODO: OpenSSL 3.0.0 has deprecated SSL_get_peer_certificate in favour of SSL_get1_peer_certificate.
certificate = SSL_get_peer_certificate(handler->ssl);
if (!certificate)
{
LOGE2((ZT("_CreateChannelBindingToken - Failed to get TLS peer certificate - skipping CBT")));
res = MI_RESULT_FAILED;
goto Done;
}

if (!OBJ_find_sigid_algs(X509_get_signature_nid(certificate), &algoNID, NULL))
{
LOGE2((ZT("_CreateChannelBindingToken - Failed to derive TLS signature algorithm from peer certificate - skipping CBT")));
res = MI_RESULT_FAILED;
goto Done;
}

switch (algoNID)
{
case NID_sha512:
algoType = EVP_sha512();
break;
case NID_sha384:
algoType = EVP_sha384();
break;

// RFC 5929 states the hash algorithm is to be SHA256 if the digest is less than 256 bits.
case NID_md5:
case NID_sha1:
case NID_sha224:
case NID_sha256:
default:
algoType = EVP_sha256();
break;
}

certHash = PAL_Malloc(EVP_MAX_MD_SIZE);
if (certHash == NULL)
{
LOGE2((ZT("_CreateChannelBindingToken - Failed to allocate certificate hash buffer - skipping CBT")));
res = MI_RESULT_SERVER_LIMITS_EXCEEDED;
goto Done;
}

if (!X509_digest(certificate, algoType, certHash, &certHashLength))
{
res = MI_RESULT_FAILED;
goto Done;
}

handler->channelBindingData = (unsigned char*)PAL_Calloc(1, bindingStructLength + bindingPrefixLength + certHashLength);
if (handler->channelBindingData == NULL)
{
LOGE2((ZT("_CreateChannelBindingToken - Failed to allocate gss_channel_bindings_t buffer - skipping CBT")));
res = MI_RESULT_SERVER_LIMITS_EXCEEDED;
goto Done;
}

bindings = (gss_channel_bindings_t)handler->channelBindingData;
bindings->application_data.length = bindingPrefixLength + certHashLength;
bindings->application_data.value = handler->channelBindingData + bindingStructLength;
memcpy(bindings->application_data.value, bindingPrefix, bindingPrefixLength);
memcpy(bindings->application_data.value + bindingPrefixLength, certHash, certHashLength);

LOGD2((ZT("_CreateChannelBindingToken - OK exit")));

Done:
if (certificate)
{
X509_free(certificate);
}

if (certHash)
{
PAL_Free(certHash);
}

return res;
}
#endif

static MI_Result _CreateConnectorSocket(
HttpClient* self,
const char* host,
Expand Down Expand Up @@ -3119,6 +3271,30 @@ MI_Result HttpClient_StartRequestV2(
const char *auth_header = NULL;
const char* sessionCookie = NULL;

// JBOREAN CHANGE: Make sure the SSL context has been connected so we can get the peer certificate for GSSAPI TLS
// channel binding token support. This must be done before the authentication header is build.
if (client->connector->ssl)
{
int res = SSL_connect(client->connector->ssl);
if (res < 1)
{
int sslError = SSL_get_error(client->connector->ssl, res);
char* sslErrorString = ERR_error_string(ERR_get_error(), NULL);
client->connector->errMsg = (MI_Char*)sslErrorString;
LOGE2((ZT("HttpClient_StartRequestV2 - SSL connect returned OpenSSL error %d (%s)"), sslError, sslErrorString));

r = MI_RESULT_FAILED;
goto Error;
}

LOGD2((ZT("HttpClient_StartRequestV2 - SSL connect using socket %d returned result: %d, errno: %d (%s)"), client->connector->base.sock, res, errno, strerror(errno)));

#if AUTHORIZATION
// Getting the CBT data shouldn't warrant a failure so we just ignore a failure for now.
_CreateChannelBindingToken(client->connector);
#endif
}

// Get the session cookie from the last response, if available
sessionCookie = SessionMap_GetCookie(&_sessionMap, client->sessionId);

Expand Down
10 changes: 7 additions & 3 deletions Unix/http/httpclient_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ typedef struct _HttpClient_SR_SocketData {
/* ssl part */
SSL *ssl;
MI_Boolean reverseOperations; /*reverse read/write Events/Handlers */
MI_Boolean connectDone;

// JBOREAN CHANGE: Added support for channel binding tokens has rearranged the connection method and now needs to
// store the TLS channel bindings token.
// MI_Boolean connectDone;
unsigned char* channelBindingData;

/* receiving data */
__field_ecount(recvBufferSize) char *recvBuffer;
Expand Down Expand Up @@ -88,7 +92,7 @@ typedef struct _HttpClient_SR_SocketData {
void *authContext; // gss_context_t
void *targetName; // gss_name_t
void *cred; // gss_cred_id_t
AuthMechanism selectedMech; //
AuthMechanism selectedMech; //
MI_Uint32 negoFlags;

/* Destination info. We use this in the authorisation transaction */
Expand All @@ -100,7 +104,7 @@ typedef struct _HttpClient_SR_SocketData {
MI_Boolean secure; // This is an SSL connection (https)
MI_Boolean isPrivate; // This connection is to be encrypted

MI_Char *errMsg; // Has a error mesisage produced in IsAuthorized or other areas
MI_Char *errMsg; // Has a error mesisage produced in IsAuthorized or other areas
// that have interesting information for CIMerror

/* For the authorisation loop we need to retain the components of the original message */
Expand Down
24 changes: 22 additions & 2 deletions Unix/http/httpclientauth.c
Original file line number Diff line number Diff line change
Expand Up @@ -2422,6 +2422,9 @@ HttpClient_NextAuthRequest(_In_ struct _HttpClient_SR_SocketData * self, _In_ co
gss_name_t target_name = (gss_name_t) self->targetName;
gss_OID chosen_mech = NULL;

// JBOREAN CHANGE: Access channel binding hash through the client.
HttpClient *client = (HttpClient *) self->base.data;

if (!pRequestHeader)
{
// complain here
Expand Down Expand Up @@ -2494,9 +2497,16 @@ HttpClient_NextAuthRequest(_In_ struct _HttpClient_SR_SocketData * self, _In_ co
// (void)DecodeToken(&input_token);
if (!skip_step)
{
// JBOREAN CHANGE: Pass along the channel bindings struct if present.
gss_channel_bindings_t input_chan_bindings = GSS_C_NO_CHANNEL_BINDINGS;
if (client->connector->channelBindingData)
{
input_chan_bindings = (gss_channel_bindings_t)client->connector->channelBindingData;
}

maj_stat = (*_g_gssClientState.Gss_Init_Sec_Context)(&min_stat, self->cred, &context_hdl, target_name, mechset->elements, self->negoFlags, // flags
0, // time_req,
GSS_C_NO_CHANNEL_BINDINGS, // input_chan_bindings,
input_chan_bindings,
&input_token, &chosen_mech, /* mech type */
&output_token, &self->negoFlags, NULL); /* time_rec */
}
Expand Down Expand Up @@ -2683,6 +2693,9 @@ static char *_BuildInitialGssAuthHeader(_In_ HttpClient_SR_SocketData * self, MI
gss_buffer_desc output_token;
gss_OID_set mechset = NULL;

// JBOREAN CHANGE: Access channel binding hash through the client.
HttpClient *client = (HttpClient *) self->base.data;

if (self->authContext)
{

Expand Down Expand Up @@ -3026,9 +3039,16 @@ static char *_BuildInitialGssAuthHeader(_In_ HttpClient_SR_SocketData * self, MI
self->negoFlags = (GSS_C_INTEG_FLAG | GSS_C_CONF_FLAG | GSS_C_REPLAY_FLAG | GSS_C_MUTUAL_FLAG | GSS_C_DELEG_POLICY_FLAG);
}

// JBOREAN CHANGE: Pass along the channel bindings struct if present.
gss_channel_bindings_t input_chan_bindings = GSS_C_NO_CHANNEL_BINDINGS;
if (client->connector->channelBindingData)
{
input_chan_bindings = (gss_channel_bindings_t)client->connector->channelBindingData;
}

maj_stat = (*_g_gssClientState.Gss_Init_Sec_Context)(&min_stat, cred, &context_hdl, target_name, mechset->elements, self->negoFlags,
0, // time_req,
GSS_C_NO_CHANNEL_BINDINGS, // input_chan_bindings,
input_chan_bindings,
GSS_C_NO_BUFFER, NULL, &output_token, &self->negoFlags, 0); // time_req

if (maj_stat == GSS_S_CONTINUE_NEEDED)
Expand Down
2 changes: 2 additions & 0 deletions distribution_meta/debian10.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
"pkg-config"
],
"test_deps": [
"gcc",
"gss-ntlmssp",
"krb5-user",
"libkrb5-dev",
"powershell"
]
}
1 change: 1 addition & 0 deletions distribution_meta/debian8.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
],
"test_deps": [
"krb5-user",
"libkrb5-dev",
"powershell"
]
}
Loading

0 comments on commit 7b3b696

Please sign in to comment.