Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

sys/shell: new module shell_lock #13082

Merged
merged 6 commits into from
Jun 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions examples/telnet_server/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,13 @@ void telnet_cb_pre_connected(sock_tcp_t *sock)
printf("%s connected\n", addr_str);
}

/* shell lock module makes use of disconnect callback */
#ifndef MODULE_SHELL_LOCK
void telnet_cb_disconneced(void)
{
puts("disconnected");
}
#endif

void telnet_cb_connected(sock_tcp_t *sock)
{
Expand Down
1 change: 1 addition & 0 deletions makefiles/pseudomodules.inc.mk
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ PSEUDOMODULES += senml_saul
PSEUDOMODULES += sha1sum
PSEUDOMODULES += sha256sum
PSEUDOMODULES += shell_hooks
PSEUDOMODULES += shell_lock_auto_locking
PSEUDOMODULES += slipdev_stdio
PSEUDOMODULES += slipdev_l2addr
PSEUDOMODULES += sock
Expand Down
4 changes: 4 additions & 0 deletions sys/Makefile.dep
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ ifneq (,$(filter trace,$(USEMODULE)))
USEMODULE += ztimer_usec
endif

ifneq (,$(filter shell_lock,$(USEMODULE)))
USEMODULE += ztimer_msec
endif

ifneq (,$(filter ssp,$(USEMODULE)))
FEATURES_REQUIRED += ssp
endif
Expand Down
4 changes: 4 additions & 0 deletions sys/Makefile.include
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,7 @@ endif
ifneq (,$(filter test_utils_netdev_eth_minimal,$(USEMODULE)))
CFLAGS += -DCONFIG_NETDEV_REGISTER_SIGNAL
endif

ifneq (,$(filter shell_lock,$(USEMODULE)))
include $(RIOTBASE)/sys/shell_lock/Makefile.include
endif
8 changes: 8 additions & 0 deletions sys/include/net/telnet.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,14 @@ int telnet_server_write(const void* buffer, size_t len);
*/
int telnet_server_read(void* buffer, size_t count);

/**
* @brief Request to disconnect the current client
*
* This only sets the disconnect request flag, so it's safe to call
* this from interrupt context.
*/
void telnet_server_disconnect(void);

/**
* @brief Callback function that gets called when a telnet client connects
* but before stdio is redirected.
Expand Down
84 changes: 84 additions & 0 deletions sys/include/shell_lock.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* Copyright (C) 2020 Freie Universität Berlin
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @defgroup sys_shell_lock Shell lock
* @ingroup sys
* @brief Simple module to provide a password protection for the shell.
* @experimental This module is an experimental feature and only shows as a proof of concept how
* the shell could be protected with a password. Do not expect relevant security from
* it for production, since Man-in-the-Middle attacks are possible depending on the
* used connection method!
*
* @{
*
* @file
* @brief Shell interface definition
*/

#ifndef SHELL_LOCK_H
#define SHELL_LOCK_H

#ifdef __cplusplus
extern "C" {
#endif

#include "shell.h"

#ifdef MODULE_SHELL_LOCK
#ifndef CONFIG_SHELL_LOCK_PASSWORD
#error Using MODULE_SHELL_LOCK requires defining CONFIG_SHELL_LOCK_PASSWORD
#endif /* CONFIG_SHELL_LOCK_PASSWORD */
#endif /* MODULE_SHELL_LOCK */

/**
* @brief Lock the login process after given attempts of failed logins for
* a few seconds
*/
#define CONFIG_SHELL_LOCK_ATTEMPTS_BEFORE_TIME_LOCK 3

#ifndef CONFIG_SHELL_LOCK_AUTO_LOCK_TIMEOUT_MS
/**
* @brief Lock the shell after this time span without user input
* Defaults to 5 minutes but can be overwritten in the applications
* Makefile.
*/
#define CONFIG_SHELL_LOCK_AUTO_LOCK_TIMEOUT_MS (5 * 60 * 1000)
#endif

/**
* @brief Entry point for the lock mechanism. If locked, the user will
* be asked for a password. This function won't return until the
* correct password has been entered.
*
* @param[in] line_buf Buffer for reading in the password from stdin
* @param[in] buf_size Buffer size
*/
void shell_lock_checkpoint(char *line_buf, int buf_size);

/**
* @brief Returns true, if the shell is in the locked state.
*
* @return Whether the shell is locked or not.
*/
bool shell_lock_is_locked(void);

#ifdef MODULE_SHELL_LOCK_AUTO_LOCKING
/**
* @brief Restart the timeout interval before the shell is locked
* automatically.
*/
void shell_lock_auto_lock_refresh(void);
#endif /* MODULE_SHELL_LOCK_AUTO_LOCKING */

#ifdef __cplusplus
}
#endif

#endif /* SHELL_LOCK_H */
/** @} */
14 changes: 14 additions & 0 deletions sys/net/application_layer/telnet/telnet_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ static pipe_t _stdin_pipe;
static sock_tcp_queue_t sock_queue;
static sock_tcp_t socks[CONFIG_TELNET_TCP_QUEUE_SIZE];
static sock_tcp_t *client;
static bool _want_disconnect;

static char telnet_stack[THREAD_STACKSIZE_DEFAULT];

Expand Down Expand Up @@ -206,6 +207,12 @@ static void *telnet_thread(void *arg)
_acquire();
res = sock_tcp_read(client, rx_buf, sizeof(rx_buf), SOCK_TCP_TIMEOUT_MS);
_release();

if (_want_disconnect) {
_want_disconnect = false;
break;
}

if (res == -ETIMEDOUT) {
continue;
}
Expand Down Expand Up @@ -280,6 +287,13 @@ int telnet_server_read(void* buffer, size_t count)
return res;
}

