This patch corrects a bug in CCP establishment which can result in a
major security hole.

The bug can cause PPP to NOT install and use a compressor  module for
sending,  even though the compressor is sucessfully negotiated by CCP.
Since encryption is sometimes implemented as a compressor module (e.g.
MPPE), this bug can cause PPP to send cleartext even though encryption
appears to be sucessfully negotiated.

The bug does not always show up--it depends on the order of CCP messages
exchanged during establishment, and therefore is not deterministic.

The specific problem is handling a sent or received CCP ConfReq. A sent
ConfReq should reset my decompressor; a received ConfReq should reset my
compressor. The original code had this logic exactly reversed.

Please forgive if I make a procedural error in submitting this patch;
I'm trying to follow the instructions in the FAQ but this is my first
time. The FAQ said to cc Linus and/or Alan Cox for security issues, so I
am doing that..

I am not currently subscribed to the list so please respond directly.

The patch is attached and also shown below.



--- drivers/net/ppp_generic.c.orig Sat Apr 21 13:33:00 2001
+++ drivers/net/ppp_generic.c Sat Apr 21 13:44:38 2001
@@ -1967,15 +1967,30 @@

  switch (CCP_CODE(dp)) {
  case CCP_CONFREQ:
+
+  /* A ConfReq starts negotiation of compression
+   * in one direction of transmission,
+   * and hence brings it down...but which way?
+   *
+   * Remember:
+   * A ConfReq indicates what the sender would like to receive */
+   */
+  if(inbound)
+   /* He is proposing what I should send */
+   ppp->xstate &= ~SC_COMP_RUN;
+  else
+   /* I am proposing to what he should send */
+   ppp->rstate &= ~SC_DECOMP_RUN;
+
+  break;
+
  case CCP_TERMREQ:
  case CCP_TERMACK:
   /*
-   * CCP is going down - disable compression.
+   * CCP is going down, both directions of transmission
    */
-  if (inbound)
-   ppp->rstate &= ~SC_DECOMP_RUN;
-  else
-   ppp->xstate &= ~SC_COMP_RUN;
+  ppp->rstate &= ~SC_DECOMP_RUN;
+  ppp->xstate &= ~SC_COMP_RUN;
   break;

  case CCP_CONFACK:



--- drivers/net/ppp_generic.c.orig      Sat Apr 21 13:33:00 2001
+++ drivers/net/ppp_generic.c   Sat Apr 21 13:44:38 2001
@@ -1967,15 +1967,30 @@
 
        switch (CCP_CODE(dp)) {
        case CCP_CONFREQ:
+
+               /* A ConfReq starts negotiation of compression 
+                * in one direction of transmission,
+                * and hence brings it down...but which way?
+                *
+                * Remember:
+                * A ConfReq indicates what the sender would like to receive */
+                */
+               if(inbound)
+                       /* He is proposing what I should send */
+                       ppp->xstate &= ~SC_COMP_RUN;
+               else    
+                       /* I am proposing to what he should send */
+                       ppp->rstate &= ~SC_DECOMP_RUN;
+               
+               break;
+               
        case CCP_TERMREQ:
        case CCP_TERMACK:
                /*
-                * CCP is going down - disable compression.
+                * CCP is going down, both directions of transmission 
                 */
-               if (inbound)
-                       ppp->rstate &= ~SC_DECOMP_RUN;
-               else
-                       ppp->xstate &= ~SC_COMP_RUN;
+               ppp->rstate &= ~SC_DECOMP_RUN;
+               ppp->xstate &= ~SC_COMP_RUN;
                break;
 
        case CCP_CONFACK:

Reply via email to