Merge pull request #13082 from HendrikVE/password_protected_shell

sys/shell: new module shell_lock

examples/telnet_server/main.c

@@ -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)
 {

makefiles/pseudomodules.inc.mk

@@ -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

sys/Makefile.dep

@@ -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

sys/Makefile.include

@@ -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

sys/include/net/telnet.h

@@ -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.

sys/include/shell_lock.h

@@ -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 */
+/** @} */

sys/net/application_layer/telnet/telnet_server.c

@@ -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];
 
@@ -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;
             }
@@ -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;

sys/shell/shell.c

@@ -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);
@@ -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);
+extern void shell_lock_auto_lock_refresh(void);
+
 enum parse_state {
     PARSE_BLANK             = 0x0,
 
@@ -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);
     }
@@ -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;
@@ -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:

sys/shell_lock/Makefile

@@ -0,0 +1 @@
+include $(RIOTBASE)/Makefile.base

sys/shell_lock/Makefile.include

@@ -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)

sys/shell_lock/shell_lock.c

@@ -0,0 +1,209 @@
+/*
+ * 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.
+ */
+
+/**
+ * @ingroup     sys_shell_lock
+ * @{
+ *
+ * @file
+ * @brief       Module to lock the running shell with a password.
+ *
+ * The Shell is proceeded only when the valid password was entered by the user.
+ * After 3 (default) failed attempts, the input is blocked for a few seconds to
+ * slow down brute force attacks.
+ * Does not make use of any cryptographic features yet.
+ *
+ * This module also provides a pseudomodule for automated locking after a given
+ * interval. Add "USEMODULE += shell_lock_auto_locking" to your Makefile to
+ * enable this feature.
+ *
+ * @author      Hendrik van Essen <hendrik.ve@fu-berlin.de>
+ *
+ * @}
+ */
+
+#include <string.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <stdlib.h>
+
+#ifdef MODULE_STDIO_TELNET
+#include "net/telnet.h"
+#endif
+#include "ztimer.h"
+
+#include "shell_lock.h"
+
+#if defined(MODULE_NEWLIB) || defined(MODULE_PICOLIBC)
+    #define flush_if_needed() fflush(stdout)
+#else
+    #define flush_if_needed()
+#endif /* MODULE_NEWLIB || MODULE_PICOLIBC */
+
+static bool _shell_is_locked = true;
+
+#ifdef MODULE_SHELL_LOCK_AUTO_LOCKING
+static ztimer_t _shell_auto_lock_ztimer;
+#endif
+
+/* defined in shell.c */
+extern int readline(char *buf, size_t size);
+
+static int _lock_handler(int argc, char **argv)
+{
+    (void) argc;
+    (void) argv;
+
+    _shell_is_locked = true;
+
+    return 0;
+}
+
+SHELL_COMMAND(lock, "Lock the shell", _lock_handler);
+
+static inline void _print_password_prompt(void)
+{
+    printf("Password: ");
+    flush_if_needed();
+}
+
+/* Implementation of strcmp that does not return after the first difference
+ * which could give away information about the first n correct characters of
+ * the password. The length of the loop is only dependent on the input string.
+ * Don't optimize this function by a compiler. */
+static bool __attribute__((optimize("O0"))) _safe_strcmp(const char* input, const char* pwd)
+{
+    bool the_same = true;
+
+    int input_len = strlen(input);
+    int pwd_len = strlen(pwd);
+
+    int input_index = 0;
+    int pwd_index = 0;
+
+    do {
+        if (input[input_index] != pwd[pwd_index]) {
+            the_same &= false;
+        }
+        else {
+            the_same &= true;
+        }
+
+        /* keep indices at last index of respective string */
+        if (input_index < input_len) {
+            input_index++;
+        }
+
+        if (pwd_index < pwd_len) {
+            pwd_index++;
+        }
+
+    } while (input[input_index] != '\0');
+
+    if (input_len != pwd_len) {
+        /* substring of the password doesn't count */
+        return false;
+    }
+
+    return the_same;
+}
+
+static bool _login(char *line_buf, size_t buf_size)
+{
+    _print_password_prompt();
+
+    while (1) {
+        memset(line_buf, 0, buf_size);
+        while (readline(line_buf, buf_size) > 0) {
+            return _safe_strcmp(line_buf, CONFIG_SHELL_LOCK_PASSWORD);
+        }
+    }
+
+    return false;
+}
+
+/**
+ * Repeatedly prompt for the password.
+ *
+ * This function won't return until the correct password has been entered.
+ */
+static void _login_barrier(char *line_buf, size_t buf_size)
+{
+    while (1) {
+        int attempts = CONFIG_SHELL_LOCK_ATTEMPTS_BEFORE_TIME_LOCK;
+
+        while (attempts--) {
+            if (_login(line_buf, buf_size)) {
+                return;
+            }
+            puts("Wrong password");
+            ztimer_sleep(ZTIMER_MSEC, 1000);
+        }
+#ifdef MODULE_STDIO_TELNET
+        telnet_server_disconnect();
+#endif
+        ztimer_sleep(ZTIMER_MSEC, 7000);
+    }
+}
+
+#ifdef MODULE_STDIO_TELNET
+void telnet_cb_disconneced(void)
+{
+    _shell_is_locked = true;
+}
+#endif
+
+#ifdef MODULE_SHELL_LOCK_AUTO_LOCKING
+static void _shell_auto_lock_ztimer_callback(void *arg)
+{
+    (void) arg;
+
+#ifdef MODULE_STDIO_TELNET
+    telnet_server_disconnect();
+#endif
+    _shell_is_locked = true;
+}
+
+void shell_lock_auto_lock_refresh(void)
+{
+    ztimer_remove(ZTIMER_MSEC, &_shell_auto_lock_ztimer);
+    ztimer_set(ZTIMER_MSEC, &_shell_auto_lock_ztimer,
+               CONFIG_SHELL_LOCK_AUTO_LOCK_TIMEOUT_MS);
+}
+#endif
+
+bool shell_lock_is_locked(void)
+{
+    return _shell_is_locked;
+}
+
+void shell_lock_checkpoint(char *line_buf, int buf_size)
+{
+    if (_shell_is_locked) {
+        printf("The shell is locked. Enter a valid password to unlock.\n\n");
+
+        _login_barrier(line_buf, buf_size);
+
+        if (IS_USED(MODULE_SHELL_LOCK_AUTO_LOCKING)) {
+            printf("Shell was unlocked.\n\n");
+        }
+        else {
+            printf("Shell was unlocked.\n\n"
+                   "IMPORTANT: Don't forget to lock the shell after usage, "
+                   "because it won't lock itself.\n\n");
+        }
+
+        _shell_is_locked = false;
+    }
+
+#ifdef MODULE_SHELL_LOCK_AUTO_LOCKING
+    _shell_auto_lock_ztimer.callback = &_shell_auto_lock_ztimer_callback;
+    shell_lock_auto_lock_refresh();
+#endif
+}

