You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/01/18 09:05:48 UTC

[GitHub] [incubator-nuttx-apps] btashton opened a new pull request #566: WIP: crypto: Initial support for mbedtls

btashton opened a new pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566


   ## Summary
   **DRAFT***  Add initial support for Mbed TLS in NuttX
    * This is not really ready for review, but I wanted to get this out there to make sure I am not duplicating work *
   
    TODO:
   - [x] Download an build Mbed TLS
   - [ ] Add support for generating the needed config header via Kconfig system
   - [ ] Support basic entropy and crypto primitives
   
   x509 and hardware driver support is out of scope for this PR
   
   ## Testing
   So far it is able to generate data from the entropy api with /dev/urandom configured.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r559961827



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,68 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+MBEDTLS_DIR = .
+MBEDTLS_DIR_NAME = mbedtls
+
+# Set up build configuration and environment
+
+WD := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKDIR =  $(WD)$(DELIM)$(MBEDTLS_UNPACKNAME)
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKDIR)$(DELIM)library
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += -D__unix__
+CFLAGS += -I$(MBEDTLS_UNPACKDIR)$(DELIM)include
+
+MBEDTLS_LIB_CSRCS := $(addprefix $(MBEDTLS_UNPACKLIBDIR)$(DELIM), $(notdir $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)))

Review comment:
       could simplify to:
   CSRCS =  $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)

##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,68 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+MBEDTLS_DIR = .
+MBEDTLS_DIR_NAME = mbedtls
+
+# Set up build configuration and environment
+
+WD := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKDIR =  $(WD)$(DELIM)$(MBEDTLS_UNPACKNAME)
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKDIR)$(DELIM)library
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += -D__unix__
+CFLAGS += -I$(MBEDTLS_UNPACKDIR)$(DELIM)include
+
+MBEDTLS_LIB_CSRCS := $(addprefix $(MBEDTLS_UNPACKLIBDIR)$(DELIM), $(notdir $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)))
+
+CSRCS += $(MBEDTLS_LIB_CSRCS)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)$(DELIM)$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)$(DELIM)$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)

Review comment:
       don't need touch folder

##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,68 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+MBEDTLS_DIR = .
+MBEDTLS_DIR_NAME = mbedtls
+
+# Set up build configuration and environment
+
+WD := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKDIR =  $(WD)$(DELIM)$(MBEDTLS_UNPACKNAME)
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKDIR)$(DELIM)library
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += -D__unix__
+CFLAGS += -I$(MBEDTLS_UNPACKDIR)$(DELIM)include

Review comment:
       don't need

##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,68 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+MBEDTLS_DIR = .

Review comment:
       remove no use

##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,68 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+MBEDTLS_DIR = .
+MBEDTLS_DIR_NAME = mbedtls
+
+# Set up build configuration and environment
+
+WD := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKDIR =  $(WD)$(DELIM)$(MBEDTLS_UNPACKNAME)

Review comment:
       remove $(WD), the relative path work fine

##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,68 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+MBEDTLS_DIR = .
+MBEDTLS_DIR_NAME = mbedtls

Review comment:
       why not reuse MBEDTLS_UNPACKNAME 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563462650



##########
File path: crypto/mbedtls/Make.defs
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifeq ($(CONFIG_CRYPTO_MBEDTLS),y)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls
+
+# Allows `<mbedtls/<>.h>` import.
+
+CFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+CXXFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+
+CFLAGS += -DMBEDTLS_CONFIG_FILE="<crypto/mbedtls_config.h>"

Review comment:
       Had to wrap the string in single quotes as well to get the command formatted correctly but seems good now.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563491758



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,91 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}
+
+CSRCS = $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)/$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)/$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)
+
+context:: $(MBEDTLS_UNPACKNAME)
+
+distclean::
+	$(call DELDIR, $(MBEDTLS_UNPACKNAME))
+	$(call DELFILE, $(MBEDTLS_ZIP))
+
+# Configuration Applications
+
+ifneq ($(CONFIG_MBEDTLS_APPS),)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls/

Review comment:
       CONFIGURED_APPS need define in Make.def, apps/Makefile can't reference it if we define it here.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-898525787


   @gustavonihei very sorry I have had to step mostly away from the project for personal reasons for a while, but this week I have started coming back. I saw you PR with the additional notice (thanks) I think we can get this all closed out in the next couple days. I would like to get the other notices up as well that we need. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-961137634


   It's fine to me to merge this PR first and refine later.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] acassis commented on pull request #566: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-961302209


   Great work @btashton and @gustavonihei !


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r559716140



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,133 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+MBEDTLS_DIR = .
+MBEDTLS_DIR_NAME = mbedtls
+
+# Set up build configuration and environment
+
+WD := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -o
+
+MBEDTLS_UNPACKDIR =  $(WD)/$(MBEDTLS_UNPACKNAME)
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKDIR)/library
+
+# Help
+CFLAGS += -D__unix__
+CFLAGS += -I$(MBEDTLS_UNPACKDIR)/include
+
+# Crypto Source
+CSRCS += \
+    $(MBEDTLS_UNPACKLIBDIR)/aesni.c \
+    $(MBEDTLS_UNPACKLIBDIR)/arc4.c \
+    $(MBEDTLS_UNPACKLIBDIR)/aria.c \
+    $(MBEDTLS_UNPACKLIBDIR)/asn1parse.c \
+    $(MBEDTLS_UNPACKLIBDIR)/asn1write.c \
+    $(MBEDTLS_UNPACKLIBDIR)/base64.c \
+    $(MBEDTLS_UNPACKLIBDIR)/bignum.c \
+    $(MBEDTLS_UNPACKLIBDIR)/blowfish.c \
+    $(MBEDTLS_UNPACKLIBDIR)/camellia.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ccm.c \
+    $(MBEDTLS_UNPACKLIBDIR)/chacha20.c \
+    $(MBEDTLS_UNPACKLIBDIR)/chachapoly.c \
+    $(MBEDTLS_UNPACKLIBDIR)/cipher.c \
+    $(MBEDTLS_UNPACKLIBDIR)/cipher_wrap.c \
+    $(MBEDTLS_UNPACKLIBDIR)/cmac.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ctr_drbg.c \
+    $(MBEDTLS_UNPACKLIBDIR)/des.c \
+    $(MBEDTLS_UNPACKLIBDIR)/dhm.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecdh.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecdsa.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecjpake.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecp.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecp_curves.c \
+    $(MBEDTLS_UNPACKLIBDIR)/entropy.c \
+    $(MBEDTLS_UNPACKLIBDIR)/entropy_poll.c \
+    $(MBEDTLS_UNPACKLIBDIR)/error.c \
+    $(MBEDTLS_UNPACKLIBDIR)/gcm.c \
+    $(MBEDTLS_UNPACKLIBDIR)/havege.c \
+    $(MBEDTLS_UNPACKLIBDIR)/hkdf.c \
+    $(MBEDTLS_UNPACKLIBDIR)/hmac_drbg.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md2.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md4.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md5.c \
+    $(MBEDTLS_UNPACKLIBDIR)/memory_buffer_alloc.c \
+    $(MBEDTLS_UNPACKLIBDIR)/nist_kw.c \
+    $(MBEDTLS_UNPACKLIBDIR)/oid.c \
+    $(MBEDTLS_UNPACKLIBDIR)/padlock.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pem.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pk.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pk_wrap.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkcs12.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkcs5.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkparse.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkwrite.c \
+    $(MBEDTLS_UNPACKLIBDIR)/platform.c \
+    $(MBEDTLS_UNPACKLIBDIR)/platform_util.c \
+    $(MBEDTLS_UNPACKLIBDIR)/poly1305.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_driver_wrappers.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_se.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_slot_management.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_storage.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_its_file.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ripemd160.c \
+    $(MBEDTLS_UNPACKLIBDIR)/rsa.c \
+    $(MBEDTLS_UNPACKLIBDIR)/rsa_internal.c \
+    $(MBEDTLS_UNPACKLIBDIR)/sha1.c \
+    $(MBEDTLS_UNPACKLIBDIR)/sha256.c \
+    $(MBEDTLS_UNPACKLIBDIR)/sha512.c \
+    $(MBEDTLS_UNPACKLIBDIR)/threading.c \
+    $(MBEDTLS_UNPACKLIBDIR)/timing.c \
+    $(MBEDTLS_UNPACKLIBDIR)/version.c \
+    $(MBEDTLS_UNPACKLIBDIR)/version_features.c \
+    $(MBEDTLS_UNPACKLIBDIR)/xtea.c \

Review comment:
       @Ouss4 I modified this a bit to be closer to what you recommended, does this look correct to you?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r559987690



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,68 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+MBEDTLS_DIR = .
+MBEDTLS_DIR_NAME = mbedtls
+
+# Set up build configuration and environment
+
+WD := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKDIR =  $(WD)$(DELIM)$(MBEDTLS_UNPACKNAME)
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKDIR)$(DELIM)library
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += -D__unix__
+CFLAGS += -I$(MBEDTLS_UNPACKDIR)$(DELIM)include
+
+MBEDTLS_LIB_CSRCS := $(addprefix $(MBEDTLS_UNPACKLIBDIR)$(DELIM), $(notdir $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)))
+
+CSRCS += $(MBEDTLS_LIB_CSRCS)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)$(DELIM)$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)$(DELIM)$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)

Review comment:
       I think we still need it.
   If you remember we talked about it before, and depending on the `mv` implementation and the underlying file system, `mv` may or may not change the timestamps.  




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563470913



##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       > You cannot call `include $(APPDIR)/Application.mk` twice in the same make file which is they need to be in there own folders. But I don't think that is related to this path issue.
   
   but you can specify multiple apps in PROGNAME and MAINSRC like zmodem:
   https://github.com/apache/incubator-nuttx-apps/blob/master/system/zmodem/Makefile#L38
   https://github.com/apache/incubator-nuttx-apps/blob/master/system/zmodem/Makefile#L45
   
   > also careful with your paths.
   > `crypto/mbedtls/programs/test` does not exist. `crypto/mbedtls/mbedtls/programs/test` does, but that is inside the downloaded source.
   
   Yes, we can't put into the download folder, crypto/mbedtls/Makefile may a better location for apps too with the multiple apps trick.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r559717074