void telnet_server_disconnect(void)
{
if (connected) {
_want_disconnect = true;
}
}

int telnet_server_start(void)
{
sock_tcp_ep_t ep = SOCK_IPV6_EP_ANY;
Expand Down
23 changes: 22 additions & 1 deletion sys/shell/shell.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include "kernel_defines.h"
#include "xfa.h"
#include "shell.h"
#include "shell_lock.h"

/* define shell command cross file array */
XFA_INIT_CONST(shell_command_t*, shell_commands_xfa);
Expand All @@ -61,6 +62,10 @@ XFA_INIT_CONST(shell_command_t*, shell_commands_xfa);

#define PARSE_ESCAPE_MASK 0x4;

extern void shell_lock_checkpoint(char *line_buf, int len);
extern bool shell_lock_is_locked(void);
Comment on lines +65 to +66
Copy link
Contributor

Choose a reason for hiding this comment

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

If you are doing that you might as well

Suggested change
extern void shell_lock_checkpoint(char *line_buf, int len);
extern bool shell_lock_is_locked(void);
#if IS_USED(MODULE_SHELL_LOCK)
extern void shell_lock_checkpoint(char *line_buf, int len);
extern bool shell_lock_is_locked(void);
#else
static inline void shell_lock_checkpoint(char *line_buf, int len)
{
(void)line_buf;
(void)len;
}
static inline bool shell_lock_is_locked(void)
{
return false;
}
#endif

and avoid cluttering the code further down with if (IS_USED(…)) blocks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mh I don't see the benefit of doing this. At the moment it is in line with MODULE_SHELL_HOOKS and MODULE_SHELL_COMMANDS. Since these are just forward declarations we don't need empty definitions in case the module is not used at all.

extern void shell_lock_auto_lock_refresh(void);

enum parse_state {
PARSE_BLANK = 0x0,

Expand Down Expand Up @@ -106,6 +111,7 @@ static shell_command_handler_t find_handler(
const shell_command_t *command_list, char *command)
{
shell_command_handler_t handler = NULL;

if (command_list != NULL) {
handler = search_commands(command_list, command);
}
Expand Down Expand Up @@ -404,7 +410,7 @@ static inline void new_line(void)
* @return EOF, if the end of the input stream was reached.
* @return -ENOBUFS if the buffer size was exceeded.
*/
static int readline(char *buf, size_t size)
int readline(char *buf, size_t size) /* needed externally by module shell_lock */
{
int curr_pos = 0;
bool length_exceeded = false;
Expand Down Expand Up @@ -471,11 +477,26 @@ static int readline(char *buf, size_t size)
void shell_run_once(const shell_command_t *shell_commands,
char *line_buf, int len)
{
if (IS_USED(MODULE_SHELL_LOCK)) {
shell_lock_checkpoint(line_buf, len);
}

print_prompt();

while (1) {
int res = readline(line_buf, len);

if (IS_USED(MODULE_SHELL_LOCK)) {
if (shell_lock_is_locked()) {
break;
}
}

if (IS_USED(MODULE_SHELL_LOCK_AUTO_LOCKING)) {
/* reset lock countdown in case of new input */
shell_lock_auto_lock_refresh();
}

switch (res) {

case EOF:
Expand Down
1 change: 1 addition & 0 deletions sys/shell_lock/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include $(RIOTBASE)/Makefile.base
4 changes: 4 additions & 0 deletions sys/shell_lock/Makefile.include
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
$(shell $(COLOR_ECHO) "$(COLOR_YELLOW)shell_lock is an experimental feature and only shows as a \
proof of concept how the shell could be protected with a password. Do not expect relevant \
security from it for production, since Man-in-the-Middle attacks are possible depending on the \
used connection method!$(COLOR_RESET)" 1>&2)
Loading