Re: [PATCH 1/4] Drop the packet in case authentication failed

Hi Eilon,

The patch proposed bellow is terribly insecure. Dropping/discarding a bogus incoming packet MUST be a silent process without any impact on existing FSM otherwise you expose to remote opponent a way to interact with your FSM, this can finally lead to a DoS.

I will not merge this patch.

regs,
Alexandre



On 11 May 2014, at 17:47, Eilon Greenstein <[email protected]> wrote:

> From: Dharmender Garg <[email protected]>
> 
> According to RFC 2338:
> 
> 7.1 Receiving VRRP Packets performed the following functions when a VRRP packet is received:
> - MUST verify that the IP TTL is 255.
> - MUST verify the VRRP version
> - MUST verify that the received packet length is greater than or equal to the VRRP header
> - MUST verify the VRRP checksum
> - MUST perform authentication specified by Auth Type
> 
> If any one of the above checks fails, the receiver MUST discard the packet...
> 
> This patch drops the packet in case authentication specified by Auth Type failed.
> 
> Signed-off-by: Dharmender Garg <[email protected]>
> Signed-off-by: Eilon Greenstein <[email protected]>
> ---
> keepalived/vrrp/vrrp.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/keepalived/vrrp/vrrp.c b/keepalived/vrrp/vrrp.c
> index 2d41ae2..8f4f702 100644
> --- a/keepalived/vrrp/vrrp.c
> +++ b/keepalived/vrrp/vrrp.c
> @@ -295,7 +295,7 @@ vrrp_in_chk(vrrp_t * vrrp, char *buffer)
> 			    - sizeof (vrrp->auth_data);
> 			if (memcmp(pw, vrrp->auth_data, sizeof(vrrp->auth_data)) != 0) {
> 				log_message(LOG_INFO, "receive an invalid passwd!");
> -				return VRRP_PACKET_KO;
> +				return VRRP_PACKET_DROP;
> 			}
> 		}
> 
> @@ -335,7 +335,7 @@ vrrp_in_chk(vrrp_t * vrrp, char *buffer)
> 	if (vrrp->auth_type != hd->auth_type) {
> 		log_message(LOG_INFO, "receive a %d auth, expecting %d!",
> 		       hd->auth_type, vrrp->auth_type);
> -		return VRRP_PACKET_KO;
> +		return VRRP_PACKET_DROP;
> 	}
> 
> 	/* MUST verify that the VRID is valid on the receiving interface_t */
> @@ -893,10 +893,12 @@ vrrp_state_backup(vrrp_t * vrrp, char *buf, int buflen)
> 	hd = vrrp_get_header(vrrp->family, buf, &proto, &saddr);
> 	ret = vrrp_check_packet(vrrp, buf, buflen);
> 
> -	if (ret == VRRP_PACKET_KO || ret == VRRP_PACKET_NULL) {
> +	if (ret == VRRP_PACKET_KO || ret == VRRP_PACKET_NULL || ret == VRRP_PACKET_DROP) {
> 		log_message(LOG_INFO, "VRRP_Instance(%s) ignoring received advertisment..."
> 			            ,  vrrp->iname);
> 		vrrp->ms_down_timer = 3 * vrrp->adver_int + VRRP_TIMER_SKEW(vrrp);
> +		vrrp->state = VRRP_STATE_GOTO_MASTER;
> +		notify_instance_exec(vrrp, VRRP_STATE_GOTO_MASTER);
> 	} else if (hd->priority == 0) {
> 		vrrp->ms_down_timer = VRRP_TIMER_SKEW(vrrp);
> 	} else if (vrrp->nopreempt || hd->priority >= vrrp->effective_priority ||
> -- 
> 1.8.4.3
> 
> 
> ------------------------------------------------------------------------------
> Is your legacy SCM system holding you back? Join Perforce May 7 to find out:
> &#149; 3 signs your SCM is hindering your productivity
> &#149; Requirements for releasing software faster
> &#149; Expert tips and advice for migrating your SCM now
> http://p.sf.net/sfu/perforce
> _______________________________________________
> Keepalived-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/keepalived-devel


------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs