This simple patch causes tcp_listen() to inherit the netbuf_idx setting
of the original pcb. Without this, it is not possible to restrict a
socket to a specific interface using SO_BINDTODEVICE before listening.
Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
To fix the build after ppp_output_cb started taking it as const in
commit b2d1fc119d.
Fixes this failure:
../contrib/examples/ppp/pppos_example.c: In function ‘ppp_output_cb’:
../contrib/examples/ppp/pppos_example.c:163:29: error: cast discards ‘const’ qualifier from pointer target type [-Werror=cast-qual]
return sio_write(ppp_sio, (u8_t*)data, len);
^
Authentication timer might still be running when entering network phase
for any necessary rechallenge, mostly for PPP server support.
Update the detailed analysis of simultaneously running PPP timers
taking into account the authentication timer that might still be
running and chose to increase the base number to 2 instead of
adding more unnecessary complexity.
upap_timeout is not currently stopped on authentication success or
fail events. This may have strange results if session is restarted
in a high pace because even if the timeout callback have a sanity
check against the PAP state the session can be restarted and be
back in the valid state before the timeout callback is actually
called.
Do not assume LWIP_RAND will return 32 bits of randomness because it is
probably going to be defined to directly return the rand() value. For
example, LCP magic numbers are 32-bit random values.
This is already what we inherently have always done for IPv4/IPv6
packets, so it works. Receivers must handle both cases anyway because
both behaviors are seen in the wild.
A previous call to ppp_input might have disconnected the session while
there were still packets in flight in the tcpip mailbox. Drop incoming
packets because ppp_input must never be called if the upper layer is
down.
Speed-up a little bit the PPPoS input parser by only checking the open
flag after calling the ppp_input function, the only one that can induce
a state change here.
ppp_set_* functions that set the PPP session parameters must only be
called when the session is in a dead state (i.e. disconnected),
otherwise not fatal but surprising results may happen.
This function call the notify phase callback that should be called from
the lwIP core thread. This is especially true if the user callback
is not designed to be reentrant.
There is no good reason why this function should take a non-const
pointer, as the output callback should never modify what lwIP gives it.
While changing that also switch to a more generic `void*` instead of
"byte".
There is no good reason why this function should take a non-const
pointer. While changing that also switch to a more generic `void*`
instead of "byte".
There is no good reason why this function should take a non-const
pointer. While changing that also switch to a more generic `void*`
instead of "byte".
We do not have equivalents in PPPAPI for ppp_set_* functions because
calling them only makes sense while session is disconnected, furthermore
they are only setting structure members of the session configuration.
We only have to reserve header space for forwarding for IPv4 and IPv6
packets, all other packets are PPP control packets. Doing so reduce
the need of having to coalesce the PBUF chain before PPP processes
control packets.
PPP peer can negotiate its MRU, therefore we don't know the MTU we are
going to use before starting PPP. This is an issue because netif_add
function assume that the netif init callback function will set the MTU,
netif_add will then copy mtu to mtu6. We have then to update mtu6 each
time we update mtu to keep them in sync. Doing so is fine because PPP
netif MTU is only updated when the netif is in link down state.
Our current HDLC decoder does not protect against starving the Rx
PBUF POOL for one packet, most likely due to received garbage on
the serial port.
Prevent starving the Rx pool by checking incoming packets length
against PPP_MRU with a 10% margin because we only want to avoid
filling all PBUFs with garbage, we don't have to be pedantic.
Fixes bug #58441: Invalid PPP data accumulates forever.
PPP_MRU is now free to be used for what it should have been. Now using
it at PPP init stage to set the wanted MRU value, triggering a MRU
negotiation at the LCP phase.
I doubt anyone needs it anyway, but, well, at least it is fixed and the
MRU/MTU config mess is cleaned.
And while we are at it, better document PPP MRU config values.
RFC1661 mandates that default MRU value, that must be used prior
negotiation of MRU value and if MRU value is not negotiated later, must
be 1500.
That is, any PPP host must accept control frames of at least 1500 when
the PPP session start (there are no way to split them in multiples
frames anyway) and must use a value of 1500 if MRU is no negotiated
during LCP exchanges.
Therefore, having it configurable in ppp_opts is a mistake. It was wrong
and never worked because changing the value never triggered a MRU value
negotiation because it changed both the wanted MRU value and the RFC
default value to which the wanted value is compared to trigger a MRU
negotiation if values are not equal.
Those are private functions, using the netif_ prefix here is not really
nice, especially with functions named netif_set_mtu and netif_get_mtu
for obvious reasons.
We currently retry indefinitely if sending packets fails, for example
if the output interface is down. We are even doing it if we are in
a middle of a connection process. This is not a very nice behavior
because PPP low level will retry indefinitely to connect and the user
application will never be warned that something is wrong.
We have the persist boolean in PPP settings to achieve more or less
the same thing anyway. Except it does it better at only retrying
indefinitely the initiation packet.
Having it configurable does not really make sense anymore, we already
need PBUF_RAM in all transmit paths. There are no real reason to keep
allocating PPP response buffers from the PBUF_POOL which should be now
reserved for receive paths only.
We need PBUF_RAM for quite a while for PPP, e.g. through pbuf_coalesce
and for all PPP transmit paths. There are no real reason to keep
allocating packets from PBUF_POOL for PPP control packets transmit path
by default today.
When pbuf_coalesce fails it does nothing and returns the previous buffer
chain. Adds checks that pbuf_coalesce succeeded, otherwise drop incoming
packet.
If we fail to receive a full packet, for exemple if a memory allocation
fail for some reason, we currently do not wait for next packet flag
character and we start filling a new packet at next received byte. Then
we expect the checksum check to discard the packet.
The behavior seem to have been broken one or two decades ago when adding
support for PFC (Protocol-Field-Compression) and ACFC
(Address-and-Control-Field-Compression).
Rework to drop any character until we receive a flag character at init
and when we drop a packet before it is complete.
VJ support is known to be broken when built with some compiler
optimizations enabled, disabling it by default until someone needs it
and fixes it.
It was mostly used with dial-up modems, it is useless with PPPoE and
PPPoL2TP and is probably useless as well with cellular modems, so
disabling it by default makes sense anyway.
In theory, if provided username or password is over 0x80000000 byte long
(err...), casts to signed integer of strlen() return values is going to
return negative values breaking lengths checks.
Fix it by only using unsigned integer or size_t (guaranteed to be
unsigned) comparisons.
Generating docs for file src/incl/home/travis/build/lwip-tcpip/lwip/src/include/lwip/ip4_addr.h:151:s
error: unable to resolve reference to `ip4_addr_eq' for \ref command (warning treated as error, aborting now)
Like pbuf_copy, but can copy part of a pbuf to an offset in another.
pbuf_copy now uses this function internally.
Replace pbuf_take_at loop in icmp6 with pbuf_copy_partial_pbuf().
Fixes bug #58553, and the newly added unit test.
The pbuf_take_at loop should probably be made into a pbuf library
function, which would avoid this mistake in the future and provide
a simpler implementation of pbuf_copy.
User application code should be responsible to call netif_set_up() but
let's not break compatibility for now.
Signed-off-by: Sylvain Rochet <gradator@gradator.net>
NETIF_FLAG_UP flag is not supposed to be set by netif init callback
anymore, call netif_set_up() instead.
Sure it would be better to let user application code call netif_set_up()
by itself as it is now meant to be but let's not break compatibility for
now and add a FIXME for next release with allowed behavior break.
Signed-off-by: Sylvain Rochet <gradator@gradator.net>
Building PPP CCP support without adding any compressor support serve
no real use case. Forbid doing so instead of bloating the code with
more ifdef.
Signed-off-by: Sylvain Rochet <gradator@gradator.net>
This reverts commit 6e7ea92d56.
We better forbid building configurations that does not make sense instead
of bloating the code with more ifdef. Here building CCP support without
adding any compressor support serve no real use case.
Fuzz tests need reproducible code, so we need an "unsafe" version of
LWIP_RAND() in this case...
Also, to reproduce fuzz tests cases from Linux on Windows,
LWIP_RAND_FOR_FUZZ_SIMULATE_GLIBC provides the first 20 random numbers that
glibc would have...
Any malicous segment could contain a SYN up to now (no check).
A SYN in the wrong segment could break OOSEQ queueing.
Fix this by allowing SYN only in states where it is required.
See bug #56397: Assert "tcp_receive: ooseq tcplen > rcv_wnd"
Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
This adds some basic checks to the subroutines of eap_input to check
that we have requested or agreed to doing EAP authentication before
doing any processing on the received packet. The motivation is to
make it harder for a malicious peer to disrupt the operation of pppd
by sending unsolicited EAP packets. Note that eap_success() already
has a check that the EAP client state is reasonable, and does nothing
(apart from possibly printing a debug message) if not.
Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Signed-off-by: Sylvain Rochet <gradator@gradator.net> (ported to lwIP PPP pcb struct)
Given that we have just checked vallen < len, it can never be the case
that vallen >= len + sizeof(rhostname). This fixes the check so we
actually avoid overflowing the rhostname array.
Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Signed-off-by: Sylvain Rochet <gradator@gradator.net> (compiler warning fix about int vs uint comparisons)
When we have multiple netifs where at least one has checksum offloading
capabilities, IP forwarding needs to set various checksum fields to 0
to prevent HW algorithms on calculating an invalid checksum.
-> set checksum fields of IP/UDP/TCP/ICMP to 0 in ip4_forward().
See bug #56288
Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
This is just to keep the code clean and prevent using the "echo" header
where any ICMP header is meant.
Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
See bug #57445. Short version of the description there: lwip_select() failed
to decrement 'select_waiting' of a socket since that code part failed on
'free_pending' sockets. However, the code does not have to check that as it
has marked the socket to be in use itself earlier.
Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
Use 'PBUF_IP_HLEN+PBUF_TRANSPORT_HLEN' instead of '40' to calculate
PBUF_POOL_BUFSIZE (the size of each PBUF_POOL buffer) since the former
can be 60 when IPv6 is enabled.
See bug #56355
Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
This converts all ppp_*() debug functions to ppp_*(()) macros that
ensure the code is left out by the linker if the corresponding debug
setting is disabled.
Downside is that many lines of code are touched, but since these
already differ to upstream PPP sources, I figured that's ok...
See bug #55199
Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
In order to reuse the debug-enable checks for PPP debug macros,
move the flag and level checks from LWIP_DEBUGF to a new macro
that can be used elsewhere.
First calculate and sum TLS overhead when altcp_mbedtls_write() is called.
Then take care of it when calling application sent callback. Give reveived
len from inner_conn, minus calculated overhead.
According to mbedTLS source code and documentation, calls to
`mbedtls_ssl_conf_session_cache` and `mbedtls_ssl_conf_session_tickets_cb`
are only available if mbedTLS is configured for server mode (ie. MBEDTLS_SSL_SRV_C
is defined). This cannot be used on client mode to resume a previous session.
To allow session reuse in client mode, application must save session parameters
(including tickets provided by the server if any) after successfull connection
and restore them before attemting to reconnect. Since `alctp_close()` free the
structure, it cannot be used to store the required information.
So, two new API were added, directly wrapped to mbedTLS functions, allow application
to do that by itself.
Also added full declaration of `struct altcp_tls_session` in altcp_tls.h to allow
easier usage in application when using mbedTLS port.
In some noisy WiFi environment, it may be necessary to increase this value to
300ms to accomodate WiFi latencies which may result in less than the required
250ms between two probe frames received by the Apple BCT application.
Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
- Count tiebreaking loss in num_conflicts to include them in rate limit detection
- Restart probing using mdns_resp_restart allowing rate limiting for those cases
This ensure rate limiting is well activated during Apple Bonjour Conformance Tests.
Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
This allow Apple Bonjour Conformance Test to not fail with the following tests:
- DISTRIBUTED DUPLICATE SUPPRESSION
- MULTIPLE QUESTIONS - DISTRIBUTED DUPLICATE SUPPRESSION
Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
TXT records isn't required to be unique in network, so it shouldn't be
included in probe packets.
Additionnaly, when TXT record is present, the Bonjour Conformance Test
from Apple Inc. always fail because generated probe nevert have TXT record.
Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
Called with `MDNS_INITIAL_PROBE_DELAY_MS` or `MDNS_PROBE_DELAY_MS` according to
needs.
When `mdns_resp_restart_delay()` called by `mdns_resp_rename_(netif|service)()`
functions, it is assumed this is because a conflict. So we should not use
`MDNS_INITIAL_PROBE_DELAY_MS` because the Bonjour Conformance Test will
complain like this:
```
START (PROBING)
NOTICE 16:40:09.501911: conflicting probe:
smarTrEMotE-f8d0a4.Local.
ERROR 16:40:09.607288: Device did not provide a sufficient time gap between receiving a conflicting probe and reprobing.
ERROR 16:40:09.607333: expected_time_gap=237,actual_time_gap=105
```
Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
- Send service slot index to the mdns result function. In case of conflict, the user
will have to remove the service or rename it.
- Break after hostname conflict in order to managed it first, and managed service name
conflict after.
- Provide a function to get the TXT userdata for a service (allowing app to match with
its own data).
Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
When more than one service (just 2) need to be probed for conflict, generation
of the probe packet fail because pbuf is too small!
So OUTPACKET_SIZE renamed to MDNS_OUTPUT_PACKET_SIZE and moved to mdns_opts.h
to allow configuration. Default configuration raise it to 1450 to have enough
space when MDNS_MAX_SERVICES > 1 else it remain 512.
Extract from RFC 6762, chapter 17, Multicast DNS Message Size:
The 1987 DNS specification [RFC1035] restricts DNS messages carried
by UDP to no more than 512 bytes (not counting the IP or UDP
headers). For UDP packets carried over the wide-area Internet in
1987, this was appropriate. For link-local multicast packets on
today's networks, there is no reason to retain this restriction.
Given that the packets are by definition link-local, there are no
Path MTU issues to consider.
Multicast DNS messages carried by UDP may be up to the IP MTU of the
physical interface, less the space required for the IP header (20
bytes for IPv4; 40 bytes for IPv6) and the UDP header (8 bytes).
Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
sys_arch_mbox_tryfetch() shall return SYS_MBOX_EMPTY or 0 according
to the documentation. Wherever the function is used the return
value is incorrectly compared to SYS_ARCH_TIMEOUT. For now
SYS_MBOX_EMPTY is defined to SYS_ARCH_TIMEOUT so this is not an
issue as long as SYS_MBOX_EMPTY isn't re-defined.
Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
As written in RFC5227 in 2.1.1 Probe Details:
A host implementing this specification MUST take precautions to limit
the rate at which it probes for new candidate addresses: if the host
experiences MAX_CONFLICTS or more address conflicts on a given
interface, then the host MUST limit the rate at which it probes for
new addresses on this interface to no more than one attempted new
address per RATE_LIMIT_INTERVAL.
But `acd_restart` restart function check for `acd->num_conflicts > MAX_CONFLICTS`
which allow one more probe than expected.
So this commit change the test to `acd->num_conflicts >= MAX_CONFLICTS`.
Signed-off-by: Simon Goldschmidt <goldsimon@gmx.de>
