Hello,
(sendbug didn't work for me, so I am sending using the same format manually)
Description:
Recently, I found an OpenBSD bug where pppoe(4) sends echo-req packets with bad
magic numbers. The following tcpdump trace below shows the bad echo-req packet
with 00-00-00-00:
Nov 1 09:24:59 smyslov /bsd: pppoe0: lcp output <echo-req id=0xea len=8
00-00-00-00>
Nov 1 09:24:59 smyslov /bsd: pppoe0 (8864) state=3, session=0x3d22 output ->
00:90:1a:a3:7e:12, len=16
Nov 1 09:24:59 smyslov /bsd: pppoe0: lcp input(opened): <term-req id=0xbf
len=4 00-00-00-...-00-00-00>
Nov 1 09:24:59 smyslov /bsd: pppoe0: lcp opened->stopping
According to RFC 1661, "A Magic-Number of zero is illegal and MUST always be
Nak'd".
The problem exists in sys/net/if_spppsubr.c:
else if (sp->pp_phase >= PHASE_AUTHENTICATE) {
unsigned long nmagic = htonl (sp->lcp.magic);
sp->lcp.echoid = ++sp->pp_seq;
sppp_cp_send (sp, PPP_LCP, ECHO_REQ,
sp->lcp.echoid, 4, &nmagic);
}
The problem here is that sparc64 is big endian and 64 bit, and passing the
address using '&nmagic' causes sppp_cp_send to see only zeros in the 'first'
four bytes (most significant bytes).
Small test case showing this:
$ cat bigendian.c
#include <stdio.h>
#include <stdint.h>
int main()
{
int j;
uint64_t i = 0x11335577;
uint8_t * p = &i;
for( j = 0; j < sizeof(unsigned long); j++)
printf("%02x ", p[j]);
printf("\n");
return 0;
}
$ cc -o be bigendian.c
bigendian.c: In function 'main':
bigendian.c:8: warning: initialization from incompatible pointer type
$ ./be
00 00 00 00 11 33 55 77
How to repeat the problem:
Problem eventually occurs during any PPPoE connection.
Fix to the problem if known:
Changing nmagic to an int32_t fixes this problem:
$ diff /usr/src/sys/net/if_spppsubr.c.orig /usr/src/sys/net/if_spppsubr.c
4595c4595
< unsigned long nmagic = htonl (sp->lcp.magic);
---
> int32_t nmagic = htonl (sp->lcp.magic);
uint32_t would work too - all that matters is that the 32 bit width is
explicitly set.
Note: this is also what NetBSD does:
https://github.com/jsonn/src/blob/trunk/sys/net/if_spppsubr.c#L4793