##########
File path: crypto/Kconfig
##########
@@ -0,0 +1,10 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+# This file is autogenerated, do not edit.
+#
+
+menu "Cryptography Support"
+source "/home/bashton/nuttx/wrk/apps/crypto/mbedtls/Kconfig"

Review comment:
       Added the gitignore since this is auto-generated.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r606919964



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,85 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}

Review comment:
       There are a couple changes that I would like to try to get up-streamed.  I can try to see if they are willing.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] gustavonihei commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-880926061


   @btashton It has been some months without any updates to this PR.
   Are you intending to resume it in the near future?
   If not, I'd propose to merge this PR in its current state, since the unfinished task is not critical for usage and may be delegated to a future PR.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-898616110


   @gustavonihei  yeah we also need to take care of this for some stuff that is already merged in as well, so I think it makes sense to do most of it together. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563498598



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,91 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}
+
+CSRCS = $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)/$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)/$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)
+
+context:: $(MBEDTLS_UNPACKNAME)
+
+distclean::
+	$(call DELDIR, $(MBEDTLS_UNPACKNAME))
+	$(call DELFILE, $(MBEDTLS_ZIP))
+
+# Configuration Applications
+
+ifneq ($(CONFIG_MBEDTLS_APPS),)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls/

Review comment:
       Since we already include Application.mk at the end do we even need to include this here since they will already be registered?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r559428425



##########
File path: examples/mbedtls/Makefile
##########
@@ -0,0 +1,34 @@
+############################################################################
+# apps/examples/mbedtls/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# mbedtls built-in application info
+
+PROGNAME  = $(CONFIG_EXAMPLES_MBEDTLS_PROGNAME)
+PRIORITY  = $(CONFIG_EXAMPLES_MBEDTLS_PRIORITY)
+STACKSIZE = $(CONFIG_EXAMPLES_MBEDTLS_STACKSIZE)
+MODULE    = $(CONFIG_EXAMPLES_MBEDTLS)
+
+# Mbed TLS Demo
+
+MAINSRC = mbedtls_main.c

Review comment:
       For an example, an alternative is to use one of those comming with mbedTLS, I've got this:
   
   `MAINSRC = $(APPDIR)/external/mbedtls/mbedtls/programs/ssl/ssl_server.c`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] gustavonihei commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-961087558


   @xiaoxiang781216 @Ouss4 This PR has been stalled for way too long, let's merge it and deal with the improvements in separate PRs.
   Also we'll get the paperwork ready for the next release (post-10.2).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] acassis merged pull request #566: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
acassis merged pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] acassis commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-882552606


   Thank you @btashton, please let me know how we can help you to complete it. Our new Wi-Fi library will not include the wpa_supplicant internally and we need to provide mbedTLS to be able to use it. Also the MCUBoot port that @gustavonihei is doing will benefit from using mbedTLS.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] yamt commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r606178090



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,85 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}

Review comment:
       our local port of mbedtls has:
   ```
   # Tell mbedtls that NuttX has socklen_t.
   # Note: _SOCKLEN_T is what macOS defines with socklen_t.
   CFLAGS += -D_SOCKLEN_T
   ```
   i guess this version needs it too.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r559655012



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,133 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+MBEDTLS_DIR = .
+MBEDTLS_DIR_NAME = mbedtls
+
+# Set up build configuration and environment
+
+WD := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -o
+
+MBEDTLS_UNPACKDIR =  $(WD)/$(MBEDTLS_UNPACKNAME)
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKDIR)/library
+
+# Help
+CFLAGS += -D__unix__
+CFLAGS += -I$(MBEDTLS_UNPACKDIR)/include
+
+# Crypto Source
+CSRCS += \
+    $(MBEDTLS_UNPACKLIBDIR)/aesni.c \
+    $(MBEDTLS_UNPACKLIBDIR)/arc4.c \
+    $(MBEDTLS_UNPACKLIBDIR)/aria.c \
+    $(MBEDTLS_UNPACKLIBDIR)/asn1parse.c \
+    $(MBEDTLS_UNPACKLIBDIR)/asn1write.c \
+    $(MBEDTLS_UNPACKLIBDIR)/base64.c \
+    $(MBEDTLS_UNPACKLIBDIR)/bignum.c \
+    $(MBEDTLS_UNPACKLIBDIR)/blowfish.c \
+    $(MBEDTLS_UNPACKLIBDIR)/camellia.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ccm.c \
+    $(MBEDTLS_UNPACKLIBDIR)/chacha20.c \
+    $(MBEDTLS_UNPACKLIBDIR)/chachapoly.c \
+    $(MBEDTLS_UNPACKLIBDIR)/cipher.c \
+    $(MBEDTLS_UNPACKLIBDIR)/cipher_wrap.c \
+    $(MBEDTLS_UNPACKLIBDIR)/cmac.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ctr_drbg.c \
+    $(MBEDTLS_UNPACKLIBDIR)/des.c \
+    $(MBEDTLS_UNPACKLIBDIR)/dhm.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecdh.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecdsa.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecjpake.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecp.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecp_curves.c \
+    $(MBEDTLS_UNPACKLIBDIR)/entropy.c \
+    $(MBEDTLS_UNPACKLIBDIR)/entropy_poll.c \
+    $(MBEDTLS_UNPACKLIBDIR)/error.c \
+    $(MBEDTLS_UNPACKLIBDIR)/gcm.c \
+    $(MBEDTLS_UNPACKLIBDIR)/havege.c \
+    $(MBEDTLS_UNPACKLIBDIR)/hkdf.c \
+    $(MBEDTLS_UNPACKLIBDIR)/hmac_drbg.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md2.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md4.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md5.c \
+    $(MBEDTLS_UNPACKLIBDIR)/memory_buffer_alloc.c \
+    $(MBEDTLS_UNPACKLIBDIR)/nist_kw.c \
+    $(MBEDTLS_UNPACKLIBDIR)/oid.c \
+    $(MBEDTLS_UNPACKLIBDIR)/padlock.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pem.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pk.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pk_wrap.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkcs12.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkcs5.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkparse.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkwrite.c \
+    $(MBEDTLS_UNPACKLIBDIR)/platform.c \
+    $(MBEDTLS_UNPACKLIBDIR)/platform_util.c \
+    $(MBEDTLS_UNPACKLIBDIR)/poly1305.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_driver_wrappers.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_se.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_slot_management.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_storage.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_its_file.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ripemd160.c \
+    $(MBEDTLS_UNPACKLIBDIR)/rsa.c \
+    $(MBEDTLS_UNPACKLIBDIR)/rsa_internal.c \
+    $(MBEDTLS_UNPACKLIBDIR)/sha1.c \
+    $(MBEDTLS_UNPACKLIBDIR)/sha256.c \
+    $(MBEDTLS_UNPACKLIBDIR)/sha512.c \
+    $(MBEDTLS_UNPACKLIBDIR)/threading.c \
+    $(MBEDTLS_UNPACKLIBDIR)/timing.c \
+    $(MBEDTLS_UNPACKLIBDIR)/version.c \
+    $(MBEDTLS_UNPACKLIBDIR)/version_features.c \
+    $(MBEDTLS_UNPACKLIBDIR)/xtea.c \

Review comment:
       It's a subset of the files. Mbed TLS has three groups of files that are built that align with the libraries targets they create. I was not sure yet if that would be useful to keep.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563498598



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,91 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}
+
+CSRCS = $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)/$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)/$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)
+
+context:: $(MBEDTLS_UNPACKNAME)
+
+distclean::
+	$(call DELDIR, $(MBEDTLS_UNPACKNAME))
+	$(call DELFILE, $(MBEDTLS_ZIP))
+
+# Configuration Applications
+
+ifneq ($(CONFIG_MBEDTLS_APPS),)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls/

Review comment:
       Since we already include Application.mk at the end do we even need to include this at all since they will already be registered?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563456155



##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192
+
+source "$APPSDIR/crypto/mbedtls/apps/benchmark/Kconfig"
+source "$APPSDIR/crypto/mbedtls/apps/selftest/Kconfig"

Review comment:
       Ok, let's keep APPDIR




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] juniskane commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
juniskane commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r560853705



##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,18 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n

Review comment:
       Consider adding:
   
   `depends on DEV_URANDOM_RANDOM_POOL || DEV_URANDOM_ARCH`
   
   to safe-guard against naive user building this with unsuitable options (e.g. `DEV_URANDOM_CONGRUENTIAL` or `DEV_URANDOM_XORSHIFT128`)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563547420



##########
File path: crypto/mbedtls/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/crypto/mbedtls/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifeq ($(CONFIG_CRYPTO_MBEDTLS),y)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls
+
+# Allows `<mbedtls/<>.h>` import.
+
+CFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+CXXFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+
+CFLAGS += ${shell $(DEFINE) "$(CC)" MBEDTLS_CONFIG_FILE='"<crypto/mbedtls_config.h>"'}

Review comment:
       Do we need to completely replace the default config file and not just override/extend configs with `MBEDTLS_USER_CONFIG_FILE`?
   
   Just asking, either are fine by me.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563491496



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,91 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}
+
+CSRCS = $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)/$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)/$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)
+
+context:: $(MBEDTLS_UNPACKNAME)
+
+distclean::
+	$(call DELDIR, $(MBEDTLS_UNPACKNAME))
+	$(call DELFILE, $(MBEDTLS_ZIP))
+
+# Configuration Applications
+
+ifneq ($(CONFIG_MBEDTLS_APPS),)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls/
+
+PROGNAME =
+PRIORITY =
+STACKSIZE =
+MAINSRC =

Review comment:
       don't empty variable since it's default value. 

##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,91 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}
+
+CSRCS = $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)/$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)/$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)
+
+context:: $(MBEDTLS_UNPACKNAME)
+
+distclean::
+	$(call DELDIR, $(MBEDTLS_UNPACKNAME))
+	$(call DELFILE, $(MBEDTLS_ZIP))
+
+# Configuration Applications
+
+ifneq ($(CONFIG_MBEDTLS_APPS),)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls/

