Saving the configuration works in Chrome and fails in Firefox, and the
difference is only how each browser splits the request. The handler
erased the config sector as soon as the request started and then parsed
the multipart body one TCP segment at a time, which requires every
boundary and every part header to fall inside a single segment. Firefox
splits inside a part header, so the parser lost its place and the
erased sector was left holding a truncated body or nothing at all. A
single-burst scripted post lost the whole body the same way.
The configuration is limited to two kilobytes, so the whole request body
now accumulates in xdata and is parsed only after the closing boundary
has arrived. The parts are walked in one pass, the part carrying a
filename is written to a freshly erased sector, and the client receives
a 200 instead of the previous silent close. No segmentation can confuse
this, since the parser only ever sees a complete body.
Locating the closing boundary first also bounds the walk over the parts,
since none can lie beyond it, so the length of the buffer is no longer
the bound and the test for the two trailing dashes is unnecessary.
The walk matches at offsets inside a buffer that is not terminated, so
neither existing helper fits: strcmp() goes on to compare the byte after
the match and is_word_x() demands a separator there. Add strstart() and
strstart_x() for that case, and use strlen_x() for the boundary length.
The firmware upload path still streams, since a megabyte cannot be
buffered, and is untouched.
The two flags added with the JSON fix sit in data, where internal RAM is
full enough that this branch stopped linking for some toolchains. __bit
puts them in the bit area instead and hands three bytes back to the
stack: SSEG goes from 131 to 134 on SWTGW218AS.
Patch by vDorst on the pull request.
The MAC table listing wrote its separator once per iteration rather than
once per object. An entry the table engine reports as invalid produces no
object, so it contributed a bare comma, and two in a row give ",," which
JSON.parse rejects. The whole table then fails to load, not just the row
that was missing. The separator now goes before each object and the
closing bracket after the loop, which is the shape send_vlanlist already
uses further down the file.
The next index for an invalid entry was computed as h | low + 1, and the
addition binds tighter than the or. That agrees with (h | low) + 1 except
when the low byte reads 0xff and bit 8 of the index is already set, eight
of the 4096 combinations. There the result is the start of the current
block of 256 rather than the start of the next one, so the walk repeats a
block it has already covered. Reading the index once after the branch
rather than once in each arm removes the second copy of that expression
along with the bug.
The VLAN now comes first in each object. It is taken from the same
L2_DATA_OUT_B read that decides whether the entry is valid, which saves
reading that register a second time. The page addresses the fields by
name, so the order they arrive in does not matter to it.
A bound check on the output buffer goes in for consistency with
send_vlanlist. Thirty entries of at most 74 bytes plus the brackets fit in
the 2500 byte buffer with 179 to spare, so nothing changes today, but the
margin was nowhere stated and L2_MAX_TRANSFER is a tunable.
5 bytes of BANK1, nothing in BANK2, xdata or internal RAM. Built for SWTGW218AS
and KP_9000_6XHML_X2 on sdcc 4.5.0.
The member mask of an aggregation group is decoded by hand in two places,
the lag command and the JSON behind the aggregation page, and every branch
that touches trunks adds another copy.
port_lag_members_get() sits next to port_lag_members_set() and both readers
call it. It answers from the hardware, so it covers a group configured with
lag and one a protocol brought up, without either having to say so.
It reads through reg_read() rather than reg_read_m(), so sfr_data is left
alone. Neither caller looked at it afterwards; both read the hash register
next.
The handler took one raw character of the request line and passed it to
send_counters(), which uses it to index machine.phys_to_log_port. That
array has nine entries and the character is whatever the client sent, so
the read ran up to 246 entries past the end and the result went on to
STAT_GET as a port number. is_word() accepts any request whose name is
followed by a question mark, so nothing constrained the byte to a digit.
Bounding it where it is read keeps the check beside the assumption it
protects and needs nothing from the machine description. sdcc leaves
plain char unsigned and the subtraction wraps in eight bits, so a byte
below '0' comes out above 200 and one upper test covers both ends:
exactly '0' to '8' now reach send_counters. The compiled test is
add a,#0xf7 followed by jnc, which I read back out of the assembly rather
than assuming.
Out of range answers 400 by the path the other malformed requests already
take, rather than an empty array. An empty array would have been worse
than useless here, since the statistics page calls BigInt on the first
element before it looks at the length. The page asks only for index zero
to the port count minus one, so nothing that answered before stops
answering, and a non-200 reply makes its handler do nothing at all.
11 bytes of BANK1, nothing in the common segment, BANK2, xdata or
internal RAM. Built for SWTGW218AS and KP_9000_6XHML_X2 on sdcc 4.5.0.
The POST path tests for a NUL before it looks at a byte. The GET path did not,
and is_separator() counts only space, tab, question mark and equals, so a request
line carrying none of those walks past the end of uip_buf and writes its
terminator into whatever xdata it happens to stop on.
Everything that is not a POST reaches that walk. The pointer advances past the
method before anything checks that the method was GET, so a TLS record sent to
port 80 by a browser trying https first is enough on its own, as is a port
scanner or a malformed line. The stop is wherever the first space, tab, question
mark or equals turns up in memory, which is why the symptoms are erratic.
Two bytes of BANK1. BANK2 and xdata do not move.
Review feedback on #306. The value used to ride in /information.json and
the picker fetched /vlanlist separately, so the page needed both requests
to mean anything. /vlanlist now answers {"mgmt":N,"vlan":[...]} and the
picker reads both from the one response. Both consumers in vlan.js were
taught the new shape, and /information.json no longer carries mgmt_vlan.
The truncation guard now reserves 141 bytes instead of 139: the closing
grew to two bytes with the wrapping object, and the comma in front of a
non-first entry was never counted, so the worst case could land one byte
past outbuf even before this change. char_to_html() does not check.
The comments added on this branch are gone as well, style.css and
system.js both, since these files are served byte for byte.
page_impl.rel stays at DSEG 5, OSEG 0, BSEG 3 and the image reports the
same 10183 bytes of XDATA before and after.
`vlan <id> mgmt` existed on the CLI, but nothing reported which VLAN
currently carries management, so the setting was invisible from the web UI -
the only way to find out was to read the startup config.
Report it in information.json and offer it where the other switch-wide
addressing settings live, between Gateway and Language, as a picker filled
from the configured VLANs. When management is untagged there is no VLAN to
select, so that state shows as a disabled entry rather than inventing an id
the switch would reject.
Confirm before applying, and say what will happen rather than echoing the
action back: the switch starts tagging its own frames, and if the port you
came in on does not carry that VLAN the page becomes unreachable and the way
back is the console. Cancelling puts the picker back where it was.
Add a device hostname settable from the CLI (`hostname <text>`) and the
System Settings page. The value is sanitized on ingest to JSON-safe
printable ASCII (<=23 chars), stored in a shared __xdata buffer, seeded
to "RTLPlayground" at boot, persisted through the startup-config, and
reported in /information.json. It lives in the common header so other
modules can advertise it (LLDP uses it as the System Name TLV).
Root cause of the "browser login always bounces back with Wrong password!
while curl works": scan_header() read the session id from a fixed offset
into the Cookie header (p + 17), assuming "session=" is the first and only
cookie. Browsers keep stale cookies for a long time - e.g. an "admin" cookie
left over from this switch's VENDOR firmware - so the header can arrive as
"Cookie: admin=..; session=..", the fixed offset then points into the admin
value, authentication silently fails and every page bounces to login although
the password had been accepted. curl sends only "session=", which is why
command-line tests passed while a real browser (with that stale cookie) failed.
- scan_header(): scan the Cookie header for the actual "session=" key
(matched as "session" - is_word() requires a separator after the pattern
and '=' is on its list, the first value byte is not).
- is_word_x(): accept ';' as a terminating separator so the session value
also matches when it is not the last cookie in the header.
Verified on hardware end-to-end in a real browser WITH the stale "admin"
cookie present: login -> index.html, all pages and JSON endpoints work.
Real root cause of the "web login fails from a browser but works from curl":
the uIP httpd is built with UIP_CONF_MAX_CONNECTIONS = 1 and uses global
response state (outbuf/slen/session), i.e. it serves exactly one TCP connection
at a time and closes it after each response - but never advertises that via
the Connection header. A browser's HTTP/1.1 client therefore assumes the
connection may be persistent and can park it in its keep-alive pool for reuse;
a later request sent on that pooled connection hits one the server has already
closed, and a POST (unlike a GET) is never retried by the browser, so it can
be silently lost this way.
This adds "Connection: close" to every response so the browser does not pool
and reuse a connection the server is about to drop. On its own this did not
fully explain the reported login failures - the actual authentication bug is
fixed in the next commit (the Cookie header parsed at a fixed offset) - but it
is correct behaviour for a server that only ever handles one connection, and
removes one source of dropped requests.
Two hygiene fixes for the web UI, prompted by a login that appeared to fail
under privacy shields (Brave Shields / NoScript-family extensions):
- httpd: replace the partial "style-src 'self' 'unsafe-inline'" CSP with a
complete, first-party policy (default-src 'self'; script-src 'self'
'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' data:;
connect-src 'self'; form-action 'self'). Everything the UI needs is
same-origin; the explicit policy stops shields injecting their own
restrictive report-only probes (the noisy script-src-elem 'none' console
spam) and passes a strict-CSP audit. Verified: no CSP violations in-browser.
- login.html: add autocomplete="current-password" so password managers
recognise the field (they showed "unknown password" without it).
NOTE: these do NOT bypass a browser's LAN-device protection (NoScript "lan" /
Brave Shields), which strips the POST body of requests to a LAN address and is
why the login can fail in-browser while the same credentials work over curl.
That is a deliberate browser security feature; the user must allow the site in
their shields to log in. The backend password (default 1234) is unchanged and
correct.
On a QSFPTEK QT-SFP+-T (RTL8261C) 10GBase-T module, the diag type field
(92) comes back as 0x00, but there's actually some statistics available
like temperature. Other metrics may be hard-coded values.
Add a basic struct that allows matching on vendor and/or model, using a
bitfield to allow multiple quirks for a given SFP module. Only
SFP_QUIRK_DDM is implemented.
For modules matching SFP_QUIRK_DDM, attempt an I2C read of the MSB of
module voltage during probe if DDM is "unsupported" - if it's not 0xff,
override the reported options so we can pull the diagnostic data.
To allow simpler comparisons, convert the ASCII fields (vendor,
model, serial) from space-padded to standard NULL-terminated strings.
strcmp() is moved from httpd.c to rtlplayground.c alongside other string
functions and shared between them.
The __reentrant keyword is used for the new functions to avoid using up
additional OSEG space. This allocates the variables on the stack, which
is OK for this particular code path.
The JSON assembly in send_status() is slightly modified to treat the
sfp_module_* data as standard NULL-terminated strings, and a repeated
subtraction was moved into a uint8_t to declutter the code.
- itoa16_html: add comment that the function is sufficient for VLAN
IDs (<=4094); generalization not needed.
- send_vlanlist: replace post-write bounds check with a pre-write
guard using worst-case entry size (138 bytes + 1 for closing
bracket). Old comment claimed ~45 bytes per entry, which only
held for short names. With a 117-char name (the actual bound from
CMD_BUF_SIZE) entries can reach 138 bytes, and the post-check
would not have prevented an overflow.
- Document the 0x02 check in sfr_data[0] as the VLAN table entry
valid flag.
Returns a JSON array of all configured VLANs with their IDs and names,
e.g. [{"id":1,"name":""},{"id":20,"name":"IoT"}].
The endpoint iterates VLAN IDs 1..4094 and filters by the validity bit
in sfr_data[0] (0x02), following the same pattern as vlan_create() and
vlan_setup() in rtl837x_port.c.
Also adds a small itoa16_html() helper for emitting decimal numbers
up to 4 digits (analogous to the existing 8-bit itoa_html()), used
for VLAN IDs which can reach 4094. Response builder uses the existing
vlan_name() helper for the name lookup, consistent with send_vlan().
Buffer overflow is prevented by breaking out of the iteration loop at
TCP_OUTBUF_SIZE - 60.
This endpoint is the foundation for upcoming UI improvements
(VLAN selector dropdown and overview table).
With content_type = "application/x-www-form-urlencoded", "+" means space.
This case was not handled.
Also refactor the code to make a loop to process the hex digits.
Chrome sends upload requests using POST with multipart/form-data
content type in multiple packets for the header part of the form-data.
Introduce a TSTATE_MULTIPART for the httpd server states that denotes
that so far only a part of the multipart header has been transmitted.
Once the full header has been transmitted, we change to TSTATE_POST
as for Firefox which sends all the multipart header in one piece.
The main further change required then is to make sure that the parsing
of the initial part of the multipart request is only parsed once and initially
to distinguish between configuration and firmware uploads.
While the current implementation works for what it is actually used, it
is broken when trying to do larger transfers.
The length field in the control register has a size of 4 bits. In every
transfer, length+1 bytes are read. Thus, each transfer is limited to a
maximum of 16 bytes. Add a check for the length, and write the correct
value to the register.
Also update the loop in "sfp_send_data" to properly increment the output
register. Remove the unused special case for a length of 128 bytes.
bloqaudio
d00f
René van Dorst
d00f
Priit Laes
Matt Merhar
TylerDurden-23
feelfree69
Erdnusschokolade
logicog
logicog
UAb5eSMn
orbisai0security
feelfree69
Jan Hoffmann