Skip to content

net: l2: wifi: Refactor certificates processing code into common file #93119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

D-Triveni
Copy link
Contributor

Refactor certificate processing code to eliminate duplication and enable reuse across modules that require enterprise support.

This change is to fix the eloop.h header file path.

Signed-off-by: Triveni Danda <[email protected]>
@D-Triveni D-Triveni force-pushed the refactor_wifi_certs_process branch from d2e696a to 134caa6 Compare July 15, 2025 11:27
Copy link

github-actions bot commented Jul 15, 2025

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff
hostap zephyrproject-rtos/hostap@bc5d22f zephyrproject-rtos/hostap#92 zephyrproject-rtos/hostap#92/files

DNM label due to: 1 project with PR revision

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@github-actions github-actions bot added manifest manifest-hostap DNM (manifest) This PR should not be merged (controlled by action-manifest) labels Jul 15, 2025
Refactor certificate processing code to eliminate duplication and
enable reuse across modules that require enterprise support.

Signed-off-by: Triveni Danda <[email protected]>
* @param AP or Station mode
*/
int wifi_set_enterprise_credentials(struct net_if *iface,
bool is_ap);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong indent

* SPDX-License-Identifier: Apache-2.0
*/

#ifndef WIFI_CREDS_H__
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix the guards to match the name

Comment on lines 13 to 16
#include <utils/common.h>
#include <eap_peer/eap_config.h>
#include <ctrl_iface_zephyr.h>
#include <wpa_supplicant/config.h>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see these being used in this file, please move to C file.

Comment on lines 37 to 36
* Clear Wi-Fi enterprise credentials
* @param Wi-Fi enterprise params
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has to be doxygen compliant, newline after brief.

enterprise_creds_params.server_key_len = ARRAY_SIZE(server_key_test);
}

void wifi_clear_enterprise_credentials(void)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a comment as to why this is no-op

@@ -22,247 +22,15 @@

#include <zephyr/net/wifi_credentials.h>

#ifdef CONFIG_WIFI_NM_WPA_SUPPLICANT_CRYPTO_ENTERPRISE
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't we use the new Kconfig itself? WIFI_CERTIFICATE_LIB

@@ -128,6 +128,10 @@ config WIFI_ENT_IDENTITY_MAX_USERS

if WIFI_NM_WPA_SUPPLICANT_CRYPTO_ENTERPRISE

config WIFI_CERTIFICATE_LIB
bool "Process certificates in enterprise mode"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we even make this configurable? What happens if use enables Enterprise but disables this?

@D-Triveni D-Triveni force-pushed the refactor_wifi_certs_process branch from 134caa6 to 1ed0285 Compare July 15, 2025 12:47
Copy link
Collaborator

@krish2718 krish2718 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, do we need an entry in the changelog?

Copy link

@krish2718
Copy link
Collaborator

LGTM, do we need an entry in the changelog?

hold-off on this till it's confirmed that it's part of 4.2 which I doubt as its too close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking area: Wi-Fi Wi-Fi DNM (manifest) This PR should not be merged (controlled by action-manifest) manifest manifest-hostap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants