npppd(8) and PROXY_AUTHEN_CHALLENGE bad length with Juniper
Hello!
Full disclosure: this took place over the course of about a month, and
I've done my best to include the relevant info..
Unsure if this is really a bug, and I don't have a real diff for a fix, just a
work-around, so misc it is.
This is done with OpenBSD 6.8-stable, syspatch 001 through 012 installed.
We considered trying -current, but noticed no activity in the npppd tree
that might make a difference.
'old' and 'new' equipment types from upstream are both Juniper, though
unsure of exact models. Old should be Juniper ERX of some type, new
I only know this from packet capture: Juniper Networks/Unisphere(4874).
I work for a small ISP and we are exploring the use of npppd(8) for
termination of L2TP with incumbent for xDSL connections.
Working with the provider, their 'old' equipment works fine[1], however,
the 'new' network would always cause these errors upon receipt of Proxy AVP:
Feb 5 14:13:13 edge9 npppd[86416]: l2tpd ctrl=2359 call=2685 Received bad
ICCN: Attribute value is too long PROXY_AUTHEN_CHALLENGE 33 > 24
Feb 5 14:13:13 edge9 npppd[86416]: l2tpd ctrl=2359 call=2685 SendCDN
result=ERROR_CODE/2 error=WRONG_LENGTH/2 messsage=none
Looking at RFC 2661, I can't actually figure where a limit of 24 is imposed,
though I was tricked as I counted the bits on top of their chart which does
hit 32, nothing in the surrounding text actually dictates this size limit:
>From RFC 2661, "Proxy Authen Challenge (ICCN)" near page 37:
0 1 2 3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| Challenge... (arbitrary number of octets) |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
Looking at npppd's l2tp_subr.h where the error above comes from:
#define AVP_MAXLEN_CHECK(_avp, _maxlen) \
do {\
if ((_avp)->length > (_maxlen) + 6) { \
snprintf(emes, sizeof(emes),\
"Attribute value is too long %s %d > %d", \
avp_attr_type_string((_avp)->attr_type),\
(_avp)->length - 6, (int)(_maxlen));\
goto size_check_failed; \
} \
} while (/* CONSTCOND */0)
I drew wild conclusions that perhaps 24 + 6, which lines up with counting
the bits on the RFC chart above, is how the limit was chosen.
After many more days of packet captures and head scratching, I specifically
saw that in the RADIUS packet, within the Attribute Value Pairs, it was
CHAP-Challenge, type 60, that was overflowing this size check.
On to RFC 2865, page 59: https://tools.ietf.org/html/rfc2865#page-59
0 1 2
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-
| Type |Length |String...
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-
Aha, this chart actually hits exactly 24. Still no mention of a hard
size limit, the only thing it dictates is:
Length
>= 7
Ultimately we managed to get this working by simply omitting the size check
as such:
Index: l2tp/l2tp_call.c
===
RCS file: /cvs/src/usr.sbin/npppd/l2tp/l2tp_call.c,v
retrieving revision 1.19
diff -u -p -r1.19 l2tp_call.c
--- l2tp/l2tp_call.c5 Dec 2015 16:10:31 - 1.19
+++ l2tp/l2tp_call.c5 Mar 2021 17:46:12 -
@@ -546,7 +546,8 @@ l2tp_call_recv_ICCN(l2tp_call *_this, u_
dpi->last_recv_lcp.ldata = avp_attr_length(avp);
break;
case L2TP_AVP_TYPE_PROXY_AUTHEN_CHALLENGE:
- AVP_MAXLEN_CHECK(avp, sizeof(dpi->auth_chall));
+ /* XXX: disable to try and skirt 'too long' errors */
+ /* AVP_MAXLEN_CHECK(avp, sizeof(dpi->auth_chall)); */
memcpy(dpi->auth_chall, avp->attr_value,
avp_attr_length(avp));
dpi->lauth_chall = avp_attr_length(avp);
We've been running this modified npppd for a week now, our upstream is happy
that they can continue phasing out their 'old' gear, and our endusers are
able to get online.
Neither myself nor my colleague can figure out how '24' is chosen for _maxlen,
but as this finally got us moving forward, wanted to share what we had and see
if we are on the right track.
I am thinking that we would want a maximum length set there still, but perhaps
it can be pushed up? I've seen many of those error types, all seem to stay
below 100:
Feb 8 11:42:53 edge9 npppd[86416]:
Re: AX201 Surface Pro 7
On Fri, Mar 05, 2021 at 04:25:53PM +0100, Fredrik Engberg wrote: > Hey, > > I had no luck with the "Qu-b0-hr-b0-48" firmware. But I had to change to > "Qu-c0-hr-b0-48" and that seems to work. Here it is the changes I had to do > to get it working. I might have done something wrong here so please point it > out to me. > Great! Thank for you doing this :) Your patch looks fine. Now we are only missing a patch for the sysutils/firmware/iwx port such that this new firmware file can be installed with fw_update. Ideally, the firmware port should be updated before support for this new device is added to the driver. And fw_update will need an updated and signed firmware package on the mirrors. The person who currently takes care of uploading the official firmware packages is sthen@ so please get in touch with him if you want to finish the job. Regards, Stefan
Re: 6.8 with gnome boots to xterm after upgrade
Hi Sivan, Sorry I've not had chance to look at everything you sent. Firstly the message about SSH keys sounds normal as this is part of a normal X session startup. I suspect you have a key that has changed or needs a passphrase entered and it's just picking it up when you try to start X. The command history looks strange, you're running shutdown and reboot and then other commands, unless these are from another session? Openbsd needs the -h option to both shutdown and power off the machine or -r for reboot. Where is your startx program and is it a custom program? If you have done unintended upgrades and your /usr is also full it's going to cause all sorts of problems. I would recommend reinstalling a release from scratch if you can. Alternatively when the boot program runs you can choose bad.rd to get the installer ramdisk and manually repair from there but it's a rather complex process. On my system I had to boot bad.rd, type s for shell, run the MAKEDEV script in /dev to create device nodes and then run disklabel manually to rearrange volumes to make space. You would also need to grow or shrink the volumes. Regarding further troubleshooting of X sessions I would recommend moving .xsession to .xsession.bak and starting with a fresh configuration. I would need to understand more about how you are starting gnome like more details of any changes you made to the standard installation. Regards Ed Gray On Fri, 5 Mar 2021, 12:03 am Sivan !, wrote: > Dear Stuart Henderson. > > I ran sysmerge. > > I posted, earlier in this thread, 11 images in response to Ed Gray's > comment that I had not shared sufficient details. In addition there > are four more images attached here that I think are important. > > One of these four images show the output of sysmerge and startx commands. > Another is a screenshot of a strange prompt that appears before boot, > it asks for the ssh password - not an encryption password, which > might be understandable, if I had an encrypted disk, I haven't > encrypted -- so why does it ask for the ssh password, before asking > for a login password in X Term? > Two more pictures show the reboot sequence that is some sort of a loop > when shutdown now command is issued as user or root, from x Term, then > the main screen command line is seen flashing the status, and > invariably reboots the system in X Term. This happened in gnome (or > gde) before the accidental upgrade to 6.9 beta and happens in x Term > in 6.9 beta. > > Thank you. > > On Thu, 4 Mar 2021 at 14:10, Stuart Henderson wrote: > > > > On 2021-03-03, Sivan ! wrote: > > > After sysupgrade -s, during which there were two or more automatic > > > reboots, freebsd, upgraded to 6.9 booted after asking password for ssh > key, > > > and started with xvterm console. Startx attempted to switch to gui, but > > > returned errors. > > > > > > Please advise. > > > > > > Thank you > > > > > > > Make sure you have run sysmerge. > > > > If that doesn't help then we need more than just "returned errors" - > *what* errors? > > >
Re: AX201 Surface Pro 7
Hey,
I had no luck with the "Qu-b0-hr-b0-48" firmware. But I had to change to
"Qu-c0-hr-b0-48" and that seems to work. Here it is the changes I had to do to
get it working. I might have done something wrong here so please point it out
to me.
Index: if_iwx.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwx.c,v
retrieving revision 1.49
diff -u -p -u -p -r1.49 if_iwx.c
--- if_iwx.c17 Jan 2021 14:24:00 - 1.49
+++ if_iwx.c28 Feb 2021 07:18:44 -
@@ -7710,6 +7710,7 @@ static const struct pci_matchid iwx_devi
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_22500_1 },
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_22500_2 },
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_22500_3 },
+ { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_22500_4,},
};
static const struct pci_matchid iwx_subsystem_id_ax201[] = {
@@ -7749,6 +7750,7 @@ iwx_match(struct device *parent, iwx_mat
return 1; /* match any device */
case PCI_PRODUCT_INTEL_WL_22500_2: /* AX201 */
case PCI_PRODUCT_INTEL_WL_22500_3: /* AX201 */
+ case PCI_PRODUCT_INTEL_WL_22500_4: /* AX201 */
for (i = 0; i < nitems(iwx_subsystem_id_ax201); i++) {
if (svid == iwx_subsystem_id_ax201[i].pm_vid &&
spid == iwx_subsystem_id_ax201[i].pm_pid)
@@ -7951,6 +7953,17 @@ iwx_attach(struct device *parent, struct
sc->sc_tx_with_siso_diversity = 0;
sc->sc_uhb_supported = 0;
break;
+ case PCI_PRODUCT_INTEL_WL_22500_4:
+ sc->sc_fwname = "iwx-Qu-c0-hr-b0-48";
+ sc->sc_device_family = IWX_DEVICE_FAMILY_22000;
+ sc->sc_fwdmasegsz = IWX_FWDMASEGSZ_8000;
+ sc->sc_integrated = 1;
+ sc->sc_ltr_delay = IWX_SOC_FLAGS_LTR_APPLY_DELAY_200;
+ sc->sc_low_latency_xtal = 0;
+ sc->sc_xtal_latency = 5000;
+ sc->sc_tx_with_siso_diversity = 0;
+ sc->sc_uhb_supported = 0;
+ break;
default:
printf("%s: unknown adapter type\n", DEVNAME(sc));
return;
Index: pcidevs
===
RCS file: /cvs/src/sys/dev/pci/pcidevs,v
retrieving revision 1.1959
diff -u -p -u -p -r1.1959 pcidevs
--- pcidevs 27 Feb 2021 03:00:54 - 1.1959
+++ pcidevs 28 Feb 2021 07:18:44 -
@@ -5942,6 +5942,7 @@ product INTEL 500SERIES_LP_XHCI 0xa0ed 5
product INTEL 500SERIES_LP_XDCI0xa0ee 500 Series xDCI
product INTEL 500SERIES_LP_SRAM0xa0ef 500 Series Shared SRAM
product INTEL WL_22500_3 0xa0f0 Wi-Fi 6 AX201
+product INTEL WL_22500_4 0x34f0 Killer Wi-Fi AX1650i (201NGW)
product INTEL 500SERIES_LP_GSPI_3 0xa0fb 500 Series GSPI
product INTEL 500SERIES_LP_ISH 0xa0fc 500 Series ISH
product INTEL 500SERIES_LP_GSPI_4 0xa0fd 500 Series GSPI
Index: pcidevs.h
===
RCS file: /cvs/src/sys/dev/pci/pcidevs.h,v
retrieving revision 1.1953
diff -u -p -u -p -r1.1953 pcidevs.h
--- pcidevs.h 27 Feb 2021 03:01:25 - 1.1953
+++ pcidevs.h 28 Feb 2021 07:18:44 -
@@ -5947,6 +5947,7 @@
#definePCI_PRODUCT_INTEL_500SERIES_LP_XDCI 0xa0ee /* 500
Series xDCI */
#definePCI_PRODUCT_INTEL_500SERIES_LP_SRAM 0xa0ef /* 500
Series Shared SRAM */
#definePCI_PRODUCT_INTEL_WL_22500_30xa0f0 /* Wi-Fi 6
AX201 */
+#definePCI_PRODUCT_INTEL_WL_22500_40x34f0 /* Killer Wi-Fi
6 AX16501i (201NGW) */
#definePCI_PRODUCT_INTEL_500SERIES_LP_GSPI_3 0xa0fb /* 500
Series GSPI */
#definePCI_PRODUCT_INTEL_500SERIES_LP_ISH 0xa0fc /* 500
Series ISH */
#definePCI_PRODUCT_INTEL_500SERIES_LP_GSPI_4 0xa0fd /* 500
Series GSPI */
Index: pcidevs_data.h
===
RCS file: /cvs/src/sys/dev/pci/pcidevs_data.h,v
retrieving revision 1.1948
diff -u -p -u -p -r1.1948 pcidevs_data.h
--- pcidevs_data.h 27 Feb 2021 03:01:25 - 1.1948
+++ pcidevs_data.h 28 Feb 2021 07:18:45 -
@@ -21208,6 +21208,10 @@ static const struct pci_known_product pc
"500 Series Shared SRAM",
},
{
+ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_22500_4,
+ "Killer Wi-Fi 6 AX1650i (201NGW)",
+ },
+ {
PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_22500_3,
"Wi-Fi 6 AX201",
},
On Fri, Feb 5, 2021 at 3:12 PM Stefan Sperling wrote:
On Fri, Feb 05, 2021 at 11:38:24AM +0100, Fredrik Engberg wrote:
> Hey
>
> I got myself a Surface Pro 7 and thought it had a supported AX201 wifi chip
> in it but after some looking around in the source I couldn’t find the device
> ID in there so I tried myself to add it to pcidevs and pcidevs.h.
> I also added the
Re: AX201 Surface Pro 7
Hey,
I had no luck with the "Qu-b0-hr-b0-48" firmware. But if I changed to
"Qu-c0-hr-b0-48" and that seems to work. Here it is the changes I had to do to
get it working. I might have done something wrong here so please point it out
to me.
Index: if_iwx.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwx.c,v
retrieving revision 1.49
diff -u -p -u -p -r1.49 if_iwx.c
--- if_iwx.c17 Jan 2021 14:24:00 - 1.49
+++ if_iwx.c28 Feb 2021 07:18:44 -
@@ -7710,6 +7710,7 @@ static const struct pci_matchid iwx_devi
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_22500_1 },
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_22500_2 },
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_22500_3 },
+ { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_22500_4,},
};
static const struct pci_matchid iwx_subsystem_id_ax201[] = {
@@ -7749,6 +7750,7 @@ iwx_match(struct device *parent, iwx_mat
return 1; /* match any device */
case PCI_PRODUCT_INTEL_WL_22500_2: /* AX201 */
case PCI_PRODUCT_INTEL_WL_22500_3: /* AX201 */
+ case PCI_PRODUCT_INTEL_WL_22500_4: /* AX201 */
for (i = 0; i < nitems(iwx_subsystem_id_ax201); i++) {
if (svid == iwx_subsystem_id_ax201[i].pm_vid &&
spid == iwx_subsystem_id_ax201[i].pm_pid)
@@ -7951,6 +7953,17 @@ iwx_attach(struct device *parent, struct
sc->sc_tx_with_siso_diversity = 0;
sc->sc_uhb_supported = 0;
break;
+ case PCI_PRODUCT_INTEL_WL_22500_4:
+ sc->sc_fwname = "iwx-Qu-c0-hr-b0-48";
+ sc->sc_device_family = IWX_DEVICE_FAMILY_22000;
+ sc->sc_fwdmasegsz = IWX_FWDMASEGSZ_8000;
+ sc->sc_integrated = 1;
+ sc->sc_ltr_delay = IWX_SOC_FLAGS_LTR_APPLY_DELAY_200;
+ sc->sc_low_latency_xtal = 0;
+ sc->sc_xtal_latency = 5000;
+ sc->sc_tx_with_siso_diversity = 0;
+ sc->sc_uhb_supported = 0;
+ break;
default:
printf("%s: unknown adapter type\n", DEVNAME(sc));
return;
Index: pcidevs
===
RCS file: /cvs/src/sys/dev/pci/pcidevs,v
retrieving revision 1.1959
diff -u -p -u -p -r1.1959 pcidevs
--- pcidevs 27 Feb 2021 03:00:54 - 1.1959
+++ pcidevs 28 Feb 2021 07:18:44 -
@@ -5942,6 +5942,7 @@ product INTEL 500SERIES_LP_XHCI 0xa0ed 5
product INTEL 500SERIES_LP_XDCI0xa0ee 500 Series xDCI
product INTEL 500SERIES_LP_SRAM0xa0ef 500 Series Shared SRAM
product INTEL WL_22500_3 0xa0f0 Wi-Fi 6 AX201
+product INTEL WL_22500_4 0x34f0 Killer Wi-Fi AX1650i (201NGW)
product INTEL 500SERIES_LP_GSPI_3 0xa0fb 500 Series GSPI
product INTEL 500SERIES_LP_ISH 0xa0fc 500 Series ISH
product INTEL 500SERIES_LP_GSPI_4 0xa0fd 500 Series GSPI
Index: pcidevs.h
===
RCS file: /cvs/src/sys/dev/pci/pcidevs.h,v
retrieving revision 1.1953
diff -u -p -u -p -r1.1953 pcidevs.h
--- pcidevs.h 27 Feb 2021 03:01:25 - 1.1953
+++ pcidevs.h 28 Feb 2021 07:18:44 -
@@ -5947,6 +5947,7 @@
#definePCI_PRODUCT_INTEL_500SERIES_LP_XDCI 0xa0ee /* 500
Series xDCI */
#definePCI_PRODUCT_INTEL_500SERIES_LP_SRAM 0xa0ef /* 500
Series Shared SRAM */
#definePCI_PRODUCT_INTEL_WL_22500_30xa0f0 /* Wi-Fi 6
AX201 */
+#definePCI_PRODUCT_INTEL_WL_22500_40x34f0 /* Killer Wi-Fi
6 AX16501i (201NGW) */
#definePCI_PRODUCT_INTEL_500SERIES_LP_GSPI_3 0xa0fb /* 500
Series GSPI */
#definePCI_PRODUCT_INTEL_500SERIES_LP_ISH 0xa0fc /* 500
Series ISH */
#definePCI_PRODUCT_INTEL_500SERIES_LP_GSPI_4 0xa0fd /* 500
Series GSPI */
Index: pcidevs_data.h
===
RCS file: /cvs/src/sys/dev/pci/pcidevs_data.h,v
retrieving revision 1.1948
diff -u -p -u -p -r1.1948 pcidevs_data.h
--- pcidevs_data.h 27 Feb 2021 03:01:25 - 1.1948
+++ pcidevs_data.h 28 Feb 2021 07:18:45 -
@@ -21208,6 +21208,10 @@ static const struct pci_known_product pc
"500 Series Shared SRAM",
},
{
+ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_22500_4,
+ "Killer Wi-Fi 6 AX1650i (201NGW)",
+ },
+ {
PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_22500_3,
"Wi-Fi 6 AX201",
},
On Fri, Feb 5, 2021 at 3:12 PM Stefan Sperling wrote:
On Fri, Feb 05, 2021 at 11:38:24AM +0100, Fredrik Engberg wrote:
> Hey
>
> I got myself a Surface Pro 7 and thought it had a supported AX201 wifi chip
> in it but after some looking around in the source I couldn’t find the device
> ID in there so I tried myself to add it to pcidevs and pcidevs.h.
> I also added the
