[tls] Avoid potential out-of-bound reads in length fields
Many TLS records contain variable-length fields. We currently
validate the overall record length, but do so only after reading the
length of the variable-length field. If the record is too short to
even contain the length field, then we may read uninitialised data
from beyond the end of the record.
This is harmless in practice (since the subsequent overall record
length check would fail regardless of the value read from the
uninitialised length field), but causes warnings from some analysis
tools.
Fix by validating that the overall record length is sufficient to
contain the length field before reading from the length field.
Signed-off-by: Michael Brown <mcb30@ipxe.org>
[infiniband] Make IPoIB support configurable at build time
Add a build configuration option VNIC_IPOIB to control whether or not
IPoIB support is included for Infiniband devices.
Signed-off-by: Michael Brown <mcb30@ipxe.org>
[infiniband] Avoid multiple calls to ib_cmrc_shutdown()
When a CMRC connection is closed, the deferred shutdown process calls
ib_destroy_qp(). This will cause the receive work queue entries to
complete in error (since they are being cancelled), which will in turn
reschedule the deferred shutdown process. This eventually leads to
ib_destroy_conn() being called on a connection that has already been
freed.
Fix by explicitly cancelling any pending shutdown process after the
shutdown process has completed.
Ironically, this almost exactly reverts commit 019d4c1 ("[infiniband]
Use a one-shot process for CMRC shutdown"); prior to the introduction
of one-shot processes the only way to achieve a one-shot process was
for the process to cancel itself.
Signed-off-by: Michael Brown <mcb30@ipxe.org>
TFTP URIs are intrinsically problematic, since:
- TFTP servers may use either normal slashes or backslashes as a
directory separator,
- TFTP servers allow filenames to be specified using relative paths
(with no initial directory separator),
- TFTP filenames present in a DHCP filename field may use special
characters such as "?" or "#" that prevent parsing as a generic URI.
As of commit 7667536 ("[uri] Refactor URI parsing and formatting"), we
have directly constructed TFTP URIs from DHCP next-server and filename
pairs, avoiding the generic URI parser. This eliminated the problems
related to special characters, but indirectly made it impossible to
parse a "tftp://..." URI string into a TFTP URI with a non-absolute
path.
Re-introduce the convention of requiring an extra slash in a
"tftp://..." URI string in order to specify a TFTP URI with an initial
slash in the filename. For example:
tftp://192.168.0.1/boot/pxelinux.0 => RRQ "boot/pxelinux.0"
tftp://192.168.0.1//boot/pxelinux.0 => RRQ "/boot/pxelinux.0"
This is ugly, but there seems to be no other sensible way to provide
the ability to specify all possible TFTP filenames.
A side-effect of this change is that format_uri() will no longer add a
spurious initial "/" when formatting a relative URI string. This
improves the console output when fetching an image specified via a
relative URI.
Signed-off-by: Michael Brown <mcb30@ipxe.org>
Expose the network interface name (e.g. "net0") as a setting. This
allows a script to obtain the name of the most recently opened network
interface via ${netX/ifname}.
Signed-off-by: Andrew Widdersheim <amwiddersheim@gmail.com>
Modified-by: Michael Brown <mcb30@ipxe.org>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
The three nominally-disambiguated ENOTSUP errors accidentally all used
the same error disambiguator, rendering them identical. Fix by
changing all three values. We avoid reusing the 0x01 disambiguator
value, since that remains ambiguous in older binaries.
Signed-off-by: Michael Brown <mcb30@ipxe.org>
[tftp] Do not change current working URI when TFTP server is cleared
For historical reasons, iPXE sets the current working URI to the root
of the TFTP server whenever the TFTP server address is changed. This
was originally implemented in the hope of allowing a DHCP-provided
TFTP filename to be treated simply as a relative URI. This usage
turns out to be impractical since DHCP-provided TFTP filenames may
include characters which would have special significance to the URI
parser, and so the DHCP next-server+filename combination is now
handled by the dedicated pxe_uri() function instead.
The practice of setting the current working URI to the root of the
TFTP server is potentially helpful for interactive uses of iPXE,
allowing a user to type e.g.
iPXE> dhcp
Configuring (net0 52:54:00:12:34:56)... ok
iPXE> chain pxelinux.0
and have the URI "pxelinux.0" interpreted as being relative to the
root of the TFTP server provided via DHCP.
The current implementation of tftp_apply_settings() has an unintended
flaw. When the "dhcp" command is used to renew a DHCP lease (or to
pick up potentially modified DHCP options), the old settings block
will be unregistered before the new settings block is registered.
This causes tftp_apply_settings() to believe that the TFTP server has
been changed twice (to 0.0.0.0 and back again), and so the current
working URI will always be set to the root of the TFTP server, even if
the DHCP response provides exactly the same TFTP server as previously.
Fix by doing nothing in tftp_apply_settings() whenever there is no
TFTP server address.
Debugged-by: Andrew Widdersheim <awiddersheim@inetu.net>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
Resolve redirection URIs as being relative to the original HTTP
request URI, rather than treating them as being implicitly relative to
the current working URI.
Signed-off-by: Michael Brown <mcb30@ipxe.org>
[dhcp] Limit maximum number of DHCP discovery deferrals
For switches which remain permanently in the non-forwarding state (or
which erroneously report a non-forwarding state), ensure that iPXE
will eventually give up waiting for the link to become unblocked.
Originally-fixed-by: Wissam Shoukair <wissams@mellanox.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
If we detect (via STP) that a switch port is in a non-forwarding
state, then the link is marked as being temporarily blocked and DHCP
discovery will be deferred until the link becomes unblocked.
The timer used to decide when to give up waiting for ProxyDHCPOFFERs
is currently based on the time that DHCP discovery was started, and
makes no allowances for any time spent waiting for the link to become
unblocked. Consequently, if STP is used then the timeout for
ProxyDHCPOFFERs becomes essentially zero.
Fix by resetting the recorded start time whenever DHCP discovery is
deferred due to a blocked link.
Debugged-by: Sebastian Roth <sebastian.roth@zoho.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
[peerdist] Avoid NULL pointer dereference for plaintext blocks
Avoid accidentally dereferencing a NULL cipher context pointer for
plaintext blocks (which are usually messages with a block length of
zero, indicating a missing block).
Signed-off-by: Michael Brown <mcb30@ipxe.org>
[tcpip] Avoid generating positive zero for transmitted UDP checksums
TCP/IP checksum fields are one's complement values and therefore have
two possible representations of zero: positive zero (0x0000) and
negative zero (0xffff).
In RFC768, UDP over IPv4 exploits this redundancy to repurpose the
positive representation of zero (0x0000) to mean "no checksum
calculated"; checksums are optional for UDP over IPv4.
In RFC2460, checksums are made mandatory for UDP over IPv4. The
wording of the RFC is such that the UDP header is mandated to use only
the negative representation of zero (0xffff), rather than simply
requiring the checksum to be correct but allowing for either
representation of zero to be used.
In RFC1071, an example algorithm is given for calculating the TCP/IP
checksum. This algorithm happens to produce only the positive
representation of zero (0x0000); this is an artifact of the way that
unsigned arithmetic is used to calculate a signed one's complement
sum (and its final negation).
A common misconception has developed (exemplified in RFC1624) that
this artifact is part of the specification. Many people have assumed
that the checksum field should never contain the negative
representation of zero (0xffff).
A sensible receiver will calculate the checksum over the whole packet
and verify that the result is zero (in whichever representation of
zero happens to be generated by the receiver's algorithm). Such a
receiver will not care which representation of zero happens to be used
in the checksum field.
However, there are receivers in existence which will verify the
received checksum the hard way: by calculating the checksum over the
remainder of the packet and comparing the result against the checksum
field. If the representation of zero used by the receiver's algorithm
does not match the representation of zero used by the transmitter (and
so placed in the checksum field), and if the receiver does not
explicitly allow for both representations to compare as equal, then
the receiver may reject packets with a valid checksum.
For UDP, the combined RFCs effectively mandate that we should generate
only the negative representation of zero in the checksum field.
For IP, TCP and ICMP, the RFCs do not mandate which representation of
zero should be used, but the misconceptions which have grown up around
RFC1071 and RFC1624 suggest that it would be least surprising to
generate only the positive representation of zero in the checksum
field.
Fix by ensuring that all of our checksum algorithms generate only the
positive representation of zero, and explicitly inverting this in the
case of transmitted UDP packets.
Reported-by: Wissam Shoukair <wissams@mellanox.com>
Tested-by: Wissam Shoukair <wissams@mellanox.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
[pxe] Populate ciaddr in fake PXE Boot Server ACK packet
We currently do not populate the ciaddr field in the constructed PXE
Boot Server ACK packet. This causes a WDS server to respond with a
broadcast packet, which is then ignored by wdsmgfw.efi since it does
not match the specified IP address filter.
Fix by populating ciaddr within the constructed PXE Boot Server ACK
packet.
Signed-off-by: Michael Brown <mcb30@ipxe.org>
[dhcp] Do not skip ProxyDHCPREQUEST if next-server is empty
We attempt to mimic the behaviour of Intel's PXE ROM by skipping the
separate ProxyDHCPREQUEST if the ProxyDHCPOFFER already contains a
boot filename or a PXE boot menu.
Experimentation reveals that Intel's PXE ROM will also check for a
non-empty next-server address alongside the boot filename. Update our
test to match this behaviour.
Reported-by: Wissam Shoukair <wissams@mellanox.com>
Tested-by: Wissam Shoukair <wissams@mellanox.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
Commit 09b057c ("[settings] Remove "uristring" setting type") removed
support for URI-encoded settings via the "uristring" setting type, on
the basis that such encoding was no longer necessary to avoid problems
with the command line parser.
Other valid use cases for the "uristring" setting type do exist: for
example, a password containing a '/' character expanded via
chain http://username:${password:uristring}@server.name/boot.php
Restore the existence of the "uristring" setting, avoiding the
potentially large stack allocations that were used in the old code
prior to commit 09b057c ("[settings] Remove "uristring" setting
type").
Requested-by: Robin Smidsrød <robin@smidsrod.no>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
[dhcp] Allow pseudo-DHCP servers to use pseudo-identifiers
Some ProxyDHCP servers and PXE boot servers do not specify a DHCP
server identifier via option 54. We currently work around this in a
variety of ad-hoc ways:
- if a ProxyDHCPACK has no server identifier then we treat it as
having the correct server identifier,
- if a boot server ACK has no server identifier then we use the
packet's source IP address as the server identifier.
Introduce the concept of a DHCP server pseudo-identifier, defined as
being:
- the server identifier (option 54), or
- if there is no server identifier, then the next-server address
(siaddr),
- if there is no server identifier or next-server address, then the
DHCP packet's source IP address.
Use the pseudo-identifier in place of the server identifier when
handling ProxyDHCP and PXE boot server responses.
Originally-fixed-by: Wissam Shoukair <wissams@mellanox.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
[ipoib] Fix a race when chain-loading undionly.kpxe in IPoIB
The Infiniband link status change callback ipoib_link_state_changed()
may be called while the IPoIB device is closed, in which case there
will not be an IPoIB queue pair to be joined to the IPv4 broadcast
group. This leads to NULL pointer dereferences in ib_mcast_attach()
and ib_mcast_detach().
Fix by not attempting to join (or leave) the broadcast group unless we
actually have an IPoIB queue pair.
Signed-off-by: Wissam Shoukair <wissams@mellanox.com>
Modified-by: Michael Brown <mcb30@ipxe.org>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
[http] Rewrite HTTP core to support content encodings
Rewrite the HTTP core to allow for the addition of arbitrary content
encoding mechanisms, such as PeerDist and gzip.
The core now exposes http_open() which can be used to create requests
with an explicitly selected HTTP method, an optional requested content
range, and an optional request body. A simple wrapper provides the
preexisting behaviour of creating either a GET request or an
application/x-www-form-urlencoded POST request (if the URI includes
parameters).
The HTTP SAN interface is now implemented using the generic block
device translator. Individual blocks are requested using http_open()
to create a range request.
Server connections are now managed via a connection pool; this allows
for multiple requests to the same server (e.g. for SAN blocks) to be
completely unaware of each other. Repeated HTTPS connections to the
same server can reuse a pooled connection, avoiding the per-connection
overhead of establishing a TLS session (which can take several seconds
if using a client certificate).
Support for HTTP SAN booting and for the Basic and Digest
authentication schemes is now optional and can be controlled via the
SANBOOT_PROTO_HTTP, HTTP_AUTH_BASIC, and HTTP_AUTH_DIGEST build
configuration options in config/general.h.
Signed-off-by: Michael Brown <mcb30@ipxe.org>
[crypto] Support SHA-{224,384,512} in X.509 certificates
Add support for SHA-224, SHA-384, and SHA-512 as digest algorithms in
X.509 certificates, and allow the choice of public-key, cipher, and
digest algorithms to be configured at build time via config/crypto.h.
Originally-implemented-by: Tufan Karadere <tufank@gmail.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
[tls] Do not access beyond the end of a 24-bit integer
The current implementation handles big-endian 24-bit integers (which
occur in several TLS record types) by treating them as big-endian
32-bit integers which are shifted by 8 bits. This can result in
"Invalid read" errors when running under valgrind, if the 24-bit field
happens to be exactly at the end of an I/O buffer.
Fix by ensuring that we touch only the three bytes which comprise the
24-bit integer.
Signed-off-by: Michael Brown <mcb30@ipxe.org>
[netdevice] Allow network devices to disclaim IRQ support at runtime
VLAN and 802.11 devices use a network device operations structure that
wraps an underlying structure. For example, the vlan_operations
structure wraps the network device operations structure of the
underlying trunk device. This can cause false positives from the
current implementation of netdev_irq_supported(), which will always
report that VLAN devices support interrupts since it has no visibility
into the support provided by the underlying trunk device.
Fix by allowing network devices to explicitly flag that interrupts are
not supported, despite the presence of an irq() method.
Originally-fixed-by: Wissam Shoukair <wissams@mellanox.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
iscsi_tx_done() is missing "break" statements at the end of each case.
(Fortunately, this happens not to cause a bug in practice, since
iscsi_login_request_done() is effectively a no-op when completing a
data-out PDU.)
Reported-by: Wissam Shoukair <wissams@mellanox.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
Michael Brown