Review comment:
       CONFIGURED_APPS need define in Make.def, Makefile can't reference it if we define it here.

##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,80 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+menuconfig MBEDTLS_APPS

Review comment:
       normally, we reuse CRYPTO_MBEDTLS for module by changing it to tristate and update the if condition in Make.defs:
   ```
   ifneq ($(CONFIG_CRYPTO_MBEDTLS),)
   CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls
   endif
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] acassis commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
acassis commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r607226398



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,85 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}

Review comment:
       Yep, replacing ```__unix__``` with ```__NuttX__``` should be good!




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563681586



##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192

Review comment:
       Why not create PR for mbedtls to allocate 100KB from heap:)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563681586



##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192

Review comment:
       Why not create PRfor mbedtls to allocate 100KB from heap:)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-882539476


   @acassis this should _not_ be merged (or the other crypto PR I have) until the export control work for Apache is completed. I can try to start the process for that this week, depending on my time.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r559773598



##########
File path: crypto/Kconfig
##########
@@ -0,0 +1,10 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+# This file is autogenerated, do not edit.
+#
+
+menu "Cryptography Support"
+source "/home/bashton/nuttx/wrk/apps/crypto/mbedtls/Kconfig"

Review comment:
       Ah, right.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] fraviofii commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
fraviofii commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-826326814


   The libraries are not exported when we execute 'make export' for NuttX.
   
   This would be a nice future for this implementation.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563547420



##########
File path: crypto/mbedtls/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/crypto/mbedtls/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifeq ($(CONFIG_CRYPTO_MBEDTLS),y)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls
+
+# Allows `<mbedtls/<>.h>` import.
+
+CFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+CXXFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+
+CFLAGS += ${shell $(DEFINE) "$(CC)" MBEDTLS_CONFIG_FILE='"<crypto/mbedtls_config.h>"'}

Review comment:
       Do we need to completely replace the default config file and not just override/extend configs with `MBEDTLS_USER_CONFIG_FILE`?
   
   Just asking, either are fine by me.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] acassis commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-882552606


   Thank you @btashton, please let me know how we can help you to complete it. Our new Wi-Fi library will not include the wpa_supplicant internally and we need to provide mbedTLS to be able to use it. Also the MCUBoot port that @gustavonihei is doing will benefit from using mbedTLS.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r559775005



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,133 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+MBEDTLS_DIR = .
+MBEDTLS_DIR_NAME = mbedtls
+
+# Set up build configuration and environment
+
+WD := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -o
+
+MBEDTLS_UNPACKDIR =  $(WD)/$(MBEDTLS_UNPACKNAME)
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKDIR)/library
+
+# Help
+CFLAGS += -D__unix__
+CFLAGS += -I$(MBEDTLS_UNPACKDIR)/include
+
+# Crypto Source
+CSRCS += \
+    $(MBEDTLS_UNPACKLIBDIR)/aesni.c \
+    $(MBEDTLS_UNPACKLIBDIR)/arc4.c \
+    $(MBEDTLS_UNPACKLIBDIR)/aria.c \
+    $(MBEDTLS_UNPACKLIBDIR)/asn1parse.c \
+    $(MBEDTLS_UNPACKLIBDIR)/asn1write.c \
+    $(MBEDTLS_UNPACKLIBDIR)/base64.c \
+    $(MBEDTLS_UNPACKLIBDIR)/bignum.c \
+    $(MBEDTLS_UNPACKLIBDIR)/blowfish.c \
+    $(MBEDTLS_UNPACKLIBDIR)/camellia.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ccm.c \
+    $(MBEDTLS_UNPACKLIBDIR)/chacha20.c \
+    $(MBEDTLS_UNPACKLIBDIR)/chachapoly.c \
+    $(MBEDTLS_UNPACKLIBDIR)/cipher.c \
+    $(MBEDTLS_UNPACKLIBDIR)/cipher_wrap.c \
+    $(MBEDTLS_UNPACKLIBDIR)/cmac.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ctr_drbg.c \
+    $(MBEDTLS_UNPACKLIBDIR)/des.c \
+    $(MBEDTLS_UNPACKLIBDIR)/dhm.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecdh.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecdsa.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecjpake.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecp.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecp_curves.c \
+    $(MBEDTLS_UNPACKLIBDIR)/entropy.c \
+    $(MBEDTLS_UNPACKLIBDIR)/entropy_poll.c \
+    $(MBEDTLS_UNPACKLIBDIR)/error.c \
+    $(MBEDTLS_UNPACKLIBDIR)/gcm.c \
+    $(MBEDTLS_UNPACKLIBDIR)/havege.c \
+    $(MBEDTLS_UNPACKLIBDIR)/hkdf.c \
+    $(MBEDTLS_UNPACKLIBDIR)/hmac_drbg.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md2.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md4.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md5.c \
+    $(MBEDTLS_UNPACKLIBDIR)/memory_buffer_alloc.c \
+    $(MBEDTLS_UNPACKLIBDIR)/nist_kw.c \
+    $(MBEDTLS_UNPACKLIBDIR)/oid.c \
+    $(MBEDTLS_UNPACKLIBDIR)/padlock.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pem.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pk.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pk_wrap.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkcs12.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkcs5.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkparse.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkwrite.c \
+    $(MBEDTLS_UNPACKLIBDIR)/platform.c \
+    $(MBEDTLS_UNPACKLIBDIR)/platform_util.c \
+    $(MBEDTLS_UNPACKLIBDIR)/poly1305.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_driver_wrappers.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_se.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_slot_management.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_storage.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_its_file.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ripemd160.c \
+    $(MBEDTLS_UNPACKLIBDIR)/rsa.c \
+    $(MBEDTLS_UNPACKLIBDIR)/rsa_internal.c \
+    $(MBEDTLS_UNPACKLIBDIR)/sha1.c \
+    $(MBEDTLS_UNPACKLIBDIR)/sha256.c \
+    $(MBEDTLS_UNPACKLIBDIR)/sha512.c \
+    $(MBEDTLS_UNPACKLIBDIR)/threading.c \
+    $(MBEDTLS_UNPACKLIBDIR)/timing.c \
+    $(MBEDTLS_UNPACKLIBDIR)/version.c \
+    $(MBEDTLS_UNPACKLIBDIR)/version_features.c \
+    $(MBEDTLS_UNPACKLIBDIR)/xtea.c \

Review comment:
       yes, that should bring everything into the build.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] gustavonihei edited a comment on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
gustavonihei edited a comment on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-898611028


   @btashton Just to see if we're aligned on the remaining tasks for this paperwork. According to the link provided by @justinmclean, there are 4 steps that the Apache guide requires to be done:
   
   **[Check the Export Control Classification Number (ECCN)](https://infra.apache.org/crypto.html#classify)**
   **[DONE]** Basically there is no work on this. MbedTLS is already used by several Apache projects, including MyNewt, so we can rely on the pre-executed assessment.
   
   **[Update the Exports Page with Source Links](https://infra.apache.org/crypto.html#sources)**
   **[Pending]** I believe we need the support from @justinmclean for fulfilling this item. We just need an entry for `APACHE NUTTX (INCUBATING) PROJECT` on [this page](https://www.apache.org/licenses/exports/) like this:
   ### APACHE NUTTX (INCUBATING) PROJECT
   - Apache NuttX
     - development / 5D002 / [Mbed TLS](https://www.trustedfirmware.org/projects/mbed-tls/)
   
   **[Notify the U.S. Government of the release](https://infra.apache.org/crypto.html#notify)**
   **[Pending]** Maybe the most complicated of all the tasks, but not that complicated at all. We need to fill the email template containing a brief description about MbedTLS or use the script from the Apache SVN for generating it. We may help on this one, Espressif has already gone through this process for enabling the integration of MbedTLS on ESP-IDF. Let us know if you need any support.
   
   **[Inform users by including a crypto notice in the distribution's README file](https://infra.apache.org/crypto.html#inform)**
   **[DONE]** We may simply copy the note from [Apache MyNewt](https://github.com/apache/mynewt-core#export-restrictions). See https://github.com/btashton/incubator-nuttx-apps/pull/1


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563452190



##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192
+
+source "$APPSDIR/crypto/mbedtls/apps/benchmark/Kconfig"
+source "$APPSDIR/crypto/mbedtls/apps/selftest/Kconfig"

Review comment:
       The Kconfigs do not use relative paths.  If you try you get an error like this:
   ```
   /home/bashton/nuttx/wrk/apps/crypto/mbedtls/Kconfig:23: can't open file "./apps/selftest/Kconfig"
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563470913



##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       > You cannot call `include $(APPDIR)/Application.mk` twice in the same make file which is they need to be in there own folders. But I don't think that is related to this path issue.
   
   but you can specify multiple apps in PROGNAME and MAINSRC like zmodem:
   https://github.com/apache/incubator-nuttx-apps/blob/master/system/zmodem/Makefile#L38
   https://github.com/apache/incubator-nuttx-apps/blob/master/system/zmodem/Makefile#L45
   
   > also careful with your paths.
   > `crypto/mbedtls/programs/test` does not exist. `crypto/mbedtls/mbedtls/programs/test` does, but that is inside the downloaded source.
   
   Yes, we can't put into the download folder, crypto/mbedtls/Makefile may a better location for apps too.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] gustavonihei commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-960965308


   @btashton I've force pushed into your `mbedtls` branch to solve the nxstyle issue. The CI should pass now.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] gustavonihei commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-961289548


   Created https://github.com/apache/incubator-nuttx-apps/issues/877 for addressing the remaining task.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r559428425



##########
File path: examples/mbedtls/Makefile
##########
@@ -0,0 +1,34 @@
+############################################################################
+# apps/examples/mbedtls/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# mbedtls built-in application info
+
+PROGNAME  = $(CONFIG_EXAMPLES_MBEDTLS_PROGNAME)
+PRIORITY  = $(CONFIG_EXAMPLES_MBEDTLS_PRIORITY)
+STACKSIZE = $(CONFIG_EXAMPLES_MBEDTLS_STACKSIZE)
+MODULE    = $(CONFIG_EXAMPLES_MBEDTLS)
+
+# Mbed TLS Demo
+
+MAINSRC = mbedtls_main.c

