On Mon, May 2, 2022 at 12:05 AM krishna t <krish271828@xxxxxxxxx> wrote: > > On Sun, May 1, 2022 at 9:12 PM Jouni Malinen <j@xxxxx> wrote: > > > > On Sun, Apr 24, 2022 at 09:40:34PM +0530, krish271828@xxxxxxxxx wrote: > > > This is ideal for low footprint embedded systems, the patch adds support > > > for STA mode, the support is minimal, only mandatory stuff is added > > > > > > * DH group19 only for SAE > > > * OWE, DPP and PASN are not supported as ECDH support is yet to be > > > added. > > > * EAP-TEAP is not supported yet > > > > > > This implementation is based on work of ESP team [1], with below changes > > > * decouple from esp_wifi > > > * avoid any changes to core wpa_supplicant (esp. for WPA3) > > > * port to 3.1.0 mbedtls > > > > > > Tested WPA2/WPA3 associations personal/enterprise. Also, add a > > > configuration file with mbedtls for build tests. > > > > > > Code auto-formatted using wpa_supplicant/binder/.clang-format. > > > > > > [1] - https://github.com/espressif/esp-idf/tree/master/components/wpa_supplicant > > > > > diff --git a/src/crypto/crypto.h b/src/crypto/crypto.h > > > +int crypto_bignum_bits(const struct crypto_bignum *a); > > > > This does not seem to be used anywhere. I would prefer to separate > > crypto.h additions into their own patch to keep things clearer. That > > said, if this is not really going to be used anywhere, it does not look > > helpful to add this function at this point. > Yes, this is unused, I can remove this, and as this is the only change > no need for a separate commit. > > > > > > > diff --git a/src/crypto/crypto_mbedtls-bignum.c b/src/crypto/crypto_mbedtls-bignum.c > > > +/* > > > + * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD > > > + * > > > + * SPDX-License-Identifier: Apache-2.0 > > > + */ > > Is there particular need for using the Apache license for these new > > files? I would prefer to use the same BSD license for all of the > > hostap.git implementation and there would need to be strong > > justification for using something different. > Sorry, I wasn't too sure about this and have not tried to contact the ESP > team. As they own the code, I cannot change the license type, and > basic googling suggested that we can use a mix of Apache2.0 > and BSD as long as we keep the original license as is. Though some > claim the resulting code has to be Apache2.0 as it is less permissive > than BSD. > > > > This patch would seem to make hostap.git contents not comply with the > > license requirements ("You must give any other recipients of the Work or > > Derivative Works a copy of this License;" and the License text is not > > included here), so even if I were to accept a different license to be > > used, I could not accept something that would not meet the actual > > license requirements. > So, IIUC, using SPDX ID's implies license text, no? I can surely add the > license file if that satisfies the needs. And also, I guess I should add a note > at the top that I have modified these. > > > > > +// Lifted from https://stackoverflow.com/a/47117431 > > > +char *strremove(char *str, const char *sub) > > ... > > > > > +// Lifted from: https://stackoverflow.com/a/779960 > > > +// You must free the result if result is non-NULL. > > > +char *str_replace(char *orig, char *rep, char *with) > > ... > > > > Can you please clarify what you mean with "lifted from"? I read this as > > "taken without explicit permission and without identifying under which > > license this is used". If that is accurate, I cannot accept such > > contribution due to unclear license situation. > Yes, this is copied as is, and that post is licensed under > https://creativecommons.org/licenses/by-sa/4.0/ (mentioned in the share link) > and mentioning the source should satisfy the license IIUC. I can change > "Lifted from" to "original source" and move them to a separate file with SPDX > ID for those two functions? > And anyways, these changes are not strictly needed, they were made > to pass the hwsim tests.So, can use a simple switch case and avoid this > algother. > > > > > diff --git a/tests/build/run-build-tests.sh b/tests/build/run-build-tests.sh > > > @@ -8,13 +8,10 @@ popd > /dev/null > > > echo "Build test directory: $DIR" > > > echo > > > > > > -for i in build-hostapd-*.config; do > > > - ./build-hostapd.sh $DIR $i > > > -done > > > - > > > -for i in build-wpa_supplicant-*.config; do > > > +for i in build-wpa_supplicant-mbedtls-3.1.0.config; do > > > ./build-wpa_supplicant.sh $DIR $i > > > done > > > > > > echo > > > echo "Build test directory: $DIR" > > > +cat $DIR/wpa_supplicant-mbedtls-3.1.0.log-* > > > > These changes are clearly not appropriate. It is fine to add a new test > > config, but the test script must not be broken for other uses. > Sorry, these are local changes, will be removed. > > > > > diff --git a/wpa_supplicant/Makefile b/wpa_supplicant/Makefile > > > @@ -35,7 +35,7 @@ export INCDIR ?= /usr/local/include > > > export BINDIR ?= /usr/local/sbin > > > PKG_CONFIG ?= pkg-config > > > > > > -CFLAGS += $(EXTRA_CFLAGS) > > > +CFLAGS += $(EXTRA_CFLAGS) -Werror > > > > How is this connected to this particular patch? > Sorry, I was preparing another patch, spilled into here, will be removed. > > > > > @@ -145,6 +145,7 @@ ifndef CONFIG_ELOOP > > > CONFIG_ELOOP=eloop > > > endif > > > OBJS += ../src/utils/$(CONFIG_ELOOP).o > > > +OBJS_p += ../src/utils/$(CONFIG_ELOOP).o > > > OBJS_c += ../src/utils/$(CONFIG_ELOOP).o > > > > Why is this needed? > Sorry, these are local changes, will be removed. > > > @@ -258,9 +259,12 @@ ifdef CONFIG_SAE > > > CFLAGS += -DCONFIG_SAE > > > OBJS += ../src/common/sae.o > > > ifdef CONFIG_SAE_PK > > > +# NO ECDH support yet > > > +ifneq ($(CONFIG_TLS), mbedtls) > > > CFLAGS += -DCONFIG_SAE_PK > > > OBJS += ../src/common/sae_pk.o > > > endif > > > +endif > > > > I'd prefer the build to fail instead of hide the fact that explicitly > > enabled build option could not be included, i.e., I would not add this > > type of exceptions into Makefile. > Okay. I will use $(error) as we do for other checks. I was waiting for you ACK on licensing before submitting a new patch with fixes for the others, sorry if that wasn't obvious. Else, I can submit a new patch right away. Please suggest. _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap
