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.

Sign up for GitHub

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

Jump to bottom

DRILL-5873: (C++ Client) Improve SASL error reporting. #992

Closed
wants to merge 2 commits into from
Closed

DRILL-5873: (C++ Client) Improve SASL error reporting. #992

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f5b343
DRILL-5873: (C++ Client) Improve SASL error reporting.
parthchandra Oct 13, 2017
6c7cef4
DRILL_5873: Add SASL message for both error conditions.
parthchandra Oct 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 10 additions & 4 deletions contrib/native/client/src/clientlib/drillClientImpl.cpp
View file
Open in desktop
Expand Up @@ -657,8 +657,11 @@ connectionStatus_t DrillClientImpl::handleAuthentication(const DrillUserProperti
// Check the negotiated SSF value and change the handlers.
if(m_encryptionCtxt.isEncryptionReqd()) {
if(SASL_OK != m_saslAuthenticator->verifyAndUpdateSaslProps()) {
logMsg << m_encryptionCtxt << "]. Negotiated Parameter is invalid."
<< " Error: " << m_saslResultCode;
logMsg << m_encryptionCtxt
<< ", Mechanism: " << m_saslAuthenticator->getAuthMechanismName()
<< ", Error: " << m_saslResultCode
<< ", Cause: " << m_saslAuthenticator->getErrorMessage(m_saslResultCode);
logMsg << "]. Negotiated Parameter is invalid.";
DRILL_MT_LOG(DRILL_LOG(LOG_DEBUG) << logMsg.str() << std::endl;)
return handleConnError(CONN_AUTH_FAILED, logMsg.str().c_str());
}
Expand All @@ -678,11 +681,14 @@ connectionStatus_t DrillClientImpl::handleAuthentication(const DrillUserProperti
m_io_service.reset();
return CONN_SUCCESS;
} else {
logMsg << m_encryptionCtxt << ", Error: " << m_saslResultCode;
logMsg << m_encryptionCtxt
<< ", Mechanism: " << m_saslAuthenticator->getAuthMechanismName()
<< ", Error: " << m_saslResultCode
<< ", Cause: " << m_saslAuthenticator->getErrorMessage(m_saslResultCode);
Copy link
Contributor

Choose a reason for hiding this comment

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

How about adding this cause message string in the if condition too ?

logMsg << "]. Check connection parameters?";
DRILL_MT_LOG(DRILL_LOG(LOG_DEBUG) << logMsg.str() << std::endl;)

// shuts down socket as well
logMsg << "]. Check connection parameters?";
return handleConnError(CONN_AUTH_FAILED, logMsg.str().c_str());
}
}
Expand Down
13 changes: 13 additions & 0 deletions contrib/native/client/src/clientlib/saslAuthenticatorImpl.cpp
View file
Open in desktop
Expand Up @@ -145,6 +145,7 @@ int SaslAuthenticatorImpl::init(const std::vector<std::string>& mechanisms, exec
authMechanismToUse = value;
}
}
m_authMechanismName = authMechanismToUse;
if (authMechanismToUse.empty()) return SASL_NOMECH;

// check if requested mechanism is supported by server
Expand Down Expand Up @@ -316,5 +317,17 @@ int SaslAuthenticatorImpl::unwrap(const char* dataToUnWrap, const int& dataToUnW
return sasl_decode(m_pConnection, dataToUnWrap, dataToUnWrapLen, output, &unWrappedLen);
}

const char* SaslAuthenticatorImpl::getErrorMessage(int errorCode) {
switch (errorCode) {
case SASL_NOMECH:
return "No mechanism found that meets requested properties ";
default:
return sasl_errdetail(m_pConnection);
}
}

const std::string &SaslAuthenticatorImpl::getAuthMechanismName() const {
return m_authMechanismName;
}

} /* namespace Drill */
7 changes: 7 additions & 0 deletions contrib/native/client/src/clientlib/saslAuthenticatorImpl.hpp
View file
Open in desktop
Expand Up @@ -55,6 +55,10 @@ class SaslAuthenticatorImpl {

int unwrap(const char* dataToUnWrap, const int& dataToUnWrapLen, const char** output, uint32_t& unWrappedLen);

const std::string &getAuthMechanismName() const;

const char *getErrorMessage(int errorCode);

private:

static const std::map<std::string, std::string> MECHANISM_MAPPING;
Expand All @@ -67,11 +71,14 @@ class SaslAuthenticatorImpl {
std::string m_username;
sasl_secret_t *m_ppwdSecret;
EncryptionContext *m_pEncryptCtxt;
std::string m_authMechanismName; // used for debugging/error messages

private:
static int passwordCallback(sasl_conn_t *conn, void *context, int id, sasl_secret_t **psecret);

static int userNameCallback(void *context, int id, const char **result, unsigned int *len);


void setSecurityProps() const;
};

Expand Down
4 changes: 2 additions & 2 deletions contrib/native/client/src/clientlib/utils.cpp
View file
Open in desktop
Expand Up @@ -156,8 +156,8 @@ void EncryptionContext::reset() {

std::ostream& operator<<(std::ostream &contextStream, const EncryptionContext& context) {
contextStream << " Encryption: " << (context.isEncryptionReqd() ? "enabled" : "disabled");
contextStream << " ,MaxWrappedSize: " << context.getMaxWrappedSize();
contextStream << " ,WrapSizeLimit: " << context.getWrapSizeLimit();
contextStream << ", MaxWrappedSize: " << context.getMaxWrappedSize();
contextStream << ", WrapSizeLimit: " << context.getWrapSizeLimit();
return contextStream;
}

Expand Down