Skip to content

Commit

Permalink
Only print decode errors if we're running in debug mode
Browse files Browse the repository at this point in the history
  • Loading branch information
@arr2036
arr2036 committed Jun 4, 2015
1 parent 4382abb commit 53d083b
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 27 deletions.
59 changes: 36 additions & 23 deletions src/lib/radius.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ RCSID("$Id$")
/*
* Some messages get printed out only in debugging mode.
*/
#define FR_STRERROR_PRINTF if (fr_debug_lvl) fr_strerror_printf
#define FR_DEBUG_STRERROR_PRINTF if (fr_debug_lvl) fr_strerror_printf

#if 0
#define VP_TRACE printf
Expand Down Expand Up @@ -318,7 +318,20 @@ void rad_recv_discard(int sockfd)
(struct sockaddr *)&src, &sizeof_src);
}


/** Basic validation of RADIUS packet header
*
* @note fr_strerror errors are only available if fr_debug_lvl > 0. This is to reduce CPU time
* consumed when discarding malformed packet.
*
* @param[in] sockfd we're reading from.
* @param[out] src_ipaddr of the packet.
* @param[out] src_port of the packet.
* @param[out] code Pointer to where to write the packet code.
* @return
* - -1 on failure.
* - 1 on decode error.
* - >= RADIUS_HDR_LEN on success. This is the packet length as specified in the header.
*/
ssize_t rad_recv_header(int sockfd, fr_ipaddr_t *src_ipaddr, uint16_t *src_port, int *code)
{
ssize_t data_len, packet_len;
Expand All @@ -337,7 +350,7 @@ ssize_t rad_recv_header(int sockfd, fr_ipaddr_t *src_ipaddr, uint16_t *src_port,
* Too little data is available, discard the packet.
*/
if (data_len < 4) {
FR_STRERROR_PRINTF("Expected at least 4 bytes of header data, got %zu bytes", data_len);
FR_DEBUG_STRERROR_PRINTF("Expected at least 4 bytes of header data, got %zu bytes", data_len);
rad_recv_discard(sockfd);

return 1;
Expand All @@ -353,8 +366,8 @@ ssize_t rad_recv_header(int sockfd, fr_ipaddr_t *src_ipaddr, uint16_t *src_port,
* a RADIUS header length: discard it.
*/
if (packet_len < RADIUS_HDR_LEN) {
FR_STRERROR_PRINTF("Expected at least " STRINGIFY(RADIUS_HDR_LEN) " bytes of packet "
"data, got %zu bytes", packet_len);
FR_DEBUG_STRERROR_PRINTF("Expected at least " STRINGIFY(RADIUS_HDR_LEN) " bytes of packet "
"data, got %zu bytes", packet_len);
rad_recv_discard(sockfd);

return 1;
Expand All @@ -364,8 +377,8 @@ ssize_t rad_recv_header(int sockfd, fr_ipaddr_t *src_ipaddr, uint16_t *src_port,
* Anything after 4k will be discarded.
*/
} else if (packet_len > MAX_PACKET_LEN) {
FR_STRERROR_PRINTF("Length field value too large, expected maximum of "
STRINGIFY(MAX_PACKET_LEN) " bytes, got %zu bytes", packet_len);
FR_DEBUG_STRERROR_PRINTF("Length field value too large, expected maximum of "
STRINGIFY(MAX_PACKET_LEN) " bytes, got %zu bytes", packet_len);
rad_recv_discard(sockfd);

return 1;
Expand All @@ -376,7 +389,7 @@ ssize_t rad_recv_header(int sockfd, fr_ipaddr_t *src_ipaddr, uint16_t *src_port,
* Convert AF. If unknown, discard packet.
*/
if (!fr_sockaddr2ipaddr(&src, sizeof_src, src_ipaddr, src_port)) {
FR_STRERROR_PRINTF("Unkown address family");
FR_DEBUG_STRERROR_PRINTF("Unkown address family");
rad_recv_discard(sockfd);

return 1;
Expand Down Expand Up @@ -2271,7 +2284,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
* "The minimum length is 20 ..."
*/
if (packet->data_len < RADIUS_HDR_LEN) {
FR_STRERROR_PRINTF("Malformed RADIUS packet from host %s: too short (received %zu < minimum %d)",
FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: too short (received %zu < minimum %d)",
inet_ntop(packet->src_ipaddr.af,
&packet->src_ipaddr.ipaddr,
host_ipaddr, sizeof(host_ipaddr)),
Expand All @@ -2295,7 +2308,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
*/
if ((hdr->code == 0) ||
(hdr->code >= FR_MAX_PACKET_CODE)) {
FR_STRERROR_PRINTF("Bad RADIUS packet from host %s: unknown packet code %d",
FR_DEBUG_STRERROR_PRINTF("Bad RADIUS packet from host %s: unknown packet code %d",
inet_ntop(packet->src_ipaddr.af,
&packet->src_ipaddr.ipaddr,
host_ipaddr, sizeof(host_ipaddr)),
Expand Down Expand Up @@ -2327,7 +2340,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
* "The minimum length is 20 ..."
*/
if (totallen < RADIUS_HDR_LEN) {
FR_STRERROR_PRINTF("Malformed RADIUS packet from host %s: too short (length %zu < minimum %d)",
FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: too short (length %zu < minimum %d)",
inet_ntop(packet->src_ipaddr.af,
&packet->src_ipaddr.ipaddr,
host_ipaddr, sizeof(host_ipaddr)),
Expand Down Expand Up @@ -2360,7 +2373,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
* i.e. No response to the NAS.
*/
if (packet->data_len < totallen) {
FR_STRERROR_PRINTF("Malformed RADIUS packet from host %s: received %zu octets, packet length says %zu",
FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: received %zu octets, packet length says %zu",
inet_ntop(packet->src_ipaddr.af,
&packet->src_ipaddr.ipaddr,
host_ipaddr, sizeof(host_ipaddr)),
Expand Down Expand Up @@ -2406,7 +2419,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
* attribute header.
*/
if (count < 2) {
FR_STRERROR_PRINTF("Malformed RADIUS packet from host %s: attribute header overflows the packet",
FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: attribute header overflows the packet",
inet_ntop(packet->src_ipaddr.af,
&packet->src_ipaddr.ipaddr,
host_ipaddr, sizeof(host_ipaddr)));
Expand All @@ -2418,7 +2431,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
* Attribute number zero is NOT defined.
*/
if (attr[0] == 0) {
FR_STRERROR_PRINTF("Malformed RADIUS packet from host %s: Invalid attribute 0",
FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: Invalid attribute 0",
inet_ntop(packet->src_ipaddr.af,
&packet->src_ipaddr.ipaddr,
host_ipaddr, sizeof(host_ipaddr)));
Expand All @@ -2431,7 +2444,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
* fields. Anything shorter is an invalid attribute.
*/
if (attr[1] < 2) {
FR_STRERROR_PRINTF("Malformed RADIUS packet from host %s: attribute %u too short",
FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: attribute %u too short",
inet_ntop(packet->src_ipaddr.af,
&packet->src_ipaddr.ipaddr,
host_ipaddr, sizeof(host_ipaddr)),
Expand All @@ -2445,7 +2458,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
* attribute, it's a bad packet.
*/
if (count < attr[1]) {
FR_STRERROR_PRINTF("Malformed RADIUS packet from host %s: attribute %u data overflows the packet",
FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: attribute %u data overflows the packet",
inet_ntop(packet->src_ipaddr.af,
&packet->src_ipaddr.ipaddr,
host_ipaddr, sizeof(host_ipaddr)),
Expand All @@ -2471,7 +2484,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)

case PW_MESSAGE_AUTHENTICATOR:
if (attr[1] != 2 + AUTH_VECTOR_LEN) {
FR_STRERROR_PRINTF("Malformed RADIUS packet from host %s: Message-Authenticator has invalid length %d",
FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: Message-Authenticator has invalid length %d",
inet_ntop(packet->src_ipaddr.af,
&packet->src_ipaddr.ipaddr,
host_ipaddr, sizeof(host_ipaddr)),
Expand Down Expand Up @@ -2500,7 +2513,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
* If not, we complain, and throw the packet away.
*/
if (count != 0) {
FR_STRERROR_PRINTF("Malformed RADIUS packet from host %s: packet attributes do NOT exactly fill the packet",
FR_DEBUG_STRERROR_PRINTF("Malformed RADIUS packet from host %s: packet attributes do NOT exactly fill the packet",
inet_ntop(packet->src_ipaddr.af,
&packet->src_ipaddr.ipaddr,
host_ipaddr, sizeof(host_ipaddr)));
Expand All @@ -2515,7 +2528,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
*/
if ((fr_max_attributes > 0) &&
(num_attributes > fr_max_attributes)) {
FR_STRERROR_PRINTF("Possible DoS attack from host %s: Too many attributes in request (received %d, max %d are allowed).",
FR_DEBUG_STRERROR_PRINTF("Possible DoS attack from host %s: Too many attributes in request (received %d, max %d are allowed).",
inet_ntop(packet->src_ipaddr.af,
&packet->src_ipaddr.ipaddr,
host_ipaddr, sizeof(host_ipaddr)),
Expand All @@ -2536,7 +2549,7 @@ bool rad_packet_ok(RADIUS_PACKET *packet, int flags, decode_fail_t *reason)
* Message-Authenticator attributes.
*/
if (require_ma && !seen_ma) {
FR_STRERROR_PRINTF("Insecure packet from host %s: Packet does not contain required Message-Authenticator attribute",
FR_DEBUG_STRERROR_PRINTF("Insecure packet from host %s: Packet does not contain required Message-Authenticator attribute",
inet_ntop(packet->src_ipaddr.af,
&packet->src_ipaddr.ipaddr,
host_ipaddr, sizeof(host_ipaddr)));
Expand Down Expand Up @@ -2592,7 +2605,7 @@ RADIUS_PACKET *rad_recv(TALLOC_CTX *ctx, int fd, int flags)
* Check for socket errors.
*/
if (data_len < 0) {
FR_STRERROR_PRINTF("Error receiving packet: %s", fr_syserror(errno));
FR_DEBUG_STRERROR_PRINTF("Error receiving packet: %s", fr_syserror(errno));
/* packet->data is NULL */
rad_free(&packet);
return NULL;
Expand All @@ -2605,7 +2618,7 @@ RADIUS_PACKET *rad_recv(TALLOC_CTX *ctx, int fd, int flags)
* packet.
*/
if (packet->data_len > MAX_PACKET_LEN) {
FR_STRERROR_PRINTF("Discarding packet: Larger than RFC limitation of 4096 bytes");
FR_DEBUG_STRERROR_PRINTF("Discarding packet: Larger than RFC limitation of 4096 bytes");
/* packet->data is NULL */
rad_free(&packet);
return NULL;
Expand All @@ -2618,7 +2631,7 @@ RADIUS_PACKET *rad_recv(TALLOC_CTX *ctx, int fd, int flags)
* packet->data == NULL
*/
if ((packet->data_len == 0) || !packet->data) {
FR_STRERROR_PRINTF("Empty packet: Socket is not ready");
FR_DEBUG_STRERROR_PRINTF("Empty packet: Socket is not ready");
rad_free(&packet);
return NULL;
}
Expand Down
8 changes: 4 additions & 4 deletions src/main/listen.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1512,7 +1512,7 @@ static int auth_socket_recv(rad_listen_t *listener)
FR_STATS_INC(auth, total_requests);

if (rcode < 20) { /* RADIUS_HDR_LEN */
ERROR("Receive - %s", fr_strerror());
if (DEBUG_ENABLED) ERROR("Receive - %s", fr_strerror());
FR_STATS_INC(auth, total_malformed_requests);
return 0;
}
Expand Down Expand Up @@ -1548,8 +1548,8 @@ static int auth_socket_recv(rad_listen_t *listener)
rad_recv_discard(listener->fd);
FR_STATS_INC(auth, total_unknown_types);

DEBUG("Invalid packet code %d sent to authentication port from client %s port %d : IGNORED",
code, client->shortname, src_port);
if (DEBUG_ENABLED) ERROR("Receive - Invalid packet code %d sent to authentication port from "
"client %s port %d", code, client->shortname, src_port);
return 0;
} /* switch over packet types */

Expand All @@ -1568,7 +1568,7 @@ static int auth_socket_recv(rad_listen_t *listener)
if (!packet) {
talloc_free(ctx);
FR_STATS_INC(auth, total_malformed_requests);
ERROR("Receive - %s", fr_strerror());
if (DEBUG_ENABLED) ERROR("Receive - %s", fr_strerror());
return 0;
}

Expand Down

0 comments on commit 53d083b

Please sign in to comment.