Hi team, I spent some time playing with the code and made the wccp part operational again, to the extent it was working before.
Checking the code I can see many possible improvements, from configurable mask parameters, up to implementing variable timers or preserving connections when caches are added into farm. I could eventually invest effort into this part, but considering the situation you explained before, there is likely no perspective user base. The changes are submitted as PR https://github.com/squid-cache/squid/pull/953 I have tested standard service in hash and mask modes as well as a dynamic service, in a combo of one cache and one router. Regards Andrej On Wed, 08.Dec.21 00:25:04 +0100, Andrej Mikus wrote: > Hi, > > It is sad to hear that the code was released without confirmed test, and the > lack of a feedback it can mean that I am apparently a single user of the > component. > > In this situation it would be perhaps feasible to abandon it entirely instead > of maintaining a neglected feature? At the other hand I can imagine that the > test suite could boot a virtual router and to try peering with it. > > By adding a single debug line I was able to confirm an issue in the function > CheckFieldDataLength. It is using sizeof(header) while header is a pointer, > with no relevance to the wccp packet. This is my diff: > > > diff --git a/src/wccp2.cc b/src/wccp2.cc > index a0629b2b7..c63245f6b 100644 > --- a/src/wccp2.cc > +++ b/src/wccp2.cc > @@ -1142,9 +1142,9 @@ static size_t > CheckFieldDataLength(const FieldHeader *header, const size_t dataLength, > const void *areaStart, const size_t areaSize, const char *error) > { > assert(header); > - const auto dataStart = reinterpret_cast<const char*>(header) + > sizeof(header); > + const auto dataStart = reinterpret_cast<const char*>(header) + > sizeof(FieldHeader); > CheckSectionLength(dataStart, dataLength, areaStart, areaSize, error); > - return sizeof(header) + dataLength; // no overflow after > CheckSectionLength() > + return sizeof(FieldHeader) + dataLength; // no overflow after > CheckSectionLength() > } > > /// Positions the given field at a given start within a given packet area. > @@ -1245,6 +1245,7 @@ wccp2HandleUdp(int sock, void *) > const auto itemHeader = reinterpret_cast<const > wccp2_item_header_t*>(&data[offset]); > const auto itemSize = CheckFieldDataLength(itemHeader, > ntohs(itemHeader->length), > data, data_length, "truncated record"); > + debugs(80, 7, "Offset: " << offset << " Item type: " << > ntohs(itemHeader->type) << " size: " << itemSize << "."); > // XXX: Check "The specified length must be a multiple of 4 octets" > // requirement to avoid unaligned memory reads after the first item. > > > It moved me a bit further, router returned the cache in its view, but there is > likely a similar issue in parsing of capability element: > > > 2021/12/08 00:06:18.686| Initialising all WCCPv2 lists > 2021/12/08 00:06:19.687| 80,6| wccp2.cc(1586) wccp2HereIam: wccp2HereIam: > Called > 2021/12/08 00:06:19.687| 80,5| wccp2.cc(1605) wccp2HereIam: wccp2HereIam: > sending to service id 0 > 2021/12/08 00:06:19.687| 80,3| wccp2.cc(1636) wccp2HereIam: Sending HereIam > packet size 144 > 2021/12/08 00:06:19.689| 80,6| wccp2.cc(1207) wccp2HandleUdp: wccp2HandleUdp: > Called. > 2021/12/08 00:06:19.690| 80,3| wccp2.cc(1231) wccp2HandleUdp: Incoming WCCPv2 > I_SEE_YOU length 176. > 2021/12/08 00:06:19.690| 80,7| wccp2.cc(1248) wccp2HandleUdp: Offset: 0 Item > type: 0 size: 8. > 2021/12/08 00:06:19.690| 80,7| wccp2.cc(1248) wccp2HandleUdp: Offset: 8 Item > type: 1 size: 28. > 2021/12/08 00:06:19.690| 80,7| wccp2.cc(1248) wccp2HandleUdp: Offset: 36 Item > type: 2 size: 24. > 2021/12/08 00:06:19.690| 80,7| wccp2.cc(1248) wccp2HandleUdp: Offset: 60 Item > type: 4 size: 72. > 2021/12/08 00:06:19.690| 80,7| wccp2.cc(1248) wccp2HandleUdp: Offset: 132 > Item type: 8 size: 44. > 2021/12/08 00:06:19.690| 80,5| wccp2.cc(1309) wccp2HandleUdp: Complete packet > received > 2021/12/08 00:06:19.690| 80,3| wccp2.cc(1350) wccp2HandleUdp: Incoming > WCCP2_I_SEE_YOU Received ID old=0 new=112418. > 2021/12/08 00:06:19.690| Unknown capability type in WCCPv2 Packet (0). > 2021/12/08 00:06:19.690| ERROR: Ignoring WCCPv2 message: capability element > truncated > exception location: wccp2.cc(1134) CheckSectionLength > 2021/12/08 00:06:29.688| 80,6| wccp2.cc(1586) wccp2HereIam: wccp2HereIam: > Called > 2021/12/08 00:06:29.688| 80,5| wccp2.cc(1605) wccp2HereIam: wccp2HereIam: > sending to service id 0 > 2021/12/08 00:06:29.688| 80,3| wccp2.cc(1636) wccp2HereIam: Sending HereIam > packet size 144 > 2021/12/08 00:06:29.691| 80,6| wccp2.cc(1207) wccp2HandleUdp: wccp2HandleUdp: > Called. > 2021/12/08 00:06:29.691| 80,3| wccp2.cc(1231) wccp2HandleUdp: Incoming WCCPv2 > I_SEE_YOU length 220. > 2021/12/08 00:06:29.691| 80,7| wccp2.cc(1248) wccp2HandleUdp: Offset: 0 Item > type: 0 size: 8. > 2021/12/08 00:06:29.691| 80,7| wccp2.cc(1248) wccp2HandleUdp: Offset: 8 Item > type: 1 size: 28. > 2021/12/08 00:06:29.691| 80,7| wccp2.cc(1248) wccp2HandleUdp: Offset: 36 Item > type: 2 size: 24. > 2021/12/08 00:06:29.691| 80,7| wccp2.cc(1248) wccp2HandleUdp: Offset: 60 Item > type: 4 size: 116. > 2021/12/08 00:06:29.691| 80,7| wccp2.cc(1248) wccp2HandleUdp: Offset: 176 > Item type: 8 size: 44. > 2021/12/08 00:06:29.691| 80,5| wccp2.cc(1309) wccp2HandleUdp: Complete packet > received > 2021/12/08 00:06:29.691| 80,3| wccp2.cc(1350) wccp2HandleUdp: Incoming > WCCP2_I_SEE_YOU Received ID old=112418 new=112420. > 2021/12/08 00:06:29.691| Unknown capability type in WCCPv2 Packet (0). > 2021/12/08 00:06:29.691| ERROR: Ignoring WCCPv2 message: capability element > truncated > exception location: wccp2.cc(1134) CheckSectionLength > > > Will have to leave it for you, or to continue another time... > Opened bug at https://bugs.squid-cache.org/show_bug.cgi?id=5179 > > Regards > Andrej Mikus > > > > On Mon, 06.Dec.21 15:34:58 -0500, Alex Rousskov wrote: > > > > > I do not have access to other > > > Cisco hardware, and I would like to know if the update was confirmed > > > working for example against a CSR1000v. > > > > I do not think that update was confirmed as working against any WCCP > > server. If you are using WCCP, you are relying on a long-neglected > > feature. There is no proper support for WCCP code in Squid today IMO. > > > > > > Alex. > > P.S. Squid side of CVE 2021-28116 is at > > https://github.com/squid-cache/squid/security/advisories/GHSA-rgf3-9v3p-qp82 > > > > On Tue, 07.Dec.21 14:37:09 +1300, Amos Jeffries wrote: > > > > Hi Andrej, > > > > Alex has summarized the state of things pretty accurately. Since CVE is > > already public please feel free to open a bug report on our Bugzilla. That > > will help with getting the fix backported to official releases. > > > > If you are able to do the testing I am happy to try and fix it for you. > > > > Amos > _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
