Skip to content

Commit

Permalink
Set custom error message inside connection close method for unauthori…
Browse files Browse the repository at this point in the history
…zed clients
  • Loading branch information
Chinmay1412 committed Nov 26, 2021
1 parent a735d9f commit ea61fbb
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 38 deletions.
23 changes: 15 additions & 8 deletions libamqpprox/amqpprox_connector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,17 +292,23 @@ void Connector::synthesizeClose(bool sendToIngressSide)
{
d_state = sendToIngressSide ? State::CLIENT_CLOSE_SENT
: State::SERVER_CLOSE_SENT;
synthesizeMessage<Reply::OK>(d_close, sendToIngressSide);
synthesizeMessage(
d_close, sendToIngressSide, Reply::Codes::reply_success, "OK");
}

void Connector::synthesizeCloseError(bool sendToIngressSide)
{
synthesizeMessage<Reply::CloseOkExpected>(d_close, sendToIngressSide);
synthesizeMessage(d_close,
sendToIngressSide,
Reply::Codes::channel_error,
"ERROR: Expected CloseOk reply");
}

void Connector::synthesizeCloseAuthError(bool sendToIngressSide)
void Connector::synthesizeCustomCloseError(bool sendToIngressSide,
uint16_t code,
std::string_view text)
{
synthesizeMessage<Reply::CloseAuthDeny>(d_close, sendToIngressSide);
synthesizeMessage(d_close, sendToIngressSide, code, text);
}

Buffer Connector::outBuffer()
Expand Down Expand Up @@ -387,12 +393,13 @@ void Connector::sendResponse(const T &response, bool sendToIngressSide)
}
}

template <typename TReply, typename TMethod>
inline void Connector::synthesizeMessage(TMethod &replyMethod,
bool sendToIngressSide)
void Connector::synthesizeMessage(methods::Close & replyMethod,
bool sendToIngressSide,
uint64_t code,
std::string_view text)
{
d_buffer = Buffer(); // forget inbound buffer
replyMethod.setReply(TReply::CODE, TReply::TEXT);
replyMethod.setReply(code, std::string(text));
sendResponse(replyMethod, sendToIngressSide);
}

Expand Down
18 changes: 12 additions & 6 deletions libamqpprox/amqpprox_connector.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <amqpprox_methods_startok.h>
#include <amqpprox_methods_tune.h>
#include <amqpprox_methods_tuneok.h>
#include <amqpprox_reply.h>

