clean up definition of FR_TACACS_CODE_MAX. Fixes #4856

The CODE_MAX definition should be one more than the last valid
packet code.

The DO_NOT_RESPOND value should be a value which will never show
up in a real packet.

The PACKET_CODE_VALID macro should be used instead of relying on
manual checks of CODE_MAX

diff --git a/share/dictionary/tacacs/dictionary.freeradius.internal b/share/dictionary/tacacs/dictionary.freeradius.internal
index 983fdc89d42181735ff389d188f650bfb153bcb1..a2ec143e98dfca8cc35961334dd188dc325892e6 100644 (file)
--- a/share/dictionary/tacacs/dictionary.freeradius.internal
+++ b/share/dictionary/tacacs/dictionary.freeradius.internal
@@ -24,6 +24,6 @@ VALUE Packet-Type                     Authorization-Request   5
 VALUE  Packet-Type                     Authorization-Reply     6
 VALUE  Packet-Type                     Accounting-Request      7
 VALUE  Packet-Type                     Accounting-Reply        8
-VALUE  Packet-Type                     Do-Not-Respond          9
+VALUE  Packet-Type                     Do-Not-Respond          256
 
 ATTRIBUTE      State                                   65537   octets

diff --git a/src/listen/tacacs/proto_tacacs.c b/src/listen/tacacs/proto_tacacs.c
index 81781435708df2077e7751b5348b3a40577fcad0..a823edd3c25d598d5e8ebdb96c8d1d1c41af9e63 100644 (file)
--- a/src/listen/tacacs/proto_tacacs.c
+++ b/src/listen/tacacs/proto_tacacs.c
@@ -104,7 +104,7 @@ static int type_parse(UNUSED TALLOC_CTX *ctx, void *out, void *parent, CONF_ITEM
        value = cf_pair_value(cp);
 
        dv = fr_dict_enum_by_name(attr_packet_type, value, -1);
-       if (!dv || (dv->value->vb_uint32 >= FR_TACACS_CODE_MAX)) {
+       if (!dv || FR_TACACS_PACKET_CODE_VALID(dv->value->vb_uint32)) {
                cf_log_err(ci, "Unknown TACACS+ packet type '%s'", value);
                return -1;
        }
@@ -341,7 +341,7 @@ static ssize_t mod_encode(void const *instance, request_t *request, uint8_t *buf
         */
        if ((buffer_len == 1) ||
            (request->reply->code == FR_PACKET_TYPE_VALUE_DO_NOT_RESPOND) ||
-           (request->reply->code == 0) || (request->reply->code >= FR_TACACS_CODE_MAX)) {
+           !FR_TACACS_PACKET_CODE_VALID(request->reply->code)) {
                track->do_not_respond = true;
                return 1;
        }
@@ -418,8 +418,7 @@ static int mod_priority_set(void const *instance, uint8_t const *buffer, UNUSED
 {
        proto_tacacs_t const *inst = talloc_get_type_abort_const(instance, proto_tacacs_t);
 
-       fr_assert(buffer[1] != FR_TAC_PLUS_INVALID);
-       fr_assert(buffer[1] < FR_TAC_PLUS_MAX);
+       fr_assert(FR_TACACS_PACKET_CODE_VALID(buffer[1]));
 
        /*
         *      Disallowed packet

diff --git a/src/protocols/tacacs/tacacs.h b/src/protocols/tacacs/tacacs.h
index 8a4cfb75f329b19dc37d08a15eae6ccd3d797721..805f2824fb7e9da54e6d8236b82e3ef16c59da9e 100644 (file)
--- a/src/protocols/tacacs/tacacs.h
+++ b/src/protocols/tacacs/tacacs.h
@@ -280,8 +280,8 @@ typedef enum {
        FR_TACACS_AUTZ_REPLY =          FR_PACKET_TYPE_VALUE_AUTHORIZATION_REPLY,
        FR_TACACS_ACCT_REQUEST =        FR_PACKET_TYPE_VALUE_ACCOUNTING_REQUEST,
        FR_TACACS_ACCT_REPLY =          FR_PACKET_TYPE_VALUE_ACCOUNTING_REPLY,
-       FR_TACACS_DO_NOT_RESPOND =      FR_PACKET_TYPE_VALUE_DO_NOT_RESPOND,
        FR_TACACS_CODE_MAX =            9,
+       FR_TACACS_DO_NOT_RESPOND =      FR_PACKET_TYPE_VALUE_DO_NOT_RESPOND,
 } fr_tacacs_packet_code_t;
 
 #define FR_TACACS_PACKET_CODE_VALID(_code) (((_code) > 0) && ((_code) < FR_TACACS_CODE_MAX))