Skip to content

Commit

Permalink
Merge pull request #84 from bloomberg/short-str
Browse files Browse the repository at this point in the history
Limit reason field to 255 characters in authResponse, while sending close method to amqp clients
  • Loading branch information
Chinmay1412 authored Jun 8, 2022
2 parents 931b3b1 + 9d510e3 commit 5887299
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 3 deletions.
2 changes: 1 addition & 1 deletion authproto/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ The service will receive POST request from amqpprox proxy with following fields
### Response
The service should respond to amqpprox proxy with following fields in [protobuf format](./authresponse.proto)
- Auth Result - This will decide, whether to allow or deny clients. It is a enum type, takes ALLOW or DENY value only.
- Reason - Reason for the returned auth result. It is an optional field, but good to specify.
- Reason - Reason for the returned auth result. It is an optional field, but good to specify. The reason(reply-text) field is defined as short string in AMQP 0.9.1 protocol implementation. So it will be truncated before sending to the amqp clients, if the size of the string is more than 255 characters.
- SASL auth data - It is an optional field. In case of absence, the START-OK AMQP connection method, received from clients will be sent to the broker without any modification during handshake.
- Broker mechanism - Authentication mechanism field for START-OK AMQP connection method. This will be injected into START-OK AMQP connection method to send to the broker during handshake. E.g. PLAIN
- Credentials - Response field for START-OK AMQP connection method. This will also be injectd into START-OK AMQP connection method to send to the broker during handshake. E.g.'\0user\0password'
Expand Down
2 changes: 2 additions & 0 deletions authproto/authresponse.proto
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ message AuthResponse {
DENY = 1;
}
AuthResult result = 1;

// Maximum 255 characters
string reason = 2;
SASL authData = 3;
}
2 changes: 2 additions & 0 deletions libamqpprox/amqpprox_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ class Constants {
{
return "authentication_failure_close";
}

static constexpr std::size_t shortStringLimit() { return 255; }
};

}
Expand Down
11 changes: 10 additions & 1 deletion libamqpprox/amqpprox_session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -582,9 +582,18 @@ void Session::disconnectUnauthClient(const FieldTable &clientProperties,
&fv, Constants::authenticationFailureClose()) &&
fv.type() == 't') {
bool authenticationFailureClose = fv.value<bool>();

// Truncate short string longer than 255 characters.
// reply-text(reason) field is defined as short string in AMQP
// 0.9.1 protocol.
const size_t sendSize =
std::min(reason.length(), Constants::shortStringLimit());

if (authenticationFailureClose) {
d_connector.synthesizeCustomCloseError(
true, Reply::Codes::access_refused, reason);
true,
Reply::Codes::access_refused,
reason.substr(0, sendSize));
sendSyntheticData();
}
}
Expand Down
3 changes: 2 additions & 1 deletion libamqpprox/amqpprox_types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <amqpprox_types.h>

#include <amqpprox_buffer.h>
#include <amqpprox_constants.h>
#include <amqpprox_fieldtable.h>
#include <amqpprox_fieldvalue.h>
#include <amqpprox_logging.h>
Expand Down Expand Up @@ -86,7 +87,7 @@ bool Types::encodeShortString(Buffer &buffer, const std::string &string)
return false;
}

if (string.size() > 255) {
if (string.size() > Constants::shortStringLimit()) {
return false;
}

Expand Down

0 comments on commit 5887299

Please sign in to comment.