tests/shell_lock/Makefile

@@ -0,0 +1,32 @@
+DEVELHELP=0
+include ../Makefile.tests_common
+
+USEMODULE += shell
+USEMODULE += shell_commands
+
+USEMODULE += shell_lock
+USEMODULE += shell_lock_auto_locking
+
+CFLAGS += -DCONFIG_SHELL_LOCK_PASSWORD=\"password\"
+CFLAGS += -DCONFIG_SHELL_LOCK_AUTO_LOCK_TIMEOUT_MS=7000
+
+# This config defaults to 1 on native, such that pm_off() would be called as soon as
+# shell_run_once is terminated in shell_run_forever. We do not want this behavior for this test.
+CFLAGS += -DCONFIG_SHELL_SHUTDOWN_ON_EXIT=0
+
+# test_utils_interactive_sync_shell assumes that the prompt is always '> ' which breaks
+# with the password prompt of the shell_lock module which is different from the shell's prompt
+DISABLE_MODULE += test_utils_interactive_sync_shell
+
+# for z1, socat doesn't work (unknown reason)
+ifeq (z1, $(BOARD))
+  RIOT_TERMINAL ?= pyterm
+endif
+
+# Use a terminal that does not introduce extra characters into the stream.
+RIOT_TERMINAL ?= socat
+
+include $(RIOTBASE)/Makefile.include
+
+# the test script skips tests if socat is not used
+$(call target-export-variables,$(RIOT_TERMINAL),RIOT_TERMINAL)

