Fixes CodeQL alerts for CWE-457:
https://cwe.mitre.org/data/definitions/457.html
Cc: Erich McMillan <emcmillan@microsoft.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3845
Enables KVM and One Click Recovery WLAN capability with WiFi Profile
Sync feature and protocol. Adding WiFiProfileSyncProtocol, which
supports the profilesync driver operations for transferring WiFi profiles
from AMT to the Supplicant. WiFiConnectionManager will check for the
WifiProfileSyncProtocol and if found will operate on the premise of a
One Click Recovery, or KVM flow with a Wifi profile provided by AMT.
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Cc: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
Signed-off-by: Zachary Clark-Williams <zachary.clark-williams@intel.com>
Acked-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4053
Add LoongArch64 architecture in to NetworkPkg.
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Chao Li <lichao@loongson.cn>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Removes Status argument that is not needed from DEBUG macros.
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2504
Add support for TLS Client Authentication using Basic Authentication
for HTTP Boot
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
RVCT is obsolete and no longer used.
Remove support for it.
Signed-off-by: Rebecca Cran <quic_rcran@quicinc.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2720
Force connection close before the next request if
the server identifies as version 1.0.
Signed-off-by: Oliver Steffen <osteffen@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2720
Force connection close before the next request if
the server sends the 'Connection: close' header.
Signed-off-by: Oliver Steffen <osteffen@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2720
Add ConnectionClose flag to HTTP_PROTOCOL.
This boolean is FALSE by default. If set to TRUE, a reconfigure
of the Http instance is forced on the next request. The flag
is then reset.
Signed-off-by: Oliver Steffen <osteffen@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2720
Check if the state of the HTTP instance is HTTP_STATE_TCP_CONNECTED, or
HTTP_STATE_TCP_CLOSED and de-configure the Tcp6 instance before
configuring it again.
Signed-off-by: Oliver Steffen <osteffen@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2720
Check if the state of the HTTP instance is HTTP_STATE_TCP_CONNECTED, or
HTTP_STATE_TCP_CLOSED and de-configure the Tcp4 instance before
configuring it again.
Signed-off-by: Oliver Steffen <osteffen@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3889
Currently the HTTP DNS Retry Interval is 2 second and no retry attempt.
If DNS response can't arrive in 2 second, the DNS request is failed,
and then HTTP boot is failed.
In order to resolve this issue, Add PCDs to support RetryCount
and RetryInterval configured in Platform.
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Signed-off-by: Heng Luo <heng.luo@intel.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3813
The size of buffer should be 3 CHAR16 for Null-terminated Unicode
string.
The first char is the AKM/Cipher Suite number, the second char is ' ',
the third char is '\0'.
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Signed-off-by: Heng Luo <heng.luo@intel.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
This change is required by f4b7b473b4.
Signed-off-by: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3737
Apply uncrustify changes to .c/.h files in the NetworkPkg package
Cc: Andrew Fish <afish@apple.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3767
Update use of DEBUG_CODE(Expression) if Expression is a complex code
block with if/while/for/case statements that use {}.
Cc: Andrew Fish <afish@apple.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3760
Update all use of ', OPTIONAL' to ' OPTIONAL,' for function params.
Cc: Andrew Fish <afish@apple.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3739
Update all use of EFI_D_* defines in DEBUG() macros to DEBUG_* defines.
Cc: Andrew Fish <afish@apple.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3719
This issue is introduced by the commit 43d7e607.
Token->RspData.H2AData is de-allocated on error but it is not
set to NULL. HTTP module attempts to free again and cause assert.
Signed-off-by: Baraneedharan Anbazhagan <anbazhagan@hp.com>
Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3688
Use DEBUG_LINE_NUMBER instead of __LINE__.
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Tested-by: Michael Kubacki <michael.kubacki@microsoft.com>
Fix NetworkPkg build breaks for ARM and RISCV64 by adding RngLib
mapping.
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Abner Chang <abner.chang@hpe.com>
Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Daniel Schaefer <daniel.schaefer@hpe.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Acked-by: Abner Chang <abner.chang@hpe.com>
The current UEFI implementation of HTTPS during its TLS configuration
uses
EFI_TLS_VERIFY_FLAG_NO_WILDCARDS for host name verification. As per the
spec
this flag does is "to disable the match of any wildcards in the host
name". So,
certificates which are issued with wildcards(*.dm.corp.net etc) in it
will fail
the TLS host name matching. On the other hand,
EFI_TLS_VERIFY_FLAG_NONE(misnomer) means "no additional flags set for
hostname
validation. Wildcards are supported and they match only in the left-most
label."
this behavior/definition is coming from openssl's X509_check_host() api
https://www.openssl.org/docs/man1.1.0/man3/X509_check_host.html
Without EFI_TLS_VERIFY_FLAG_NONE any UEFI application using certificates
issued
with wildcards in them would fail to match while trying to communicate
with
HTTPS endpoint.
BugZilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3691
Signed-off-by: Vineel Kovvuri <vineelko@microsoft.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
https://bugzilla.tianocore.org/show_bug.cgi?id=3694
HttpBootCheckImageType() was using the case-sensitive AsciiStrCmp() to
check the file extensions and this could reject the images with
upper-case file names. Using the case-insensitive AsciiStriCmp() to
avoid the issue.
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Gary Lin <gary.lin@hpe.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3496
Add new EDKII_HTTP_CALLBACK_PROTOCOL in NetworkPkg,
Send HTTP Events via EDKII_HTTP_CALLBACK_PROTOCOL
when Dns/ConnectTcp/TlsConnectSession/InitSession
occurs.
Signed-off-by: Heng Luo <heng.luo@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3507
HTTP boot has a default set forced timeout value of 5 seconds
for getting the recovery image from a remote source.
This change allows the HTTP boot flow to get the IO timeout value
from the PcdHttpIoTimeout.
PcdHttpIoTimeout value is set in platform code.
Signed-off-by: Zachary Clark-Williams <zachary.clark-williams@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Introduce the NETWORK_ISCSI_MD5_ENABLE feature test macro for NetworkPkg.
When explicitly set to FALSE, remove MD5 from IScsiDxe's CHAP algorithm
list.
Set NETWORK_ISCSI_MD5_ENABLE to TRUE by default, for compatibility
reasons. Not just to minimize the disruption for platforms that currently
include IScsiDxe, but also because RFC 7143 mandates MD5 for CHAP, and
some vendors' iSCSI targets support MD5 only.
With MD5 enabled, IScsiDxe will suggest SHA256, and then fall back to MD5
if the target requests it. With MD5 disabled, IScsiDxe will suggest
SHA256, and break off the connection (and session) if the target doesn't
support SHA256.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3355
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210629163337.14120-7-lersek@redhat.com>
Insert a SHA256 CHAP_HASH structure at the start of "mChapHash".
Update ISCSI_CHAP_MAX_DIGEST_SIZE to SHA256_DIGEST_SIZE (32).
This enables the initiator and the target to negotiate SHA256 for CHAP, in
preference to MD5.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3355
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Message-Id: <20210629163337.14120-6-lersek@redhat.com>
Introduce the "mChapHash" table, containing the hash algorithms supported
for CHAP. Hash algos listed at the beginning of the table are preferred by
the initiator.
In ISCSI_CHAP_STEP_ONE, send such a CHAP_A value that is the
comma-separated, ordered list of algorithm identifiers from "mChapHash".
Pre-format this value string at driver startup, in the new function
IScsiCHAPInitHashList().
(In IScsiCHAPInitHashList(), also enforce that every hash algo's digest
size fit into ISCSI_CHAP_MAX_DIGEST_SIZE, as the latter controls the
digest, outgoing challenge, and hex *allocations*.)
In ISCSI_CHAP_STEP_TWO, allow the target to select one of the offered hash
algorithms, and remember the selection for the later steps. For
ISCSI_CHAP_STEP_THREE, hash the challenge from the target with the
selected hash algo.
In ISCSI_CHAP_STEP_THREE, send the correctly sized digest to the target.
If the initiator wants mutual authentication, then generate a challenge
with as many bytes as the target's digest will have, in
ISCSI_CHAP_STEP_FOUR.
In ISCSI_CHAP_STEP_FOUR (i.e., when mutual authentication is required by
the initiator), verify the target's response (digest) with the selected
algorithm.
Clear the selected hash algorithm before every login (remember that in
IScsiDxe, every login is a leading login).
There is no peer-observable change from this patch, as it only reworks the
current MD5 support into the new internal representation.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3355
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210629163337.14120-5-lersek@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
IScsiDxe uses the ISCSI_CHAP_RSP_LEN macro for expressing the size of the
digest (16) that it solely supports at this point (MD5).
ISCSI_CHAP_RSP_LEN is used for both (a) *allocating* digest-related
buffers (binary buffers and hex encodings alike), and (b) *processing*
binary digest buffers (comparing them, filling them, reading them).
In preparation for adding other hash algorithms, split purpose (a) from
purpose (b). For purpose (a) -- buffer allocation --, introduce
ISCSI_CHAP_MAX_DIGEST_SIZE. For purpose (b) -- processing --, rely on
MD5_DIGEST_SIZE from <BaseCryptLib.h>.
Distinguishing these purposes is justified because purpose (b) --
processing -- must depend on the hashing algorithm negotiated between
initiator and target, while for purpose (a) -- allocation --, using the
maximum supported digest size is suitable. For now, because only MD5 is
supported, introduce ISCSI_CHAP_MAX_DIGEST_SIZE *as* MD5_DIGEST_SIZE.
Note that the argument for using the digest size as the size of the
outgoing challenge (in case mutual authentication is desired by the
initiator) remains in place. Because of this, the above two purposes are
distinguished for the "ISCSI_CHAP_AUTH_DATA.OutChallenge" field as well.
This patch is functionally a no-op, just yet.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3355
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Message-Id: <20210629163337.14120-4-lersek@redhat.com>
In the next patches, we'll need more room for various macro and parameter
names. For maintaining the current visual alignments, insert some
horizontal whitespace in preparation. "git show -b" produces no output for
this patch; the patch introduces no functional changes.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3355
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Message-Id: <20210629163337.14120-3-lersek@redhat.com>
RFC 7143 explains that a single iSCSI session may use multiple TCP
connections. The first connection established is called the leading
connection. The login performed on the leading connection is called the
leading login. Before the session is considered full-featured, the leading
login must succeed. Further (non-leading) connections can be associated
with the session later.
(It's unclear to me from RFC 7143 whether the non-leading connections
require individual (non-leading) logins as well, but that particular
question is irrelevant from the perspective of this patch; see below.)
The data model in IScsiDxe exhibits some confusion, regarding connection /
session association:
- On one hand, the "ISCSI_SESSION.Conns" field is a *set* (it has type
LIST_ENTRY), and accordingly, connections can be added to, and removed
from, a session, with the IScsiAttatchConnection() and
IScsiDetatchConnection() functions.
- On the other hand, ISCSI_MAX_CONNS_PER_SESSION has value 1, therefore no
session will ever use more than 1 connection at a time (refer to
instances of "Session->MaxConnections" in
"NetworkPkg/IScsiDxe/IScsiProto.c").
This one-to-many confusion between ISCSI_SESSION and ISCSI_CONNECTION is
very visible in the CHAP logic, where the progress of the authentication
is maintained *per connection*, in the "ISCSI_CONNECTION.AuthStep" field
(with values such as ISCSI_AUTH_INITIAL, ISCSI_CHAP_STEP_ONE, etc), but
the *data* for the authentication are maintained *per session*, in the
"AuthType" and "AuthData" fields of ISCSI_SESSION. Clearly, this makes no
sense if multiple connections are eligible for logging in.
Knowing that IScsiDxe uses only one connection per session (put
differently: knowing that any connection is a leading connection, and any
login is a leading login), there is no functionality bug. But the data
model is still broken: "AuthType", "AuthData", and "AuthStep" should be
maintained at the *same* level -- be it "session-level" or "(leading)
connection-level".
Fixing this data model bug is more than what I'm signing up for. However,
I do need to add one function, in preparation for multi-hash support:
whenever a new login is attempted (put differently: whenever the leading
login is re-attempted), which always happens with a fresh connection, the
session-level authentication data needs to be rewound to a sane initial
state.
Introduce the IScsiSessionResetAuthData() function. Call it from the
central -- session-level -- IScsiSessionLogin() function, just before the
latter calls the -- connection-level -- IScsiConnLogin() function.
Right now, do nothing in IScsiSessionResetAuthData(); so functionally
speaking, the patch is a no-op.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3355
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210629163337.14120-2-lersek@redhat.com>
IScsiDxe (that is, the initiator) receives two hex-encoded strings from
the iSCSI target:
- CHAP_C, where the target challenges the initiator,
- CHAP_R, where the target answers the challenge from the initiator (in
case the initiator wants mutual authentication).
Accordingly, we have two IScsiHexToBin() call sites:
- At the CHAP_C decoding site, check whether the decoding succeeds. The
decoded buffer ("AuthData->InChallenge") can accommodate 1024 bytes,
which is a permissible restriction on the target, per
<https://tools.ietf.org/html/rfc7143#section-12.1.3>. Shorter challenges
from the target are acceptable.
- At the CHAP_R decoding site, enforce that the decoding both succeed, and
provide exactly ISCSI_CHAP_RSP_LEN bytes. CHAP_R contains the digest
calculated by the target, therefore it must be of fixed size. We may
only call IScsiCHAPAuthTarget() if "TargetRsp" has been fully populated.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Message-Id: <20210608121259.32451-11-lersek@redhat.com>
The IScsiHexToBin() function documents the EFI_BUFFER_TOO_SMALL return
condition, but never actually checks whether the decoded buffer fits into
the caller-provided room (i.e., the input value of "BinLength"), and
EFI_BUFFER_TOO_SMALL is never returned. The decoding of "HexStr" can
overflow "BinBuffer".
This is remotely exploitable, as shown in a subsequent patch, which adds
error checking to the IScsiHexToBin() call sites. This issue allows the
target to compromise the initiator.
Introduce EFI_BAD_BUFFER_SIZE, in addition to the existent
EFI_BUFFER_TOO_SMALL, for reporting a special case of the buffer overflow,
plus actually catch the buffer overflow.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210608121259.32451-10-lersek@redhat.com>
The IScsiHexToBin() function has the following parser issues:
(1) If the *subject sequence* in "HexStr" is empty, the function returns
EFI_SUCCESS (with "BinLength" set to 0 on output). Such inputs should
be rejected.
(2) The function mis-handles a "HexStr" that ends with a stray nibble. For
example, if "HexStr" is "0xABC", the function decodes it to the bytes
{0xAB, 0x0C}, sets "BinLength" to 2 on output, and returns
EFI_SUCCESS. Such inputs should be rejected.
(3) If an invalid hex char is found in "HexStr", the function treats it as
end-of-hex-string, and returns EFI_SUCCESS. Such inputs should be
rejected.
All of the above cases are remotely triggerable, as shown in a subsequent
patch, which adds error checking to the IScsiHexToBin() call sites. While
the initiator is not immediately compromised, incorrectly parsing CHAP_R
from the target, in case of mutual authentication, is not great.
Extend the interface contract of IScsiHexToBin() with
EFI_INVALID_PARAMETER, for reporting issues (1) through (3), and implement
the new checks.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210608121259.32451-9-lersek@redhat.com>
We'll need further return values for IScsiHexToBin() in a subsequent
patch; make room for them in the leading comment block of the function.
While at it, rewrap the comment block to 80 characters width.
No functional changes.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210608121259.32451-8-lersek@redhat.com>
IScsiBinToHex() is called for encoding:
- the answer to the target's challenge; that is, CHAP_R;
- the challenge for the target, in case mutual authentication is enabled;
that is, CHAP_C.
The initiator controls the size of both blobs, the sizes of their hex
encodings are correctly calculated in "RspLen" and "ChallengeLen".
Therefore the IScsiBinToHex() calls never fail; assert that.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Message-Id: <20210608121259.32451-7-lersek@redhat.com>
Considering IScsiBinToHex():
> if (((*HexLength) - 3) < BinLength * 2) {
> *HexLength = BinLength * 2 + 3;
> }
the following subexpressions are problematic:
(*HexLength) - 3
BinLength * 2
BinLength * 2 + 3
The first one may wrap under zero, the latter two may wrap over
MAX_UINT32.
Rewrite the calculation using SafeIntLib.
While at it, change the type of the "Index" variable from UINTN to UINT32.
The largest "Index"-based value that we calculate is
Index * 2 + 2 (with (Index == BinLength))
Because the patch makes
BinLength * 2 + 3
safe to calculate in UINT32, using UINT32 for
Index * 2 + 2 (with (Index == BinLength))
is safe too. Consistently using UINT32 improves readability.
This patch is best reviewed with "git show -W".
The integer overflows that this patch fixes are theoretical; a subsequent
patch in the series will audit the IScsiBinToHex() call sites, and show
that none of them can fail.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210608121259.32451-6-lersek@redhat.com>
Sort the library class dependencies in the #include directives and in the
INF file. Remove the DpcLib class from the #include directives -- it is
not listed in the INF file, and IScsiDxe doesn't call either DpcLib API
(QueueDpc(), DispatchDpc()). No functional changes.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Message-Id: <20210608121259.32451-5-lersek@redhat.com>
The "ISCSI_CHAP_AUTH_DATA.OutChallenge" field is declared as a UINT8 array
with ISCSI_CHAP_AUTH_MAX_LEN (1024) elements. However, when the challenge
is generated and formatted, only ISCSI_CHAP_RSP_LEN (16) octets are used
in the array.
Change the array size to ISCSI_CHAP_RSP_LEN, and remove the (now unused)
ISCSI_CHAP_AUTH_MAX_LEN macro.
Remove the "ISCSI_CHAP_AUTH_DATA.OutChallengeLength" field, which is
superfluous too.
Most importantly, explain in a new comment *why* tying the challenge size
to the digest size (ISCSI_CHAP_RSP_LEN) has always made sense. (See also
Linux kernel commit 19f5f88ed779, "scsi: target: iscsi: tie the challenge
length to the hash digest size", 2019-11-06.) For sure, the motivation
that the new comment now explains has always been there, and has always
been the same, for IScsiDxe; it's just that now we spell it out too.
No change in peer-visible behavior.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Message-Id: <20210608121259.32451-4-lersek@redhat.com>
The ISCSI_CHAP_AUTH_MAX_LEN macro is defined with value 1024.
The usage of this macro currently involves a semantic (not functional)
bug, which we're going to fix in a subsequent patch, eliminating
ISCSI_CHAP_AUTH_MAX_LEN altogether.
For now, remove the macro's usage from all
"ISCSI_CHAP_AUTH_DATA.InChallenge" contexts. This is doable without
duplicating open-coded constants.
No changes in functionality.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Message-Id: <20210608121259.32451-3-lersek@redhat.com>
Working with overlong lines is difficult for me; rewrap the CHAP-related
source files in IScsiDxe to 80 characters width. No functional changes.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210608121259.32451-2-lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3246
MdeLibs.dsc.inc was added for some basic/default library
instances provided by MdePkg and RegisterFilterLibNull Library
was also added into it as the first version of MdeLibs.dsc.inc.
So update platform dsc to consume MdeLibs.dsc.inc for
RegisterFilterLibNull which will be consumed by IoLib and BaseLib.
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
https://bugzilla.tianocore.org/show_bug.cgi?id=3170
Using PcdHttpIoTimeout to set default timeout value to HttpIoLib.
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: GregX Yeh <gregx.yeh@intel.com>
Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Move HTTP header manipulation functions to DxeHttpLib from
HttpBootSupport.c. These general functions are used by both
Http BOOT and RedfishLib (patches will be sent later).
Signed-off-by: Abner Chang <abner.chang@hpe.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Fan Wang <fan.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Nickle Wang <nickle.wang@hpe.com>
Cc: Peter O'Hanley <peter.ohanley@hpe.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Remove HTTP IO realted funcitons from HttpBootDxe
and use HttpIoLib instead.
Signed-off-by: Abner Chang <abner.chang@hpe.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Nickle Wang <nickle.wang@hpe.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Michael Kubacki
Zachary Clark-Williams
Chao Li
Saloni Kasbekar
Heng Luo
Rebecca Cran
Oliver Steffen
Liming Gao
Michael D Kinney
Vineel Kovvuri
Lin, Gary (HPS OE-Linux)
Zachary Clark-Williams
Laszlo Ersek
Dandan Bi
GregX Yeh
Abner Chang
wenyi,xie via groups.io