#include <functional>
#include <string_view>
Expand Down Expand Up @@ -101,9 +102,10 @@ class Connector {
template <typename T>
void sendResponse(const T &response, bool sendToIngressSide);

template <typename TReply, typename TMethod>
inline void synthesizeMessage(TMethod &replyMethod,
bool sendToIngressSide);
inline void synthesizeMessage(methods::Close & replyMethod,
bool sendToIngressSide,
uint64_t code,
std::string_view text);

public:
Connector(SessionState * sessionState,
Expand Down Expand Up @@ -159,12 +161,16 @@ class Connector {
void synthesizeCloseError(bool sendToIngressSide);

/**
* \brief Send AMQP connection Close method with ERROR status to
* client/server based on specified direction
* \brief Send AMQP connection Close method with custom ERROR code and text
* to client/server based on specified direction
* \param sendToIngressSide true for communicating with client and false
* for communicating with server
* \param code custom error code
* \param text custom error text
*/
void synthesizeCloseAuthError(bool sendToIngressSide);
void synthesizeCustomCloseError(bool sendToIngressSide,
uint16_t code,
std::string_view text);

/**
* \brief Synthesize AMQP protocol header buffer, which will eventually be
Expand Down
16 changes: 0 additions & 16 deletions libamqpprox/amqpprox_reply.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,22 +50,6 @@ struct Codes {
};
};

struct OK {
constexpr static const uint16_t CODE = Codes::reply_success;
constexpr static const char *const TEXT = "OK";
};

struct CloseAuthDeny {
constexpr static const uint16_t CODE = Codes::access_refused;
constexpr static const char *const TEXT =
"ERROR: Not authorized by amqpprox proxy";
};

struct CloseOkExpected {
constexpr static const uint16_t CODE = Codes::channel_error;
constexpr static const char *const TEXT = "ERROR: Expected CloseOk reply";
};

}
}
}
Expand Down
13 changes: 9 additions & 4 deletions libamqpprox/amqpprox_session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include <amqpprox_method.h>
#include <amqpprox_packetprocessor.h>
#include <amqpprox_proxyprotocolheaderv1.h>
#include <amqpprox_reply.h>
#include <amqpprox_tlsutil.h>
#include <authrequest.pb.h>
#include <authresponse.pb.h>
Expand Down Expand Up @@ -420,7 +421,8 @@ void Session::establishConnection()
LOG_ERROR << "Disconnecting unauthenticated/unauthorized client, "
"reason: "
<< authResponseData.reason();
disconnectUnauthClient(d_connector.getClientProperties());
disconnectUnauthClient(d_connector.getClientProperties(),
authResponseData.reason());
return;
}
else if (authResponseData.result() == authproto::AuthResponse::ALLOW) {
Expand All @@ -443,7 +445,8 @@ void Session::establishConnection()
"service, isAllowed: "
<< static_cast<int>(authResponseData.result())
<< ", reason: " << authResponseData.reason();
disconnectUnauthClient(d_connector.getClientProperties());
disconnectUnauthClient(d_connector.getClientProperties(),
authResponseData.reason());
return;
}
};
Expand Down Expand Up @@ -479,7 +482,8 @@ void Session::pause()
}
}

void Session::disconnectUnauthClient(const FieldTable &clientProperties)
void Session::disconnectUnauthClient(const FieldTable &clientProperties,
std::string_view reason)
{
d_sessionState.setAuthDeniedConnection(true);
std::shared_ptr<FieldTable> capabilitiesTable;
Expand All @@ -492,7 +496,8 @@ void Session::disconnectUnauthClient(const FieldTable &clientProperties)
fv.type() == 't') {
bool authenticationFailureClose = fv.value<bool>();
if (authenticationFailureClose) {
d_connector.synthesizeCloseAuthError(true);
d_connector.synthesizeCustomCloseError(
true, Reply::Codes::access_refused, reason);
sendSyntheticData();
}
}
Expand Down
4 changes: 3 additions & 1 deletion libamqpprox/amqpprox_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,10 @@ class Session : public std::enable_shared_from_this<Session> {
* https://www.rabbitmq.com/auth-notification.html
* \param clientProperties will be used to find the client
* 'authentication_failure_close' capability value
* \param reason custom reason/message for unauthorized client
*/
void disconnectUnauthClient(const FieldTable &clientProperties);
void disconnectUnauthClient(const FieldTable &clientProperties,
std::string_view reason);

/**
* \brief Disconnect the session from the backend, but leave the client
Expand Down
6 changes: 3 additions & 3 deletions tests/amqpprox_session.t.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,8 @@ void SessionTest::testSetupUnauthClientOpenWithShutdown(
d_serverState.pushItem(idx, Data(encode(clientTuneOk())));
d_serverState.pushItem(idx, Data(encode(clientOpen())));
methods::Close closeMethod = methods::Close();
closeMethod.setReply(Reply::CloseAuthDeny::CODE,
Reply::CloseAuthDeny::TEXT);
closeMethod.setReply(Reply::Codes::access_refused,
"Unauthorized test client");
d_serverState.expect(
idx,
[this, closeMethod, authenticationFailureClose](const auto &items) {
Expand Down Expand Up @@ -1788,6 +1788,6 @@ methods::CloseOk SessionTest::closeOk()
methods::Close SessionTest::close()
{
auto result = methods::Close();
result.setReply(Reply::OK::CODE, Reply::OK::TEXT);
result.setReply(Reply::Codes::reply_success, "OK");
return result;
}

0 comments on commit ea61fbb

Please sign in to comment.