Review comment:
       For an example, an alternative is to use the one of those comming with mbedTLS, I've got this:
   
   `MAINSRC = $(APPDIR)/external/mbedtls/mbedtls/programs/ssl/ssl_server.c`

##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,133 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+MBEDTLS_DIR = .
+MBEDTLS_DIR_NAME = mbedtls
+
+# Set up build configuration and environment
+
+WD := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -o
+
+MBEDTLS_UNPACKDIR =  $(WD)/$(MBEDTLS_UNPACKNAME)
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKDIR)/library
+
+# Help
+CFLAGS += -D__unix__
+CFLAGS += -I$(MBEDTLS_UNPACKDIR)/include
+
+# Crypto Source
+CSRCS += \
+    $(MBEDTLS_UNPACKLIBDIR)/aesni.c \
+    $(MBEDTLS_UNPACKLIBDIR)/arc4.c \
+    $(MBEDTLS_UNPACKLIBDIR)/aria.c \
+    $(MBEDTLS_UNPACKLIBDIR)/asn1parse.c \
+    $(MBEDTLS_UNPACKLIBDIR)/asn1write.c \
+    $(MBEDTLS_UNPACKLIBDIR)/base64.c \
+    $(MBEDTLS_UNPACKLIBDIR)/bignum.c \
+    $(MBEDTLS_UNPACKLIBDIR)/blowfish.c \
+    $(MBEDTLS_UNPACKLIBDIR)/camellia.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ccm.c \
+    $(MBEDTLS_UNPACKLIBDIR)/chacha20.c \
+    $(MBEDTLS_UNPACKLIBDIR)/chachapoly.c \
+    $(MBEDTLS_UNPACKLIBDIR)/cipher.c \
+    $(MBEDTLS_UNPACKLIBDIR)/cipher_wrap.c \
+    $(MBEDTLS_UNPACKLIBDIR)/cmac.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ctr_drbg.c \
+    $(MBEDTLS_UNPACKLIBDIR)/des.c \
+    $(MBEDTLS_UNPACKLIBDIR)/dhm.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecdh.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecdsa.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecjpake.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecp.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecp_curves.c \
+    $(MBEDTLS_UNPACKLIBDIR)/entropy.c \
+    $(MBEDTLS_UNPACKLIBDIR)/entropy_poll.c \
+    $(MBEDTLS_UNPACKLIBDIR)/error.c \
+    $(MBEDTLS_UNPACKLIBDIR)/gcm.c \
+    $(MBEDTLS_UNPACKLIBDIR)/havege.c \
+    $(MBEDTLS_UNPACKLIBDIR)/hkdf.c \
+    $(MBEDTLS_UNPACKLIBDIR)/hmac_drbg.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md2.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md4.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md5.c \
+    $(MBEDTLS_UNPACKLIBDIR)/memory_buffer_alloc.c \
+    $(MBEDTLS_UNPACKLIBDIR)/nist_kw.c \
+    $(MBEDTLS_UNPACKLIBDIR)/oid.c \
+    $(MBEDTLS_UNPACKLIBDIR)/padlock.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pem.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pk.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pk_wrap.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkcs12.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkcs5.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkparse.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkwrite.c \
+    $(MBEDTLS_UNPACKLIBDIR)/platform.c \
+    $(MBEDTLS_UNPACKLIBDIR)/platform_util.c \
+    $(MBEDTLS_UNPACKLIBDIR)/poly1305.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_driver_wrappers.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_se.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_slot_management.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_storage.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_its_file.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ripemd160.c \
+    $(MBEDTLS_UNPACKLIBDIR)/rsa.c \
+    $(MBEDTLS_UNPACKLIBDIR)/rsa_internal.c \
+    $(MBEDTLS_UNPACKLIBDIR)/sha1.c \
+    $(MBEDTLS_UNPACKLIBDIR)/sha256.c \
+    $(MBEDTLS_UNPACKLIBDIR)/sha512.c \
+    $(MBEDTLS_UNPACKLIBDIR)/threading.c \
+    $(MBEDTLS_UNPACKLIBDIR)/timing.c \
+    $(MBEDTLS_UNPACKLIBDIR)/version.c \
+    $(MBEDTLS_UNPACKLIBDIR)/version_features.c \
+    $(MBEDTLS_UNPACKLIBDIR)/xtea.c \

Review comment:
       These are all the files?  are you planning the select some using Kconfig?
   Otherwise we can grab all of them at once with `CSRCS := $(notdir $(wildcard $(MBEDTLS_SRCDIR)$(DELIM)*.c))`

##########
File path: crypto/Kconfig
##########
@@ -0,0 +1,10 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+# This file is autogenerated, do not edit.
+#
+
+menu "Cryptography Support"
+source "/home/bashton/nuttx/wrk/apps/crypto/mbedtls/Kconfig"

Review comment:
       A reminder to change the path here.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563450371



##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192

Review comment:
       The issue is the default stack size is only 2KB but I normally see 3-4KB on real hardware.  I get that the sim is probably much higher so if we kept this I could conditional this like the DEFAULT_TASK_STACKSIZE.  Thoughts?

##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192
+
+source "$APPSDIR/crypto/mbedtls/apps/benchmark/Kconfig"
+source "$APPSDIR/crypto/mbedtls/apps/selftest/Kconfig"

Review comment:
       The Kconfigs do not use relative paths.  If you try you get an error like this:
   ```
   /home/bashton/nuttx/wrk/apps/crypto/mbedtls/Kconfig:23: can't open file "./apps/selftest/Kconfig"
   ```

##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       I could not get that to work, but maybe I am doing something wrong
   with:
   ```
   MAINSRC = ../../mbedtls/test/selftest.c
   ```
   I get this
   ```
   make[3]: *** No rule to make target '../../mbedtls/test/selftest.c', needed by '../../mbedtls/test/selftest.home.bashton.nuttx.wrk.apps.crypto.mbedtls.apps.selftest.o'.  Stop.
   ```

##########
File path: crypto/mbedtls/Make.defs
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifeq ($(CONFIG_CRYPTO_MBEDTLS),y)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls
+
+# Allows `<mbedtls/<>.h>` import.
+
+CFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+CXXFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+
+CFLAGS += -DMBEDTLS_CONFIG_FILE="<crypto/mbedtls_config.h>"

Review comment:
       Had to wrap the string in single quotes as well to get the command formatted correctly but seems good now.

##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       You cannot call `include $(APPDIR)/Application.mk` twice in the same make file which is they need to be in there own folders.  But I don't think that is related to this path issue.

##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       also careful with your paths.
   `crypto/mbedtls/programs/test` does not exist. `crypto/mbedtls/mbedtls/programs/test` does, but that is inside the downloaded source.

##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       Hmm interesting I did not realize that.  Can you take a look at how I set it up now.  MODULE only take a single value so I had to create a wrapper for all the apps to drive that, but I think it is reasonable.

##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,91 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}
+
+CSRCS = $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)/$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)/$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)
+
+context:: $(MBEDTLS_UNPACKNAME)
+
+distclean::
+	$(call DELDIR, $(MBEDTLS_UNPACKNAME))
+	$(call DELFILE, $(MBEDTLS_ZIP))
+
+# Configuration Applications
+
+ifneq ($(CONFIG_MBEDTLS_APPS),)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls/

Review comment:
       Since we already include Application.mk at the end do we even need to include this here since they will already be registered?

##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,91 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}
+
+CSRCS = $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)/$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)/$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)
+
+context:: $(MBEDTLS_UNPACKNAME)
+
+distclean::
+	$(call DELDIR, $(MBEDTLS_UNPACKNAME))
+	$(call DELFILE, $(MBEDTLS_ZIP))
+
+# Configuration Applications
+
+ifneq ($(CONFIG_MBEDTLS_APPS),)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls/

Review comment:
       Since we already include Application.mk at the end do we even need to include this at all since they will already be registered?

##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,80 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+menuconfig MBEDTLS_APPS

Review comment:
       This seems wrong. The applications and the library are two separate things. Enabling the library should have no impact (besides dependency) on the applications. 

##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192

Review comment:
       So what are you thinking is the right thing for some of these apps?  DEFAULT_TASK_STACKSIZE wont work for most of them.  The an especially bad offender is the app for testing parsing CRL it allocates a 100KB buffer on the stack right away (I don't know why they don't use the heap for this...)
   https://github.com/ARMmbed/mbedtls/blob/6fbff5b557efe661cc019ef59f42c835524e9bf2/programs/x509/crl_app.c#L75
   

##########
File path: crypto/mbedtls/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/crypto/mbedtls/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifeq ($(CONFIG_CRYPTO_MBEDTLS),y)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls
+
+# Allows `<mbedtls/<>.h>` import.
+
+CFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+CXXFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+
+CFLAGS += ${shell $(DEFINE) "$(CC)" MBEDTLS_CONFIG_FILE='"<crypto/mbedtls_config.h>"'}

Review comment:
       My plan is to completely replace it so that it is fully hooked into the Kconfig, I think that will end up being cleaner for us.

##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,91 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}
+
+CSRCS = $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)/$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)/$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)
+
+context:: $(MBEDTLS_UNPACKNAME)
+
+distclean::
+	$(call DELDIR, $(MBEDTLS_UNPACKNAME))
+	$(call DELFILE, $(MBEDTLS_ZIP))
+
+# Configuration Applications
+
+ifneq ($(CONFIG_MBEDTLS_APPS),)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls/

Review comment:
       Ok, done.

##########
File path: crypto/mbedtls/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/crypto/mbedtls/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifeq ($(CONFIG_CRYPTO_MBEDTLS),y)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls
+
+# Allows `<mbedtls/<>.h>` import.
+
+CFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+CXXFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+
+CFLAGS += ${shell $(DEFINE) "$(CC)" MBEDTLS_CONFIG_FILE='"<crypto/mbedtls_config.h>"'}

Review comment:
       Another thing I have not figured out how to do is how to get make to be aware that all the source files in here depend on `crypto/mbedtls_config.h`  right now when I make change to it or to `nuttx/config.h` that drives changes it in they are not rebuilt.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] acassis commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
