sched/nsh: Remove Hard-coded Default Password#18396
sched/nsh: Remove Hard-coded Default Password#18396Abhishekmishra2808 wants to merge 3 commits intoapache:masterfrom
Conversation
|
@Abhishekmishra2808 the Documentation and the boardshould be (each one) in a separated. Normally we separate the logic implementation from the board support and Documentation. |
53b43ad to
0670d28
Compare
|
Thank you @Abhishekmishra2808 :-)
|
0670d28 to
0214816
Compare
|
Hi @cederom , I used AI tools only to help refine wording and improve clarity in the description, but the implementation, debugging, and testing were done by me. |
|
@acassis I have fixed the changes suggested by you, and CI was failing because password generation was enabled in the defconfig files without setting a password. I have now removed |
|
@Abhishekmishra2808 please normalize the boards/sim/sim/sim/configs/login/defconfig to this the CI error, run: $ ./tools/refresh.sh --silent sim:login And then squash the modification to your previous commit |
9bc112a to
f9dff1a
Compare
| .. code:: console | ||
|
|
||
| $ grep <your-password> boards/<arch>/<chip>/<board>/src/etctmp.c | ||
| # must print nothing |
There was a problem hiding this comment.
@Abhishekmishra2808 could you please add at the end of this Documentation explaining that to avoid leaking user password (CONFIG_BOARD_ETC_ROMFS_PASSWD_PASSWORD) and keys (CONFIG_FSUTILS_PASSWD_KEY1-4) when user run "make savedefconfig" it will not be saved in the defconfig. And if the user needs it in their local defconfig they need to add it manually in their defconfig.
There was a problem hiding this comment.
Thanks- addressed this in the latest commit.
f9dff1a to
8dd55e8
Compare
|
@Abhishekmishra2808 you need to analyze the errors (just click in the task that failed) and scroll the errors Seem like many boards need to be normalized after that, just an example:
Saving the new configuration file See here how to normalize many profiles are same time (for a board, or a chip family, etc) https://nuttx.apache.org/docs/latest/components/tools/refresh.html |
8dd55e8 to
c486112
Compare
|
Finally, CI turned green, ready to get final approvals :) |
|
@raiden00pl please review again! |
cmake/savedefconfig.cmake
Outdated
| endforeach() | ||
|
|
||
| message( | ||
| WARNING "CONFIG_BOARD_ETC_ROMFS_PASSWD_PASSWORD and" |
There was a problem hiding this comment.
why not move to the first patch? the change should be completed in one patch
|
@Abhishekmishra2808 could you refine your patchset?
|
c486112 to
4585eb1
Compare
|
@xiaoxiang781216 Thanks for the review feedback. I've refined the patchset as requested and addressed all the comments you made, please check. |
tools/Unix.mk
Outdated
| $(Q) rm -f warning.tmp | ||
| $(Q) rm -f defconfig.tmp | ||
| $(Q) rm -f sortedconfig.tmp | ||
| $(Q) echo "WARNING: CONFIG_BOARD_ETC_ROMFS_PASSWD_PASSWORD was not saved in defconfig." |
There was a problem hiding this comment.
could you remove the warning
There was a problem hiding this comment.
Agree, I will remove unconditional warning prints. I will only print this warning when ETC passwd autogen is enabled during savedefconfig, so users are informed only when it is relevant.
cmake/savedefconfig.cmake
Outdated
| WARNING "CONFIG_BOARD_ETC_ROMFS_PASSWD_PASSWORD and" | ||
| " CONFIG_FSUTILS_PASSWD_KEY1-4 were intentionally excluded from" | ||
| " defconfig by savedefconfig to avoid leaking credentials." | ||
| " Add them manually in local defconfig if needed.") |
There was a problem hiding this comment.
why always output warning
There was a problem hiding this comment.
@Abhishekmishra2808 @xiaoxiang781216 this warning should be printed when to user enabled the ETC PASSWD and run "make savedefconfig", it is printed to warn the user that the password and keys cannot be stored automatically in the generated defconfig
There was a problem hiding this comment.
Good point. I will make the warning conditional in CMake as well, same behavior as Make path, so it is not always noisy but still warns users when password/key values are intentionally excluded.
boards/Board.mk
Outdated
| $(call PREPROCESS, $<, $@) | ||
|
|
||
| $(ETCSRC): $(foreach raw,$(RCRAWS), $(if $(wildcard $(BOARD_DIR)$(DELIM)src$(DELIM)$(raw)), $(BOARD_DIR)$(DELIM)src$(DELIM)$(raw), $(if $(wildcard $(BOARD_COMMON_DIR)$(DELIM)$(raw)), $(BOARD_COMMON_DIR)$(DELIM)$(raw), $(BOARD_DIR)$(DELIM)src$(DELIM)$(raw)))) $(RCOBJS) | ||
| $(ETCSRC): $(foreach raw,$(RCRAWS), $(if $(wildcard $(BOARD_DIR)$(DELIM)src$(DELIM)$(raw)), $(BOARD_DIR)$(DELIM)src$(DELIM)$(raw), $(if $(wildcard $(BOARD_COMMON_DIR)$(DELIM)$(raw)), $(BOARD_COMMON_DIR)$(DELIM)$(raw), $(BOARD_DIR)$(DELIM)src$(DELIM)$(raw)))) $(RCOBJS) $(TOPDIR)$(DELIM).config $(TOPDIR)$(DELIM)tools$(DELIM)mkpasswd.c |
There was a problem hiding this comment.
don't need add the dependence, since you build mkpasswd in action manually
boards/Board.mk
Outdated
| ifeq ($(CONFIG_BOARD_ETC_ROMFS_PASSWD_PASSWORD),"") | ||
| $(error CONFIG_BOARD_ETC_ROMFS_PASSWD_PASSWORD must be set when BOARD_ETC_ROMFS_PASSWD_ENABLE is enabled. Run 'make menuconfig' to set a password.) | ||
| endif | ||
| $(Q) if [ ! -f $(TOPDIR)$(DELIM)tools$(DELIM)mkpasswd$(HOSTEXEEXT) ] || \ |
There was a problem hiding this comment.
why need change the existence of mkpasswd? make will do it automatically
boards/Board.mk
Outdated
| ifeq ($(CONFIG_BOARD_ETC_ROMFS_PASSWD_PASSWORD),) | ||
| $(error CONFIG_BOARD_ETC_ROMFS_PASSWD_PASSWORD must be set when BOARD_ETC_ROMFS_PASSWD_ENABLE is enabled. Run 'make menuconfig' to set a password.) | ||
| endif | ||
| ifeq ($(CONFIG_BOARD_ETC_ROMFS_PASSWD_PASSWORD),"") |
There was a problem hiding this comment.
remove the check since mkpasswd already do the check
There was a problem hiding this comment.
I think the idea here is to verify if the user defined the password in the "menuconfig"
There was a problem hiding this comment.
mkpasswd already check it, why dup the check in many places
There was a problem hiding this comment.
Right, maybe keeping the testing only is mkpasswd is enough, but we need to understand where is better to report the error. I think failing early is better to let user to fix the issue. @Abhishekmishra2808 please comment your idea here
There was a problem hiding this comment.
for me I think we should keep mkpasswd as the source of truth for validation and keep only one early empty password check for better user feedback when the feature is enabled. this avoids duplicated checks while still failing early with a clear message. For savedefconfig, we can print the warning conditionally only when this feature is enabled, so users are informed without adding noise.
boards/Kconfig
Outdated
|
|
||
| config BOARD_ETC_ROMFS_PASSWD_PASSWORD | ||
| string "Admin password (required)" | ||
| default "" |
There was a problem hiding this comment.
remove the default value
There was a problem hiding this comment.
If the test to verify if the user defined a password is used, I think the default password needs to be defined here as empty, so seems correct, isn't it?
There was a problem hiding this comment.
I think we can satisfy both points by keeping the behavior but removing redundancy: I’ll remove the explicit default "" line (since empty is already implicit for string Kconfig symbols), and I’ll keep the fail-fast check when passwd autogen is enabled so we still enforce that the user sets a password in menuconfig. So we keep the same user-facing behavior, but with cleaner Kconfig and less duplicated logic.
|
|
||
| # Auto-generate /etc/passwd at build time if configured | ||
| if(CONFIG_BOARD_ETC_ROMFS_PASSWD_ENABLE) | ||
| if("${CONFIG_BOARD_ETC_ROMFS_PASSWD_PASSWORD}" STREQUAL "") |
There was a problem hiding this comment.
Same approach here as Make path: no duplicated detailed checks, only one early empty-password check for fail-fast feedback and leave full validation to mkpasswd.
|
I have shared my view in the thread above. If this aligns with everyone, I will implement the following in the next commit: keep mkpasswd as source of truth for detailed validation, keep one fail-fast empty-password check when passwd autogen is enabled, remove duplicated checks, make savedefconfig warning conditional, keep credentials excluded from generated defconfig, remove redundant explicit empty default in Kconfig, and ensure Make/CMake behavior stays consistent. |
Introduce mkpasswd, a host tool for generating encrypted password files at build time using TEA encryption. This enables secure, credential-free firmware images while allowing build-time password configuration. Changes: * Add mkpasswd.c host tool for TEA-based password hashing and encryption * Integrate mkpasswd into Make build system (tools/Makefile.host) * Add CMake support for mkpasswd compilation and ROMFS passwd generation * Add CONFIG_BOARD_ETC_ROMFS_PASSWD_* configuration options to Kconfig * Implement credential exclusion from defconfig to prevent password leaking * Update savedefconfig.cmake to strip sensitive credentials * Handle common build infrastructure for passwd file auto-generation * Implement default-key warning detection in mkpasswd tool only This provides generic framework before per-board migrations. Signed-off-by: Abhishek Mishra <mishra.abhishek2808@gmail.com>
Migrate boards from static /etc/passwd files to build-time generation: * Remove static etc/passwd files from SIM and ESP32-C3-legacy boards * Update board configurations to enable BOARD_ETC_ROMFS_PASSWD_ENABLE * Configure SIM board with login demo user (admin/Administrator) * Update board build rules to use newly generated passwd files * Remove CMakeLists.txt dependency on static passwd in SIM This completes the infrastructure migration for boards supporting login functionality. Signed-off-by: Abhishek Mishra <mishra.abhishek2808@gmail.com>
Document the new mkpasswd-based password generation system and its integration with the build process. Changes: * Add comprehensive mkpasswd tool documentation to components/tools * Update SIM board docs to explain generated passwd workflow * Update ESP32-C3-legacy board docs for passwd generation * Update RX65N board docs with credential handling guidance * Document how to configure and use BOARD_ETC_ROMFS_PASSWD_* options * Explain security benefits of build-time generation vs static files Helps users understand password configuration and security implications. Signed-off-by: Abhishek Mishra <mishra.abhishek2808@gmail.com>
4585eb1 to
239da6b
Compare
Summary
This PR introduces build-time generation of the
/etc/passwdfile for the ROMFS image when authentication is enabled.Instead of relying on a static
etc/passwdfile embedded in the source tree, the passwd entry is now generated during the build using the configuration values:CONFIG_ETC_ROMFS_PASSWD_USERCONFIG_ETC_ROMFS_PASSWD_PASSWORDCONFIG_ETC_ROMFS_PASSWD_UIDCONFIG_ETC_ROMFS_PASSWD_GIDCONFIG_ETC_ROMFS_PASSWD_HOMEThe generated passwd entry is written into the ROMFS staging directory and included in the firmware image.
Behavior
Authentication disabled
Authentication enabled
/etc/passwdautomatically.Password missing
CONFIG_ETC_ROMFS_GENPASSWD=ybut the password is empty, the build fails with an explicit error.This ensures that credentials are always explicitly configured when authentication is enabled and prevents firmware images from being built with empty passwords.
Security Improvement
Previously
/etc/passwdcould be included as a static file in the ROMFS source tree.With this change, the credentials are generated at build time and must be explicitly configured, avoiding implicit or default credentials in firmware images.
Testing
Generated passwd entry
Plaintext password check

Build failure when password is empty