Re: [edk2-devel] [PATCH V2 2/2] NetworkPkg: Add WiFi profile sync protocol support
Thank you Heng and all,
My responses:
1. Fixed.
2. Fixed, it was a missed change now added to the new patch.
3. Added CC.
Sending an updated patch file for reviewal.
Thanks,
Zack
-Original Message-
From: Luo, Heng
Sent: Tuesday, September 27, 2022 12:31 AM
To: devel@edk2.groups.io; Clark-williams, Zachary
Cc: Zachary Clark-Williams
Subject: RE: [edk2-devel] [PATCH V2 2/2] NetworkPkg: Add WiFi profile sync
protocol support
Hi Zack,
1.
> + Status = UnicodeStrToAsciiStrS (Profile->Password, (CHAR8
> + *)AsciiPassword, ((StrLen (Profile->Password) + 1) * sizeof
> + (CHAR8)));
I think we should remove '* sizeof (CHAR8)' because the third parameter is the
length of string but not size of the memory:
Status = UnicodeStrToAsciiStrS (Profile->Password, (CHAR8
*)AsciiPassword, ((StrLen (Profile->Password) + 1)));
2.
> + if (StrLen (Profile->Password) > PASSWORD_STORAGE_SIZE) {
> +ASSERT (EFI_INVALID_PARAMETER);
> +return EFI_INVALID_PARAMETER;
> + }
Need null terminator at end of password, so I think the conditional should be:
if (StrLen (Profile->Password) >= PASSWORD_STORAGE_SIZE) in order to
support 32 bytes PSK passwords with null terminator. Maybe need to change
PASSWORD_STORAGE_SIZE to 65? But PASSWORD_MAX_LEN should still be 63, because
according to the 802.11i specification:
A pass-phrase is a sequence of between 8 and 63 ASCII-encoded characters. The
limit of 63 comes from the desire to distinguish between a pass-phrase and a
PSK displayed as 64 hexadecimal characters.
3.
Suggest to add maintainers in Cc of the commit msg to speed up review.
Fox example:
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
Cc: Fu Siyuan
Cc: Wu Jiaxin
Signed-off-by: Zachary Clark-Williams
Thanks,
Heng
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of Clark-
> williams, Zachary
> Sent: Tuesday, September 27, 2022 2:20 AM
> To: devel@edk2.groups.io
> Cc: Zachary Clark-Williams ; Clark-williams,
> Zachary
> Subject: [edk2-devel] [PATCH V2 2/2] NetworkPkg: Add WiFi profile sync
> protocol support
>
> From: Zachary Clark-Williams
>
> 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.
>
> Signed-off-by: Zachary Clark-Williams
>
> ---
> .../Protocol/WiFiProfileSyncProtocol.h| 83
> NetworkPkg/NetworkPkg.dec | 3 +
> .../WifiConnectionManagerDxe.inf | 3 +-
> .../WifiConnectionMgrDriver.c | 126
> .../WifiConnectionMgrDxe.h| 4 +-
> .../WifiConnectionMgrImpl.c | 193 --
> .../WifiConnectionMgrMisc.c | 13 ++
> 7 files changed, 366 insertions(+), 59 deletions(-) create mode
> 100644 NetworkPkg/Include/Protocol/WiFiProfileSyncProtocol.h
>
> diff --git a/NetworkPkg/Include/Protocol/WiFiProfileSyncProtocol.h
> b/NetworkPkg/Include/Protocol/WiFiProfileSyncProtocol.h
> new file mode 100644
> index 00..e36daceabf
> --- /dev/null
> +++ b/NetworkPkg/Include/Protocol/WiFiProfileSyncProtocol.h
> @@ -0,0 +1,83 @@
> +/** @file
> + WiFi profile sync protocol. Supports One Click Recovery or KVM OS
> +recovery
> + boot flow over WiFi.
> +
> + Copyright (c) 2022, Intel Corporation. All rights reserved.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent **/
> +
> +#ifndef WIFI_PROFILE_SYNC_PROTOCOL_H_ #define
> +WIFI_PROFILE_SYNC_PROTOCOL_H_
> +
> +#include
> +
> +//
> +// WiFi Profile Sync Protocol GUID variable.
> +//
> +extern EFI_GUID gEfiWiFiProfileSyncProtocolGuid;
> +
> +/**
> + Used by the WiFi connection manager to get the WiFi profile that
> +AMT shared
> + and was stored in WiFi profile protocol. Aligns the AMT WiFi
> +profile data to
> + the WiFi connection manager profile structure fo connection use.
> +
> + @param[in, out] WcmProfile WiFi Connection Manager profile
> structure
> + @param[in, out] MacAddress MAC address from AMT saved to NiC
> MAC address
> +
> + @retval EFI_SUCCESS Stored WiFi profile converted and
> returned
> succefully
> + @retval EFI_UNSUPPORTED Pro
Re: [edk2-devel] [PATCH V2 2/2] NetworkPkg: Add WiFi profile sync protocol support
Hi Zack,
1.
> + Status = UnicodeStrToAsciiStrS (Profile->Password, (CHAR8
> + *)AsciiPassword, ((StrLen (Profile->Password) + 1) * sizeof (CHAR8)));
I think we should remove '* sizeof (CHAR8)' because the third parameter is the
length of string but not size of the memory:
Status = UnicodeStrToAsciiStrS (Profile->Password, (CHAR8
*)AsciiPassword, ((StrLen (Profile->Password) + 1)));
2.
> + if (StrLen (Profile->Password) > PASSWORD_STORAGE_SIZE) {
> +ASSERT (EFI_INVALID_PARAMETER);
> +return EFI_INVALID_PARAMETER;
> + }
Need null terminator at end of password, so I think the conditional should be:
if (StrLen (Profile->Password) >= PASSWORD_STORAGE_SIZE)
in order to support 32 bytes PSK passwords with null terminator. Maybe need to
change PASSWORD_STORAGE_SIZE to 65? But PASSWORD_MAX_LEN should still be 63,
because according to the 802.11i specification:
A pass-phrase is a sequence of between 8 and 63 ASCII-encoded characters. The
limit of 63 comes from the desire to distinguish between a pass-phrase and a
PSK displayed as 64 hexadecimal characters.
3.
Suggest to add maintainers in Cc of the commit msg to speed up review.
Fox example:
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
Cc: Fu Siyuan
Cc: Wu Jiaxin
Signed-off-by: Zachary Clark-Williams
Thanks,
Heng
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of Clark-
> williams, Zachary
> Sent: Tuesday, September 27, 2022 2:20 AM
> To: devel@edk2.groups.io
> Cc: Zachary Clark-Williams ; Clark-williams, Zachary
>
> Subject: [edk2-devel] [PATCH V2 2/2] NetworkPkg: Add WiFi profile sync
> protocol support
>
> From: Zachary Clark-Williams
>
> 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.
>
> Signed-off-by: Zachary Clark-Williams
> ---
> .../Protocol/WiFiProfileSyncProtocol.h| 83
> NetworkPkg/NetworkPkg.dec | 3 +
> .../WifiConnectionManagerDxe.inf | 3 +-
> .../WifiConnectionMgrDriver.c | 126
> .../WifiConnectionMgrDxe.h| 4 +-
> .../WifiConnectionMgrImpl.c | 193 --
> .../WifiConnectionMgrMisc.c | 13 ++
> 7 files changed, 366 insertions(+), 59 deletions(-) create mode 100644
> NetworkPkg/Include/Protocol/WiFiProfileSyncProtocol.h
>
> diff --git a/NetworkPkg/Include/Protocol/WiFiProfileSyncProtocol.h
> b/NetworkPkg/Include/Protocol/WiFiProfileSyncProtocol.h
> new file mode 100644
> index 00..e36daceabf
> --- /dev/null
> +++ b/NetworkPkg/Include/Protocol/WiFiProfileSyncProtocol.h
> @@ -0,0 +1,83 @@
> +/** @file
> + WiFi profile sync protocol. Supports One Click Recovery or KVM OS
> +recovery
> + boot flow over WiFi.
> +
> + Copyright (c) 2022, Intel Corporation. All rights reserved.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent **/
> +
> +#ifndef WIFI_PROFILE_SYNC_PROTOCOL_H_
> +#define WIFI_PROFILE_SYNC_PROTOCOL_H_
> +
> +#include
> +
> +//
> +// WiFi Profile Sync Protocol GUID variable.
> +//
> +extern EFI_GUID gEfiWiFiProfileSyncProtocolGuid;
> +
> +/**
> + Used by the WiFi connection manager to get the WiFi profile that AMT
> +shared
> + and was stored in WiFi profile protocol. Aligns the AMT WiFi profile
> +data to
> + the WiFi connection manager profile structure fo connection use.
> +
> + @param[in, out] WcmProfile WiFi Connection Manager profile
> structure
> + @param[in, out] MacAddress MAC address from AMT saved to NiC
> MAC address
> +
> + @retval EFI_SUCCESS Stored WiFi profile converted and
> returned
> succefully
> + @retval EFI_UNSUPPORTED Profile protocol sharing not supported or
> enabled
> + @retval EFI_NOT_FOUND No profiles to returned
> + @retval OthersError Occurred
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *WIFI_PROFILE_GET)(
> + IN OUT WIFI_MGR_NETWORK_PROFILE *Profile,
> + IN OUT EFI_80211_MAC_ADDRESS MacAddress
> + );
> +
> +/**
> + Saves the WiFi connection status recieved by the
> +WiFiCon
[edk2-devel] [PATCH V2 2/2] NetworkPkg: Add WiFi profile sync protocol support
From: Zachary Clark-Williams
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.
Signed-off-by: Zachary Clark-Williams
---
.../Protocol/WiFiProfileSyncProtocol.h| 83
NetworkPkg/NetworkPkg.dec | 3 +
.../WifiConnectionManagerDxe.inf | 3 +-
.../WifiConnectionMgrDriver.c | 126
.../WifiConnectionMgrDxe.h| 4 +-
.../WifiConnectionMgrImpl.c | 193 --
.../WifiConnectionMgrMisc.c | 13 ++
7 files changed, 366 insertions(+), 59 deletions(-)
create mode 100644 NetworkPkg/Include/Protocol/WiFiProfileSyncProtocol.h
diff --git a/NetworkPkg/Include/Protocol/WiFiProfileSyncProtocol.h
b/NetworkPkg/Include/Protocol/WiFiProfileSyncProtocol.h
new file mode 100644
index 00..e36daceabf
--- /dev/null
+++ b/NetworkPkg/Include/Protocol/WiFiProfileSyncProtocol.h
@@ -0,0 +1,83 @@
+/** @file
+ WiFi profile sync protocol. Supports One Click Recovery or KVM OS recovery
+ boot flow over WiFi.
+
+ Copyright (c) 2022, Intel Corporation. All rights reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef WIFI_PROFILE_SYNC_PROTOCOL_H_
+#define WIFI_PROFILE_SYNC_PROTOCOL_H_
+
+#include
+
+//
+// WiFi Profile Sync Protocol GUID variable.
+//
+extern EFI_GUID gEfiWiFiProfileSyncProtocolGuid;
+
+/**
+ Used by the WiFi connection manager to get the WiFi profile that AMT shared
+ and was stored in WiFi profile protocol. Aligns the AMT WiFi profile data to
+ the WiFi connection manager profile structure fo connection use.
+
+ @param[in, out] WcmProfile WiFi Connection Manager profile structure
+ @param[in, out] MacAddress MAC address from AMT saved to NiC MAC
address
+
+ @retval EFI_SUCCESS Stored WiFi profile converted and returned
succefully
+ @retval EFI_UNSUPPORTED Profile protocol sharing not supported or
enabled
+ @retval EFI_NOT_FOUND No profiles to returned
+ @retval OthersError Occurred
+**/
+typedef
+EFI_STATUS
+(EFIAPI *WIFI_PROFILE_GET)(
+ IN OUT WIFI_MGR_NETWORK_PROFILE *Profile,
+ IN OUT EFI_80211_MAC_ADDRESS MacAddress
+ );
+
+/**
+ Saves the WiFi connection status recieved by the WiFiConnectionManager when
+ in a KVM OR One Click Recovery WLAN recovery flow. Input as
+ EFI_80211_CONNECT_NETWORK_RESULT_CODE then converted and stored as
EFI_STATUS type.
+
+ @param[in] ConnectionStatus WiFi connection attempt results
+**/
+typedef
+VOID
+(EFIAPI *WIFI_SET_CONNECT_STATE)(
+ IN EFI_80211_CONNECT_NETWORK_RESULT_CODE ConnectionStatus
+ );
+
+/**
+ Retrieves the stored WiFi connection status when in either KVM OR One Click
+ Recovery WLAN recovery flow.
+
+ @retval EFI_SUCCESS WiFi connection completed succesfully
+ @retval OthersConnection failure occurred
+**/
+typedef
+EFI_STATUS
+(EFIAPI *WIFI_GET_CONNECT_STATE)(
+ VOID
+ );
+
+//
+// WiFi Profile Sync Protocol structure.
+//
+typedef struct {
+ UINT32Revision;
+ WIFI_SET_CONNECT_STATEWifiProfileSyncSetConnectState;
+ WIFI_GET_CONNECT_STATEWifiProfileSyncGetConnectState;
+ WIFI_PROFILE_GET WifiProfileSyncGetProfile;
+} EFI_WIFI_PROFILE_SYNC_PROTOCOL;
+
+/**
+ WiFi Profile Protocol revision number.
+
+ Revision 1: Initial version
+**/
+#define EFI_WIFI_PROFILE_SYNC_PROTOCOL_REVISION 1
+
+#endif // WIFI_PROFILE_SYNC_PROTOCOL_H_
diff --git a/NetworkPkg/NetworkPkg.dec b/NetworkPkg/NetworkPkg.dec
index 5e43ebf8c5..53fb34c4a0 100644
--- a/NetworkPkg/NetworkPkg.dec
+++ b/NetworkPkg/NetworkPkg.dec
@@ -91,6 +91,9 @@
## Include/Protocol/HttpCallback.h
gEdkiiHttpCallbackProtocolGuid = {0x64f1, 0xa37b, 0x4468, {0xa4, 0x36,
0x5b, 0xdd, 0xa1, 0x6a, 0xa2, 0x40}}
+ ## Include/Protocol/WiFiProfileSyncProtocol.h
+ gEfiWiFiProfileSyncProtocolGuid = {0x399a2b8a, 0xc267, 0x44aa, {0x9a, 0xb4,
0x30, 0x58, 0x8c, 0xd2, 0x2d, 0xcc}}
+
[PcdsFixedAtBuild]
## The max attempt number will be created by iSCSI driver.
# @Prompt Max attempt number.
diff --git a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionManagerDxe.inf
b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionManagerDxe.inf
index 4394b6f4bb..7e36016cf8 100644
--- a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionManagerDxe.inf
+++ b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionManagerDxe.inf
@@ -9,7 +9,7 @@
# 2). WPA2 Personal Network
# 3). EAP Networks (EAP-TLS, EAP-TTLS/MSCHAPv2 and PEAPv0/MSCHAPv2)
#
-#