acassis commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r607226398



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,85 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}

Review comment:
       Yep, replacing "__unix__" with "__NuttX__" should be good!




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-766507563


   I still want to wire up more of the configuration into Kconfig but I would appreciate a quick review if how I have wired this up makes sense.  I am able to run the two test applications on my nrf52-feather board
   
   ```
   NuttShell (NSH) NuttX-10.0.1
   nsh> uname -a
   NuttX 10.0.1 56ef94086f Jan 24 2021 18:08:43 arm nrf52-feather
   nsh> ?
   help usage:  help [-v] [<cmd>]
   
     .         cd        echo      hexdump   mkrd      pwd       test      usleep    
     [         cp        exec      kill      mh        rm        time      xd        
     ?         cmp       exit      ls        mount     rmdir     true      
     basename  dirname   false     mb        mv        set       uname     
     break     dd        free      mkdir     mw        sleep     umount    
     cat       df        help      mkfatfs   ps        source    unset     
   
   Builtin Apps:
     mbedbenchmark  mbedselftest   nsh            sh             
   nsh> mbedselftest
   
     CALLOC(0): passed (NULL)
     CALLOC(1): passed
     CALLOC(1 again): passed (same address)
   
     MD5 test #1: passed
     MD5 test #2: passed
     MD5 test #3: passed
     MD5 test #4: passed
     MD5 test #5: passed
     MD5 test #6: passed
     MD5 test #7: passed
   
     SHA-1 test #1: passed
     SHA-1 test #2: passed
     SHA-1 test #3: passed
   
     SHA-224 test #1: passed
     SHA-224 test #2: passed
     SHA-224 test #3: passed
     SHA-256 test #1: passed
     SHA-256 test #2: passed
     SHA-256 test #3: passed
   
     DES -ECB- 56 (dec): passed
     DES -ECB- 56 (enc): passed
     DES3-ECB-112 (dec): passed
     DES3-ECB-112 (enc): passed
     DES3-ECB-168 (dec): passed
     DES3-ECB-168 (enc): passed
   
     DES -CBC- 56 (dec): passed
     DES -CBC- 56 (enc): passed
     DES3-CBC-112 (dec): passed
     DES3-CBC-112 (enc): passed
     DES3-CBC-168 (dec): passed
     DES3-CBC-168 (enc): passed
   
     AES-ECB-128 (dec): passed
     AES-ECB-128 (enc): passed
     AES-ECB-192 (dec): passed
     AES-ECB-192 (enc): passed
     AES-ECB-256 (dec): passed
     AES-ECB-256 (enc): passed
   
     AES-CBC-128 (dec): passed
     AES-CBC-128 (enc): passed
     AES-CBC-192 (dec): passed
     AES-CBC-192 (enc): passed
     AES-CBC-256 (dec): passed
     AES-CBC-256 (enc): passed
   
     Base64 encoding test: passed
     Base64 decoding test: passed
   
     MPI test #1 (mul_mpi): passed
     MPI test #2 (div_mpi): passed
     MPI test #3 (exp_mod): passed
     MPI test #4 (inv_mod): passed
     MPI test #5 (simple gcd): passed
   
     RSA key validation: passed
     PKCS#1 encryption : passed
     PKCS#1 decryption : passed
     PKCS#1 data sign  : passed
     PKCS#1 sig. verify: passed
   
     X.509 certificate load: passed
     X.509 signature verify: passed
   
     CTR_DRBG (PR = TRUE) : passed
     CTR_DRBG (PR = FALSE): passed
   
     ENTROPY test: passed
   
     TIMING tests note: will take some time!
     TIMING test #1 (set_alarm / get_timer): passed
     TIMING test #2 (set/get_delay        ): passed
     TIMING test #3 (hardclock / get_timer): failed (ignored)
   
     Executed 13 test suites
   
     [ All tests PASS ]
   
   nsh> mbedbenchmark
   
     MD5                      :       1014 KiB/s,          0 cycles/byte
     SHA-1                    :        694 KiB/s,          1 cycles/byte
     SHA-256                  :        185 KiB/s,          5 cycles/byte
     3DES                     :         65 KiB/s,         15 cycles/byte
     DES                      :        161 KiB/s,          6 cycles/byte
     AES-CBC-128              :        279 KiB/s,          3 cycles/byte
     AES-CBC-192              :        254 KiB/s,          3 cycles/byte
     AES-CBC-256              :        231 KiB/s,          4 cycles/byte
     CTR_DRBG (NOPR)          :        237 KiB/s,          4 cycles/byte
     CTR_DRBG (PR)            :        162 KiB/s,          6 cycles/byte
    
   ``` 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] acassis merged pull request #566: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
acassis merged pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] acassis merged pull request #566: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
acassis merged pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563685502



##########
File path: crypto/mbedtls/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/crypto/mbedtls/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifeq ($(CONFIG_CRYPTO_MBEDTLS),y)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls
+
+# Allows `<mbedtls/<>.h>` import.
+
+CFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+CXXFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+
+CFLAGS += ${shell $(DEFINE) "$(CC)" MBEDTLS_CONFIG_FILE='"<crypto/mbedtls_config.h>"'}

Review comment:
       It's the known build system issue, @anchao has the patch to fix it, please wait a moment.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563499356



##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,80 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+menuconfig MBEDTLS_APPS

Review comment:
       This seems wrong. The applications and the library are two separate things. Enabling the library should have no impact (besides dependency) on the applications. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] gustavonihei commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-898611028


   @btashton Just to see if we're aligned on the remaining tasks for this paperwork. There are 4 steps that the Apache guide requires to be done:
   
   **[Check the Export Control Classification Number (ECCN)](https://infra.apache.org/crypto.html#classify)**
   **[DONE]** Basically there is no work on this. MbedTLS is already used by several Apache projects, including MyNewt, so we can rely on the pre-executed assessment.
   
   **[Update the Exports Page with Source Links](https://infra.apache.org/crypto.html#sources)**
   **[Pending]** I believe we need the support from @justinmclean for fulfilling this item. We just need an entry for `APACHE NUTTX (INCUBATING) PROJECT` on [this page](https://www.apache.org/licenses/exports/) like this:
   ### APACHE NUTTX (INCUBATING) PROJECT
   - Apache NuttX
     - development / 5D002 / [ARM MbedTLS](https://www.trustedfirmware.org/projects/mbed-tls/)
   
   **[Notify the U.S. Government of the release](https://infra.apache.org/crypto.html#notify)**
   **[Pending]** Maybe the most complicated of all the tasks, but not that complicated at all. We need to fill the email template containing a brief description about MbedTLS or use the script from the Apache SVN for generating it. We may help on this one, Espressif has already gone through this process for enabling the integration of MbedTLS on ESP-IDF. Let us know if you need any support.
   
   **[Inform users by including a crypto notice in the distribution's README file](https://infra.apache.org/crypto.html#inform)**
   **[DONE]** We may simply copy the note from [Apache MyNewt](https://github.com/apache/mynewt-core#export-restrictions). See https://github.com/btashton/incubator-nuttx-apps/pull/1


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563510975



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,91 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}
+
+CSRCS = $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)/$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)/$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)
+
+context:: $(MBEDTLS_UNPACKNAME)
+
+distclean::
+	$(call DELDIR, $(MBEDTLS_UNPACKNAME))
+	$(call DELFILE, $(MBEDTLS_ZIP))
+
+# Configuration Applications
+
+ifneq ($(CONFIG_MBEDTLS_APPS),)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls/

Review comment:
       We don't need define CONFIGURED_APPS in Makefile, it's enough to define CONFIGURED_APPS in Make.defs




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-882539476


   @acassis this should _not_ be merged (or the other crypto PR I have) until the export control work for Apache is completed. I can try to start the process for that this week, depending on my time.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563533366



##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192

Review comment:
       So what are you thinking is the right thing for some of these apps?  DEFAULT_TASK_STACKSIZE wont work for most of them.  The an especially bad offender is the app for testing parsing CRL it allocates a 100KB buffer on the stack right away (I don't know why they don't use the heap for this...)
   https://github.com/ARMmbed/mbedtls/blob/6fbff5b557efe661cc019ef59f42c835524e9bf2/programs/x509/crl_app.c#L75
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] gustavonihei commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-961289548


   Created https://github.com/apache/incubator-nuttx-apps/issues/877 for addressing the remaining task.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563485565



##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       Hmm interesting I did not realize that.  Can you take a look at how I set it up now.  MODULE only take a single value so I had to create a wrapper for all the apps to drive that, but I think it is reasonable.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-882539476


   @acassis this should _not_ be merged (or the other crypto PR I have) until the export control work for Apache is completed. I can try to start the process for that this week, depending on my time.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563443006



##########
File path: crypto/mbedtls/Make.defs
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifeq ($(CONFIG_CRYPTO_MBEDTLS),y)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls
+
+# Allows `<mbedtls/<>.h>` import.
+
+CFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+CXXFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+
+CFLAGS += -DMBEDTLS_CONFIG_FILE="<crypto/mbedtls_config.h>"

Review comment:
       should we use $(DEIFNE) like $(INCDIR)?

##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192
+
+source "$APPSDIR/crypto/mbedtls/apps/benchmark/Kconfig"
+source "$APPSDIR/crypto/mbedtls/apps/selftest/Kconfig"

Review comment:
       can we remove $APPSDIR/? since other location uses the relative path.

##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,61 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+WD := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}

Review comment:
       remove WD, nobody use it now.

##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192

Review comment:
       can we use the small stack size or DEFAULT_TASK_STACKSIZE?

##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       why not use the relative path? So we can remove MBEDPROGDIR and  crypto/mbedtls/apps/Makefile.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563460021



##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       I know your problem, both selftest.c and benchmark.c put in crypto/mbedtls/programs/test, but you put Makefile under:
   crypto/mbedtls/apps/selftest/Makefile
   crypto/mbedtls/apps/benchmark/Makefile
   Why not put Makfile to:
   crypto/mbedtls/programs
   and merge two apps into one Mafile?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r560895786



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,68 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+MBEDTLS_DIR = .
+MBEDTLS_DIR_NAME = mbedtls
+
+# Set up build configuration and environment
+
+WD := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKDIR =  $(WD)$(DELIM)$(MBEDTLS_UNPACKNAME)
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKDIR)$(DELIM)library
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += -D__unix__
+CFLAGS += -I$(MBEDTLS_UNPACKDIR)$(DELIM)include
+
+MBEDTLS_LIB_CSRCS := $(addprefix $(MBEDTLS_UNPACKLIBDIR)$(DELIM), $(notdir $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)))
+
+CSRCS += $(MBEDTLS_LIB_CSRCS)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)$(DELIM)$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)$(DELIM)$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)

