kern/40690: pppoe(4) doesn't work when PPPoE relays are present

[jgordeev%dir.bg@localhost]

>Number:         40690
>Category:       kern
>Synopsis:       pppoe(4) doesn't work when PPPoE relays are present
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Feb 19 15:00:00 +0000 2009
>Originator:     Jordan Gordeev
>Release:        5.0-RC1
>Organization:
>Environment:
>Description:
The RFC on PPPoE says that if you receive a relay session id TAG, you must copy 
it in responses. The pppoe(4) driver doesn't do that.
As a result, attempts to use pppoe(4) to connect to an Access Concentrator 
behind a PPPoE relay fail. This makes connecting to the Internet impossible for 
some users.

>How-To-Repeat:
Use pppoe(4) to connect to an AC behind a PPPoE relay.
>Fix:
The following patch (adapted from revision 1.15 of OpenBSD's if_pppoe.c) fixes 
the problem.
Index: sys/net/if_pppoe.c
===================================================================
RCS file: /cvs-mirror/nbsd/src/sys/net/if_pppoe.c,v
retrieving revision 1.93
diff -u -r1.93 if_pppoe.c
--- sys/net/if_pppoe.c  15 Oct 2008 20:08:33 -0000      1.93
+++ sys/net/if_pppoe.c  19 Feb 2009 15:53:23 -0000
@@ -138,6 +138,8 @@
        char *sc_concentrator_name;     /* if != NULL: requested concentrator 
id */
        uint8_t *sc_ac_cookie;          /* content of AC cookie we must echo 
back */
        size_t sc_ac_cookie_len;        /* length of cookie data */
+       uint8_t *sc_relay_sid;          /* content of relay SID we must echo 
back */
+       size_t sc_relay_sid_len;        /* length of relay SID data */
 #ifdef PPPOE_SERVER
        uint8_t *sc_hunique;            /* content of host unique we must echo 
back */
        size_t sc_hunique_len;          /* length of host unique */
@@ -283,6 +285,8 @@
                free(sc->sc_service_name, M_DEVBUF);
        if (sc->sc_ac_cookie)
                free(sc->sc_ac_cookie, M_DEVBUF);
+       if (sc->sc_relay_sid)
+               free(sc->sc_relay_sid, M_DEVBUF);
        callout_destroy(&sc->sc_timeout);
        free(sc, M_DEVBUF);
 
@@ -400,6 +404,8 @@
        char *error;
        uint8_t *ac_cookie;
        size_t ac_cookie_len;
+       uint8_t *relay_sid;
+       size_t relay_sid_len;
 #ifdef PPPOE_SERVER
        uint8_t *hunique;
        size_t hunique_len;
@@ -423,6 +429,8 @@
 
        ac_cookie = NULL;
        ac_cookie_len = 0;
+       relay_sid = NULL;
+       relay_sid_len = 0;
 #ifdef PPPOE_SERVER
        hunique = NULL;
        hunique_len = 0;
@@ -528,6 +536,19 @@
                                ac_cookie_len = len;
                        }
                        break;
+               case PPPOE_TAG_RELAYSID:
+                       if (relay_sid == NULL) {
+                               n = m_pulldown(m, off + sizeof(*pt), len,
+                                   &noff);
+                               if (!n) {
+                                       err_msg = "TAG RELAYSID ERROR";
+                                       m = NULL;
+                                       break;
+                               }
+                               relay_sid = mtod(n, char *) + noff;
+                               relay_sid_len = len;
+                       }
+                       break;
                case PPPOE_TAG_SNAME_ERR:
                        err_msg = "SERVICE NAME ERROR";
                        errortag = 1;
@@ -669,6 +690,20 @@
                        sc->sc_ac_cookie_len = ac_cookie_len;
                        memcpy(sc->sc_ac_cookie, ac_cookie, ac_cookie_len);
                }
+               if (relay_sid) {
+                       if (sc->sc_relay_sid)
+                               free(sc->sc_relay_sid, M_DEVBUF);
+                       sc->sc_relay_sid = malloc(relay_sid_len, M_DEVBUF,
+                           M_DONTWAIT);
+                       if (sc->sc_relay_sid == NULL) {
+                               printf("%s: FATAL: could not allocate memory "
+                                   "for relay SID\n",
+                                   sc->sc_sppp.pp_if.if_xname);
+                               goto done;
+                       }
+                       sc->sc_relay_sid_len = relay_sid_len;
+                       memcpy(sc->sc_relay_sid, relay_sid, relay_sid_len);
+               }
                memcpy(&sc->sc_dest, eh->ether_shost, sizeof sc->sc_dest);
                callout_stop(&sc->sc_timeout);
                sc->sc_padr_retried = 0;
@@ -1200,6 +1235,11 @@
                sc->sc_ac_cookie = NULL;
        }
        sc->sc_ac_cookie_len = 0;
+       if (sc->sc_relay_sid) {
+               free(sc->sc_relay_sid, M_DEVBUF);
+               sc->sc_relay_sid = NULL;
+       }
+       sc->sc_relay_sid_len = 0;
 #ifdef PPPOE_SERVER
        if (sc->sc_hunique) {
                free(sc->sc_hunique, M_DEVBUF);
@@ -1251,6 +1291,8 @@
        }
        if (sc->sc_ac_cookie_len > 0)
                len += 2 + 2 + sc->sc_ac_cookie_len;    /* AC cookie */
+       if (sc->sc_relay_sid_len > 0)
+               len += 2 + 2 + sc->sc_relay_sid_len;    /* Relay SID */
        m0 = pppoe_get_mbuf(len + PPPOE_HEADERLEN);
        if (!m0)
                return ENOBUFS;
@@ -1270,6 +1312,12 @@
                memcpy(p, sc->sc_ac_cookie, sc->sc_ac_cookie_len);
                p += sc->sc_ac_cookie_len;
        }
+       if (sc->sc_relay_sid_len > 0) {
+               PPPOE_ADD_16(p, PPPOE_TAG_RELAYSID);
+               PPPOE_ADD_16(p, sc->sc_relay_sid_len);
+               memcpy(p, sc->sc_relay_sid, sc->sc_relay_sid_len);
+               p += sc->sc_relay_sid_len;
+       }
        PPPOE_ADD_16(p, PPPOE_TAG_HUNIQUE);
        PPPOE_ADD_16(p, sizeof(sc));
        memcpy(p, &sc, sizeof sc);
@@ -1508,6 +1556,10 @@
                free(sc->sc_ac_cookie, M_DEVBUF);
                sc->sc_ac_cookie = NULL;
        }
+       if (sc->sc_relay_sid) {
+               free(sc->sc_relay_sid, M_DEVBUF);
+               sc->sc_relay_sid = NULL;
+       }
        sc->sc_ac_cookie_len = 0;
        sc->sc_session = 0;
 }