While trying to test my RFC 4638 changes for pppoe(4) against a PPPoE
server that won't agree to the increased MTU, I found that after setting
up a pppoe(8)-based server I wasn't getting the expected behaviour.
It turns out pppoe(8) returns all of the clients tags in addition to any
tags specific to the PADO or PADS packets so if a client requests an
increased MTU, pppoe(8) blindly agrees to it by echoing the max payload
tag back.
Patch below adds a new function to scrub out any unknown tags that are
not defined in pppoe.h. With this applied I get the correct negotiation.
Matt
--- src/usr.sbin/pppoe/server.c.orig Wed Jan 4 21:34:58 2012
+++ src/usr.sbin/pppoe/server.c Thu Jan 5 06:49:47 2012
@@ -72,6 +72,8 @@
static u_int8_t *key_make(u_int8_t *, int, u_int8_t *, int);
static int key_cmp(u_int8_t *, int, u_int8_t *, int, u_int8_t *, int);
+static u_long scrub_pkt(u_long, u_int8_t *);
+
void
server_mode(int bpffd, u_int8_t *sysname, u_int8_t *srvname,
struct ether_addr *ea)
@@ -302,6 +304,7 @@
struct pppoe_header *ph, u_long pktlen, u_int8_t *pktbuf)
{
struct pppoe_tag ktag, htag;
+ u_long newlen;
u_int8_t hn[MAXHOSTNAMELEN];
u_int8_t *k = NULL;
struct iovec v[7];
@@ -315,8 +318,14 @@
ph->code = PPPOE_CODE_PADO;
v[idx].iov_base = ph; v[idx].iov_len = sizeof(*ph); idx++;
- v[idx].iov_base = pktbuf; v[idx].iov_len = pktlen; idx++;
+ /* Scrub unknown tags from the packet before returning them */
+ if ((newlen = scrub_pkt(pktlen, pktbuf)) == 0)
+ return;
+ if (newlen != pktlen)
+ ph->len -= pktlen - newlen;
+ v[idx].iov_base = pktbuf; v[idx].iov_len = newlen; idx++;
+
if (gethostname((char *)hn, sizeof(hn)) < 0)
return;
htag.len = strlen((char *)hn);
@@ -383,6 +392,7 @@
struct ether_header *eh, struct pppoe_header *ph,
u_long pktlen, u_int8_t *pktbuf)
{
+ u_long newlen;
u_int8_t hn[MAXHOSTNAMELEN];
struct iovec v[16];
struct pppoe_session *s;
@@ -404,8 +414,14 @@
return;
v[idx].iov_base = ph; v[idx].iov_len = sizeof(*ph); idx++;
- v[idx].iov_base = pktbuf; v[idx].iov_len = pktlen; idx++;
+ /* Scrub unknown tags from the packet before returning them */
+ if ((newlen = scrub_pkt(pktlen, pktbuf)) == 0)
+ return;
+ if (newlen != pktlen)
+ ph->len -= pktlen - newlen;
+ v[idx].iov_base = pktbuf; v[idx].iov_len = newlen; idx++;
+
htag.len = strlen((char *)hn);
htag.type = htons(PPPOE_TAG_AC_NAME);
htag.val = hn;
@@ -446,4 +462,57 @@
out:
tag_destroy(&tl);
+}
+
+static u_long
+scrub_pkt(u_long pktlen, u_int8_t *pkt)
+{
+ u_long newlen = 0;
+ u_int16_t ttype, tlen;
+ u_int8_t *p = pkt;
+
+ while (pktlen != 0) {
+ if (pktlen < sizeof(u_int16_t))
+ break;
+ ttype = pkt[1] | (pkt[0] << 8);
+ pkt += sizeof(u_int16_t);
+ pktlen -= sizeof(u_int16_t);
+
+ if (pktlen < sizeof(u_int16_t))
+ break;
+ tlen = pkt[1] | (pkt[0] << 8);
+ pkt += sizeof(u_int16_t);
+ pktlen -= sizeof(u_int16_t);
+
+ if (pktlen < tlen)
+ break;
+
+ pkt += tlen;
+ pktlen -= tlen;
+
+ switch (ttype) {
+ case PPPOE_TAG_END_OF_LIST:
+ case PPPOE_TAG_SERVICE_NAME:
+ case PPPOE_TAG_AC_NAME:
+ case PPPOE_TAG_HOST_UNIQ:
+ case PPPOE_TAG_AC_COOKIE:
+ case PPPOE_TAG_VENDOR_SPEC:
+ case PPPOE_TAG_RELAY_SESSION:
+ case PPPOE_TAG_SERVICE_NAME_ERROR:
+ case PPPOE_TAG_AC_SYSTEM_ERROR:
+ case PPPOE_TAG_GENERIC_ERROR:
+ p = pkt;
+ newlen += sizeof(u_int16_t) + sizeof(u_int16_t) + tlen;
+ break;
+ default:
+ if (pktlen != 0)
+ bcopy(pkt, p, pktlen);
+ pkt = p;
+ break;
+ }
+ }
+
+ if (pktlen != 0)
+ return (0);
+ return (newlen);
}