Review comment:
       Yes, you are right.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-766507563


   I still want to wire up more of the configuration into Kconfig but I would appreciate a quick review if how I have wired this up makes sense.  I am able to run the two test applications on my nrf52-feather board
   
   ```
   NuttShell (NSH) NuttX-10.0.1
   nsh> uname -a
   NuttX 10.0.1 56ef94086f Jan 24 2021 18:08:43 arm nrf52-feather
   nsh> ?
   help usage:  help [-v] [<cmd>]
   
     .         cd        echo      hexdump   mkrd      pwd       test      usleep    
     [         cp        exec      kill      mh        rm        time      xd        
     ?         cmp       exit      ls        mount     rmdir     true      
     basename  dirname   false     mb        mv        set       uname     
     break     dd        free      mkdir     mw        sleep     umount    
     cat       df        help      mkfatfs   ps        source    unset     
   
   Builtin Apps:
     mbedbenchmark  mbedselftest   nsh            sh             
   nsh> mbedselftest
   
     CALLOC(0): passed (NULL)
     CALLOC(1): passed
     CALLOC(1 again): passed (same address)
   
     MD5 test #1: passed
     MD5 test #2: passed
     MD5 test #3: passed
     MD5 test #4: passed
     MD5 test #5: passed
     MD5 test #6: passed
     MD5 test #7: passed
   
     SHA-1 test #1: passed
     SHA-1 test #2: passed
     SHA-1 test #3: passed
   
     SHA-224 test #1: passed
     SHA-224 test #2: passed
     SHA-224 test #3: passed
     SHA-256 test #1: passed
     SHA-256 test #2: passed
     SHA-256 test #3: passed
   
     DES -ECB- 56 (dec): passed
     DES -ECB- 56 (enc): passed
     DES3-ECB-112 (dec): passed
     DES3-ECB-112 (enc): passed
     DES3-ECB-168 (dec): passed
     DES3-ECB-168 (enc): passed
   
     DES -CBC- 56 (dec): passed
     DES -CBC- 56 (enc): passed
     DES3-CBC-112 (dec): passed
     DES3-CBC-112 (enc): passed
     DES3-CBC-168 (dec): passed
     DES3-CBC-168 (enc): passed
   
     AES-ECB-128 (dec): passed
     AES-ECB-128 (enc): passed
     AES-ECB-192 (dec): passed
     AES-ECB-192 (enc): passed
     AES-ECB-256 (dec): passed
     AES-ECB-256 (enc): passed
   
     AES-CBC-128 (dec): passed
     AES-CBC-128 (enc): passed
     AES-CBC-192 (dec): passed
     AES-CBC-192 (enc): passed
     AES-CBC-256 (dec): passed
     AES-CBC-256 (enc): passed
   
     Base64 encoding test: passed
     Base64 decoding test: passed
   
     MPI test #1 (mul_mpi): passed
     MPI test #2 (div_mpi): passed
     MPI test #3 (exp_mod): passed
     MPI test #4 (inv_mod): passed
     MPI test #5 (simple gcd): passed
   
     RSA key validation: passed
     PKCS#1 encryption : passed
     PKCS#1 decryption : passed
     PKCS#1 data sign  : passed
     PKCS#1 sig. verify: passed
   
     X.509 certificate load: passed
     X.509 signature verify: passed
   
     CTR_DRBG (PR = TRUE) : passed
     CTR_DRBG (PR = FALSE): passed
   
     ENTROPY test: passed
   
     TIMING tests note: will take some time!
     TIMING test #1 (set_alarm / get_timer): passed
     TIMING test #2 (set/get_delay        ): passed
     TIMING test #3 (hardclock / get_timer): failed (ignored)
   
     Executed 13 test suites
   
     [ All tests PASS ]
   
   nsh> mbedbenchmark
   
     MD5                      :       1014 KiB/s,          0 cycles/byte
     SHA-1                    :        694 KiB/s,          1 cycles/byte
     SHA-256                  :        185 KiB/s,          5 cycles/byte
     3DES                     :         65 KiB/s,         15 cycles/byte
     DES                      :        161 KiB/s,          6 cycles/byte
     AES-CBC-128              :        279 KiB/s,          3 cycles/byte
     AES-CBC-192              :        254 KiB/s,          3 cycles/byte
     AES-CBC-256              :        231 KiB/s,          4 cycles/byte
     CTR_DRBG (NOPR)          :        237 KiB/s,          4 cycles/byte
     CTR_DRBG (PR)            :        162 KiB/s,          6 cycles/byte
    
   ``` 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r606208537



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,85 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}

Review comment:
       Should we add _SOCKLEN_T to our socket.h header file?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563512239



##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192

Review comment:
       Yes, sim may need more stack size, DEFAULT_TASK_STACKSIZE has special value(64KB?) for sim.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] justinmclean commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
justinmclean commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-768671705


   Hi,
   
   Just a note on adding this Mbed is dual licensed under Apache 2.0 and GPL so license wise this is OK. But you may need to inform the US government that NuttX contains encryption software. See here for the process [1]
   
   Thanks,.
   Justin
   
   1. https://infra.apache.org/crypto.html


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563564668



##########
File path: crypto/mbedtls/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/crypto/mbedtls/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifeq ($(CONFIG_CRYPTO_MBEDTLS),y)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls
+
+# Allows `<mbedtls/<>.h>` import.
+
+CFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+CXXFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+
+CFLAGS += ${shell $(DEFINE) "$(CC)" MBEDTLS_CONFIG_FILE='"<crypto/mbedtls_config.h>"'}

Review comment:
       Another thing I have not figured out how to do is how to get make to be aware that all the source files in here depend on `crypto/mbedtls_config.h`  right now when I make change to it or to `nuttx/config.h` that drives changes it in they are not rebuilt.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] gustavonihei commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-898406877


   > @acassis this should _not_ be merged (or the other crypto PR I have) until the export control work for Apache is completed. I can try to start the process for that this week, depending on my time.
   
   @btashton Do you have any updates on this work? It has been almost another month without any report on this PR. In case you are unable to prioritize it, please let's work together for making it happen.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-902014889


   @btashton the conflict need resolve first.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] gustavonihei edited a comment on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