tests/shell_lock/Makefile.ci

@@ -0,0 +1,8 @@
+BOARD_INSUFFICIENT_MEMORY := \
+    arduino-duemilanove \
+    arduino-leonardo \
+    arduino-nano \
+    arduino-uno \
+    atmega328p \
+    nucleo-l011k4 \
+    #

tests/shell_lock/main.c

@@ -0,0 +1,36 @@
+/*
+ * Copyright (C) 2021 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.
+ */
+
+/**
+ * @file
+ * @brief       Test for the lock behaviour of the shell
+ *
+ * @author      Hendrik van Essen <hendrik.ve@fu-berlin.de>
+ *
+ */
+
+#include <stdio.h>
+
+#include "shell.h"
+
+#include "test_utils/interactive_sync.h"
+
+int main(void)
+{
+    test_utils_interactive_sync();
+
+    puts("test_shell_lock");
+
+    /* define buffer to be used by the shell */
+    char line_buf[SHELL_DEFAULT_BUFSIZE];
+
+    /* start shell */
+    shell_run(NULL, line_buf, SHELL_DEFAULT_BUFSIZE);
+
+    return 0;
+}

tests/shell_lock/tests/01-run.py

@@ -0,0 +1,99 @@
+#!/usr/bin/env python3
+
+# Copyright (C) 2021 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.
+
+import os
+import sys
+import time
+
+from testrunner import run
+
+PASSWORD_CORRECT = "password"
+PASSWORDS_INCORRECT = [
+    "pass",
+    "word",
+    "asswor",
+    "passw0rd",
+    "password_",
+    "_password"
+]
+
+EXPECTED_HELP = (
+    'Command              Description',
+    '---------------------------------------',
+    'lock                 Lock the shell',
+    'pm                   interact with layered PM subsystem',
+    'reboot               Reboot the node',
+    'version              Prints current RIOT_VERSION',
+)
+
+AUTO_LOCK_TIMEOUT_MS = 7000
+SHELL_PROMPT = '> '
+PASSWORD_PROMPT = 'Password: '
+BOARD = os.environ['BOARD']
+
+
+def testfunc(child):
+
+    # avoid sending an extra empty line on native.
+    if BOARD == 'native':
+        child.crlf = '\n'
+
+    # unlock
+    child.sendline(PASSWORD_CORRECT)
+    child.expect_exact('Shell was unlocked.')
+    child.expect_exact(SHELL_PROMPT)
+
+    # check we have access
+    child.sendline('help')
+    for line in EXPECTED_HELP:
+        child.expect_exact(line)
+
+    # lock
+    child.sendline('lock')
+    child.expect(SHELL_PROMPT)
+
+    # trigger password prompt
+    child.sendline('help')
+    child.expect('The shell is locked. Enter a valid password to unlock.')
+
+    # test different incorrect passwords
+    for i, pwd in enumerate(PASSWORDS_INCORRECT):
+
+        # every third incorrect attempt leads to 7 second of sleep, otherwise
+        # just 1 second
+        if i > 0 and i % 3 == 0:
+            timeout = 7
+        else:
+            timeout = 1
+
+        # some boards react quite slow, give them 2 extra seconds
+        child.expect_exact(PASSWORD_PROMPT, timeout=(timeout + 2))
+        child.sendline(pwd)
+        child.expect_exact('Wrong password')
+
+    # unlock
+    child.sendline(PASSWORD_CORRECT)
+    child.expect_exact('Shell was unlocked.')
+    child.expect_exact(SHELL_PROMPT)
+
+    # check we have access
+    child.sendline('help')
+    for line in EXPECTED_HELP:
+        child.expect_exact(line)
+
+    # wait until auto_lock locks the shell after
+    # CONFIG_SHELL_LOCK_AUTO_LOCK_TIMEOUT_MS (+ 1 second buffer time)
+    time.sleep((AUTO_LOCK_TIMEOUT_MS / 1000.0) + 1)
+
+    # trigger password prompt
+    child.sendline('help')
+    child.expect('The shell is locked. Enter a valid password to unlock.')
+
+
+if __name__ == "__main__":
+    sys.exit(run(testfunc))