gustavonihei edited a comment on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-898611028


   @btashton Just to see if we're aligned on the remaining tasks for this paperwork. According to the link provided by @justinmclean, there are 4 steps that the Apache guide requires to be done:
   
   **[Check the Export Control Classification Number (ECCN)](https://infra.apache.org/crypto.html#classify)**
   **[DONE]** Basically there is no work on this. MbedTLS is already used by several Apache projects, including MyNewt, so we can rely on the pre-executed assessment.
   
   **[Update the Exports Page with Source Links](https://infra.apache.org/crypto.html#sources)**
   **[Pending]** I believe we need the support from @justinmclean for fulfilling this item. We just need an entry for `APACHE NUTTX (INCUBATING) PROJECT` on [this page](https://www.apache.org/licenses/exports/) like this:
   ### APACHE NUTTX (INCUBATING) PROJECT
   - Apache NuttX
     - development / 5D002 / [MbedTLS](https://www.trustedfirmware.org/projects/mbed-tls/)
   
   **[Notify the U.S. Government of the release](https://infra.apache.org/crypto.html#notify)**
   **[Pending]** Maybe the most complicated of all the tasks, but not that complicated at all. We need to fill the email template containing a brief description about MbedTLS or use the script from the Apache SVN for generating it. We may help on this one, Espressif has already gone through this process for enabling the integration of MbedTLS on ESP-IDF. Let us know if you need any support.
   
   **[Inform users by including a crypto notice in the distribution's README file](https://infra.apache.org/crypto.html#inform)**
   **[DONE]** We may simply copy the note from [Apache MyNewt](https://github.com/apache/mynewt-core#export-restrictions). See https://github.com/btashton/incubator-nuttx-apps/pull/1


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] fraviofii commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
fraviofii commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-826326814


   The libraries are not exported when we execute 'make export' for NuttX.
   
   This would be a nice future for this implementation.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r559656017



##########
File path: examples/mbedtls/Makefile
##########
@@ -0,0 +1,34 @@
+############################################################################
+# apps/examples/mbedtls/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# mbedtls built-in application info
+
+PROGNAME  = $(CONFIG_EXAMPLES_MBEDTLS_PROGNAME)
+PRIORITY  = $(CONFIG_EXAMPLES_MBEDTLS_PRIORITY)
+STACKSIZE = $(CONFIG_EXAMPLES_MBEDTLS_STACKSIZE)
+MODULE    = $(CONFIG_EXAMPLES_MBEDTLS)
+
+# Mbed TLS Demo
+
+MAINSRC = mbedtls_main.c

Review comment:
       Yep just using this as a minimal test for now.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563465122



##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       You cannot call `include $(APPDIR)/Application.mk` twice in the same make file which is they need to be in there own folders.  But I don't think that is related to this path issue.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] acassis commented on pull request #566: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-961302209


   Great work @btashton and @gustavonihei !


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] gustavonihei commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-961289548


   Created https://github.com/apache/incubator-nuttx-apps/issues/877 for addressing the remaining task.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] acassis commented on pull request #566: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-961302209


   Great work @btashton and @gustavonihei !


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563470913



##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       but you can specify multiple apps in PROGNAME and MAINSRC like zmodem:
   https://github.com/apache/incubator-nuttx-apps/blob/master/system/zmodem/Makefile#L38
   https://github.com/apache/incubator-nuttx-apps/blob/master/system/zmodem/Makefile#L45




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563452622



##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       I could not get that to work, but maybe I am doing something wrong
   with:
   ```
   MAINSRC = ../../mbedtls/test/selftest.c
   ```
   I get this
   ```
   make[3]: *** No rule to make target '../../mbedtls/test/selftest.c', needed by '../../mbedtls/test/selftest.home.bashton.nuttx.wrk.apps.crypto.mbedtls.apps.selftest.o'.  Stop.
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-768684410


   > Hi,
   > 
   > Just a note on adding this Mbed is dual licensed under Apache 2.0 and GPL so license wise this is OK. But you may need to inform the US government that NuttX contains encryption software. See here for the process [1]
   > 
   > Thanks,.
   > Justin
   > 
   > 1. https://infra.apache.org/crypto.html
   
   Thanks Justin, we have a note to handle that as well for a some other cypto code that came up in the last release.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] yamt commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r606210446



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,85 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}

Review comment:
       maybe.
   we can add all of these.
   https://github.com/ARMmbed/mbedtls/blob/d5200371ec383d32dd3cb9a105cb11f75749ae25/library/net_sockets.c#L319-L321




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] gustavonihei commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-898617693


   > @gustavonihei yeah we also need to take care of this for some stuff that is already merged in as well, so I think it makes sense to do most of it together.
   
   I don't have the tracking for these ones. I remember there is the `libtomcrypt` you've been working on some time ago. Besides that, is there anything else?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563443006



##########
File path: crypto/mbedtls/Make.defs
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifeq ($(CONFIG_CRYPTO_MBEDTLS),y)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls
+
+# Allows `<mbedtls/<>.h>` import.
+
+CFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+CXXFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+
+CFLAGS += -DMBEDTLS_CONFIG_FILE="<crypto/mbedtls_config.h>"

Review comment:
       should we use $(DEIFNE) like $(INCDIR)?

##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192
+
+source "$APPSDIR/crypto/mbedtls/apps/benchmark/Kconfig"
+source "$APPSDIR/crypto/mbedtls/apps/selftest/Kconfig"

Review comment:
       can we remove $APPSDIR/? since other location uses the relative path.

##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,61 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+WD := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}

Review comment:
       remove WD, nobody use it now.

##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192

Review comment:
       can we use the small stack size or DEFAULT_TASK_STACKSIZE?

##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       why not use the relative path? So we can remove MBEDPROGDIR and  crypto/mbedtls/apps/Makefile.

##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192
+
+source "$APPSDIR/crypto/mbedtls/apps/benchmark/Kconfig"
+source "$APPSDIR/crypto/mbedtls/apps/selftest/Kconfig"

Review comment:
       Ok, let's keep APPDIR

##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       I know your problem, both selftest.c and benchmark.c put in crypto/mbedtls/programs/test, but you put Makefile under:
   crypto/mbedtls/apps/selftest/Makefile
   crypto/mbedtls/apps/benchmark/Makefile
   Why not put Makfile to:
   crypto/mbedtls/programs
   and merge two apps into one Mafile?

##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       but you can specify multiple apps in PROGNAME and MAINSRC like zmodem:
   https://github.com/apache/incubator-nuttx-apps/blob/master/system/zmodem/Makefile#L38
   https://github.com/apache/incubator-nuttx-apps/blob/master/system/zmodem/Makefile#L45

##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       > You cannot call `include $(APPDIR)/Application.mk` twice in the same make file which is they need to be in there own folders. But I don't think that is related to this path issue.
   
   but you can specify multiple apps in PROGNAME and MAINSRC like zmodem:
   https://github.com/apache/incubator-nuttx-apps/blob/master/system/zmodem/Makefile#L38
   https://github.com/apache/incubator-nuttx-apps/blob/master/system/zmodem/Makefile#L45
   
   > also careful with your paths.
   > `crypto/mbedtls/programs/test` does not exist. `crypto/mbedtls/mbedtls/programs/test` does, but that is inside the downloaded source.
   
   Yes, we can't put into the download folder, crypto/mbedtls/Makefile may a better location for apps too.

##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       > You cannot call `include $(APPDIR)/Application.mk` twice in the same make file which is they need to be in there own folders. But I don't think that is related to this path issue.
   
   but you can specify multiple apps in PROGNAME and MAINSRC like zmodem:
   https://github.com/apache/incubator-nuttx-apps/blob/master/system/zmodem/Makefile#L38
   https://github.com/apache/incubator-nuttx-apps/blob/master/system/zmodem/Makefile#L45
   
   > also careful with your paths.
   > `crypto/mbedtls/programs/test` does not exist. `crypto/mbedtls/mbedtls/programs/test` does, but that is inside the downloaded source.
   
   Yes, we can't put into the download folder, crypto/mbedtls/Makefile may a better location for apps too with the multiple apps trick.

##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,91 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}
+
+CSRCS = $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)/$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)/$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)
+
+context:: $(MBEDTLS_UNPACKNAME)
+
+distclean::
+	$(call DELDIR, $(MBEDTLS_UNPACKNAME))
+	$(call DELFILE, $(MBEDTLS_ZIP))
+
+# Configuration Applications
+
+ifneq ($(CONFIG_MBEDTLS_APPS),)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls/
+
+PROGNAME =
+PRIORITY =
+STACKSIZE =
+MAINSRC =

Review comment:
       don't empty variable since it's default value. 

##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,91 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}
+
+CSRCS = $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)/$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)/$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)
+
+context:: $(MBEDTLS_UNPACKNAME)
+
+distclean::
+	$(call DELDIR, $(MBEDTLS_UNPACKNAME))
+	$(call DELFILE, $(MBEDTLS_ZIP))
+
+# Configuration Applications
+
+ifneq ($(CONFIG_MBEDTLS_APPS),)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls/

Review comment:
       CONFIGURED_APPS need define in Make.def, Makefile can't reference it if we define it here.

##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,80 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+menuconfig MBEDTLS_APPS

Review comment:
       normally, we reuse CRYPTO_MBEDTLS for module by changing it to tristate and update the if condition in Make.defs:
   ```
   ifneq ($(CONFIG_CRYPTO_MBEDTLS),)
   CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls
   endif
   ```

##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,91 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}
+
+CSRCS = $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)/$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)/$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)
+
+context:: $(MBEDTLS_UNPACKNAME)
+
+distclean::
+	$(call DELDIR, $(MBEDTLS_UNPACKNAME))
+	$(call DELFILE, $(MBEDTLS_ZIP))
+
+# Configuration Applications
+
+ifneq ($(CONFIG_MBEDTLS_APPS),)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls/

Review comment:
       CONFIGURED_APPS need define in Make.def, apps/Makefile can't reference it if we define it here.

##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       Yes, another normal approach is reuse CRYPTO_MBEDTLS option.

##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,91 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}
+
+CSRCS = $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)/$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)/$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)
+
+context:: $(MBEDTLS_UNPACKNAME)
+
+distclean::
+	$(call DELDIR, $(MBEDTLS_UNPACKNAME))
+	$(call DELFILE, $(MBEDTLS_ZIP))
+
+# Configuration Applications
+
+ifneq ($(CONFIG_MBEDTLS_APPS),)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls/

Review comment:
       We don't need define CONFIGURED_APPS in Makefile, it's enough to define CONFIGURED_APPS in Make.defs

##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192

Review comment:
       Yes, sim may need more stack size, DEFAULT_TASK_STACKSIZE has special value(64KB?) for sim.

##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,80 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+menuconfig MBEDTLS_APPS

Review comment:
       Yes, in this case(not zmodem), it may make sense to add a new variable for module.

##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192

Review comment:
       Why not create PRfor mbedtls to allocate 100KB from heap:)

##########
File path: crypto/mbedtls/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/crypto/mbedtls/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifeq ($(CONFIG_CRYPTO_MBEDTLS),y)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls
+
+# Allows `<mbedtls/<>.h>` import.
+
+CFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+CXXFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+
+CFLAGS += ${shell $(DEFINE) "$(CC)" MBEDTLS_CONFIG_FILE='"<crypto/mbedtls_config.h>"'}

Review comment:
       It's the known build system issue, @anchao has the patch to fix it, please wait a moment.

##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192

Review comment:
       Why not create PR for mbedtls to allocate 100KB from heap:)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-856995972


   @btashton do you have time to refine this PR too?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r561031046



##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,18 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n

Review comment:
       Yep I'm working on getting the Kconfig stuff wired up to turn on the config features correctly. This will take a bit.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563450371



##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,25 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+config MBEDTLS_DEFAULT_TASK_STACKSIZE
+	int "Mbed TLS app default stack size"
+	default 8192

Review comment:
       The issue is the default stack size is only 2KB but I normally see 3-4KB on real hardware.  I get that the sim is probably much higher so if we kept this I could conditional this like the DEFAULT_TASK_STACKSIZE.  Thoughts?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r606629850



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,85 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}

Review comment:
       Should we do the same as we did for MQTT-C and add `__NuttX__` to upstream mbedTLS?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563493208



##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       Yes, another normal approach is reuse CRYPTO_MBEDTLS option.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563553501



##########
File path: crypto/mbedtls/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/crypto/mbedtls/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifeq ($(CONFIG_CRYPTO_MBEDTLS),y)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls
+
+# Allows `<mbedtls/<>.h>` import.
+
+CFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+CXXFLAGS += ${shell $(INCDIR) $(INCDIROPT) "$(CC)" $(APPDIR)/crypto/mbedtls/mbedtls/include}
+
+CFLAGS += ${shell $(DEFINE) "$(CC)" MBEDTLS_CONFIG_FILE='"<crypto/mbedtls_config.h>"'}

Review comment:
       My plan is to completely replace it so that it is fully hooked into the Kconfig, I think that will end up being cleaner for us.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r559427697



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,133 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+MBEDTLS_DIR = .
+MBEDTLS_DIR_NAME = mbedtls
+
+# Set up build configuration and environment
+
+WD := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -o
+
+MBEDTLS_UNPACKDIR =  $(WD)/$(MBEDTLS_UNPACKNAME)
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKDIR)/library
+
+# Help
+CFLAGS += -D__unix__
+CFLAGS += -I$(MBEDTLS_UNPACKDIR)/include
+
+# Crypto Source
+CSRCS += \
+    $(MBEDTLS_UNPACKLIBDIR)/aesni.c \
+    $(MBEDTLS_UNPACKLIBDIR)/arc4.c \
+    $(MBEDTLS_UNPACKLIBDIR)/aria.c \
+    $(MBEDTLS_UNPACKLIBDIR)/asn1parse.c \
+    $(MBEDTLS_UNPACKLIBDIR)/asn1write.c \
+    $(MBEDTLS_UNPACKLIBDIR)/base64.c \
+    $(MBEDTLS_UNPACKLIBDIR)/bignum.c \
+    $(MBEDTLS_UNPACKLIBDIR)/blowfish.c \
+    $(MBEDTLS_UNPACKLIBDIR)/camellia.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ccm.c \
+    $(MBEDTLS_UNPACKLIBDIR)/chacha20.c \
+    $(MBEDTLS_UNPACKLIBDIR)/chachapoly.c \
+    $(MBEDTLS_UNPACKLIBDIR)/cipher.c \
+    $(MBEDTLS_UNPACKLIBDIR)/cipher_wrap.c \
+    $(MBEDTLS_UNPACKLIBDIR)/cmac.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ctr_drbg.c \
+    $(MBEDTLS_UNPACKLIBDIR)/des.c \
+    $(MBEDTLS_UNPACKLIBDIR)/dhm.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecdh.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecdsa.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecjpake.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecp.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ecp_curves.c \
+    $(MBEDTLS_UNPACKLIBDIR)/entropy.c \
+    $(MBEDTLS_UNPACKLIBDIR)/entropy_poll.c \
+    $(MBEDTLS_UNPACKLIBDIR)/error.c \
+    $(MBEDTLS_UNPACKLIBDIR)/gcm.c \
+    $(MBEDTLS_UNPACKLIBDIR)/havege.c \
+    $(MBEDTLS_UNPACKLIBDIR)/hkdf.c \
+    $(MBEDTLS_UNPACKLIBDIR)/hmac_drbg.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md2.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md4.c \
+    $(MBEDTLS_UNPACKLIBDIR)/md5.c \
+    $(MBEDTLS_UNPACKLIBDIR)/memory_buffer_alloc.c \
+    $(MBEDTLS_UNPACKLIBDIR)/nist_kw.c \
+    $(MBEDTLS_UNPACKLIBDIR)/oid.c \
+    $(MBEDTLS_UNPACKLIBDIR)/padlock.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pem.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pk.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pk_wrap.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkcs12.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkcs5.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkparse.c \
+    $(MBEDTLS_UNPACKLIBDIR)/pkwrite.c \
+    $(MBEDTLS_UNPACKLIBDIR)/platform.c \
+    $(MBEDTLS_UNPACKLIBDIR)/platform_util.c \
+    $(MBEDTLS_UNPACKLIBDIR)/poly1305.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_driver_wrappers.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_se.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_slot_management.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_crypto_storage.c \
+    $(MBEDTLS_UNPACKLIBDIR)/psa_its_file.c \
+    $(MBEDTLS_UNPACKLIBDIR)/ripemd160.c \
+    $(MBEDTLS_UNPACKLIBDIR)/rsa.c \
+    $(MBEDTLS_UNPACKLIBDIR)/rsa_internal.c \
+    $(MBEDTLS_UNPACKLIBDIR)/sha1.c \
+    $(MBEDTLS_UNPACKLIBDIR)/sha256.c \
+    $(MBEDTLS_UNPACKLIBDIR)/sha512.c \
+    $(MBEDTLS_UNPACKLIBDIR)/threading.c \
+    $(MBEDTLS_UNPACKLIBDIR)/timing.c \
+    $(MBEDTLS_UNPACKLIBDIR)/version.c \
+    $(MBEDTLS_UNPACKLIBDIR)/version_features.c \
+    $(MBEDTLS_UNPACKLIBDIR)/xtea.c \

Review comment:
       These are all the files?  are you planning to select some using Kconfig?
   Otherwise we can grab all of them at once with `CSRCS := $(notdir $(wildcard $(MBEDTLS_SRCDIR)$(DELIM)*.c))`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563563200



##########
File path: crypto/mbedtls/Makefile
##########
@@ -0,0 +1,91 @@
+############################################################################
+# apps/crypto/mbedtls/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+
+# Mbed TLS crypto library
+
+# Set up build configuration and environment
+
+MBEDTLS_URL ?= "https://github.com/ARMmbed/mbedtls/archive"
+
+MBEDTLS_VERSION = $(patsubst "%",%,$(strip $(CONFIG_MBEDTLS_VERSION)))
+MBEDTLS_ZIP = v$(MBEDTLS_VERSION).zip
+
+MBEDTLS_UNPACKNAME = mbedtls
+UNPACK ?= unzip -q -o
+
+MBEDTLS_UNPACKLIBDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)library
+MBEDTLS_UNPACKPROGDIR = $(MBEDTLS_UNPACKNAME)$(DELIM)programs
+
+# This lets Mbed TLS better use some of the POSIX features we have
+CFLAGS += ${shell $(DEFINE) "$(CC)" __unix__}
+
+CSRCS = $(wildcard $(MBEDTLS_UNPACKLIBDIR)$(DELIM)*.c)
+
+
+$(MBEDTLS_ZIP):
+	@echo "Downloading: $(MBEDTLS_URL)/$(MBEDTLS_ZIP)"
+	$(Q) curl -O -L $(MBEDTLS_URL)/$(MBEDTLS_ZIP)
+
+$(MBEDTLS_UNPACKNAME): $(MBEDTLS_ZIP)
+	@echo "Unpacking: $(MBEDTLS_ZIP) -> $(MBEDTLS_UNPACKNAME)"
+	$(Q) $(UNPACK) $(MBEDTLS_ZIP)
+	$(Q) mv	mbedtls-$(MBEDTLS_VERSION) $(MBEDTLS_UNPACKNAME)
+	$(Q) touch $(MBEDTLS_UNPACKNAME)
+
+context:: $(MBEDTLS_UNPACKNAME)
+
+distclean::
+	$(call DELDIR, $(MBEDTLS_UNPACKNAME))
+	$(call DELFILE, $(MBEDTLS_ZIP))
+
+# Configuration Applications
+
+ifneq ($(CONFIG_MBEDTLS_APPS),)
+CONFIGURED_APPS += $(APPDIR)/crypto/mbedtls/

Review comment:
       Ok, done.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] btashton commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563465666



##########
File path: crypto/mbedtls/apps/selftest/Makefile
##########
@@ -0,0 +1,33 @@
+############################################################################
+# apps/crypto/mbedtls/apps/selftest/Makefile
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+include $(APPDIR)/Make.defs
+include ../Makefile
+
+# mbedtls built-in benchmark application info
+
+PROGNAME  = "mbedselftest"
+PRIORITY  = $(CONFIG_MBEDTLS_APP_SELFTEST_PRIORITY)
+STACKSIZE = $(CONFIG_MBEDTLS_APP_SELFTEST_STACKSIZE)
+MODULE    = $(CONFIG_MBEDTLS_APP_SELFTEST)
+
+MAINSRC = $(MBEDPROGDIR)/test/selftest.c

Review comment:
       also careful with your paths.
   `crypto/mbedtls/programs/test` does not exist. `crypto/mbedtls/mbedtls/programs/test` does, but that is inside the downloaded source.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#discussion_r563512474



##########
File path: crypto/mbedtls/Kconfig
##########
@@ -0,0 +1,80 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+menuconfig CRYPTO_MBEDTLS
+	bool "Mbed TLS Cryptography Library"
+	default n
+	---help---
+		Enable support for Mbed TLS.
+
+if CRYPTO_MBEDTLS
+
+config MBEDTLS_VERSION
+	string "MBEDTLS Version"
+	default "2.25.0"
+
+menuconfig MBEDTLS_APPS

Review comment:
       Yes, in this case(not zmodem), it may make sense to add a new variable for module.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] gustavonihei edited a comment on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
gustavonihei edited a comment on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-898611028


   @btashton Just to see if we're aligned on the remaining tasks for this paperwork. According to the link provided by @justinmclean, there are 4 steps that the Apache guide requires to be done:
   
   **[Check the Export Control Classification Number (ECCN)](https://infra.apache.org/crypto.html#classify)**
   **[DONE]** Basically there is no work on this. MbedTLS is already used by several Apache projects, including MyNewt, so we can rely on the pre-executed assessment.
   
   **[Update the Exports Page with Source Links](https://infra.apache.org/crypto.html#sources)**
   **[Pending]** I believe we need the support from @justinmclean for fulfilling this item. We just need an entry for `APACHE NUTTX (INCUBATING) PROJECT` on [this page](https://www.apache.org/licenses/exports/) like this:
   ### APACHE NUTTX (INCUBATING) PROJECT
   - Apache NuttX
     - development / 5D002 / [ARM MbedTLS](https://www.trustedfirmware.org/projects/mbed-tls/)
   
   **[Notify the U.S. Government of the release](https://infra.apache.org/crypto.html#notify)**
   **[Pending]** Maybe the most complicated of all the tasks, but not that complicated at all. We need to fill the email template containing a brief description about MbedTLS or use the script from the Apache SVN for generating it. We may help on this one, Espressif has already gone through this process for enabling the integration of MbedTLS on ESP-IDF. Let us know if you need any support.
   
   **[Inform users by including a crypto notice in the distribution's README file](https://infra.apache.org/crypto.html#inform)**
   **[DONE]** We may simply copy the note from [Apache MyNewt](https://github.com/apache/mynewt-core#export-restrictions). See https://github.com/btashton/incubator-nuttx-apps/pull/1


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] acassis commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-882552606


   Thank you @btashton, please let me know how we can help you to complete it. Our new Wi-Fi library will not include the wpa_supplicant internally and we need to provide mbedTLS to be able to use it. Also the MCUBoot port that @gustavonihei is doing will benefit from using mbedTLS.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx-apps] gustavonihei commented on pull request #566: WIP: crypto: Initial support for Mbed TLS

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #566:
URL: https://github.com/apache/incubator-nuttx-apps/pull/566#issuecomment-925788825


   There was a conflict on `crypto/Makefile`, which I got to resolve via the web UI.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org