From a83d4cbced91dac7a79bd14ce44ec183cd77ba8a Mon Sep 17 00:00:00 2001 From: Agoston Szepessy Date: Sun, 21 Jul 2024 14:15:18 +0200 Subject: [PATCH 1/4] Implement getpass() Also make fields in `termios` public; required for modifying them. There's an new type of test: `EXPECT_INPUT_BINS`. These require a `.exp` file to be present along with the `.c` file. The `.exp` file takes the produced binary as an argument and sends input to the program. This is useful for testing functions like `getpass()`. --- src/header/limits/mod.rs | 1 + src/header/termios/mod.rs | 30 +++++++++++----------- src/header/unistd/mod.rs | 52 ++++++++++++++++++++++++++++++++++++--- tests/Makefile | 18 +++++++++++++- tests/unistd/getpass.c | 39 +++++++++++++++++++++++++++++ tests/unistd/getpass.exp | 18 ++++++++++++++ 6 files changed, 138 insertions(+), 20 deletions(-) create mode 100644 tests/unistd/getpass.c create mode 100644 tests/unistd/getpass.exp diff --git a/src/header/limits/mod.rs b/src/header/limits/mod.rs index 0c08687e8b..80cfa1904b 100644 --- a/src/header/limits/mod.rs +++ b/src/header/limits/mod.rs @@ -2,3 +2,4 @@ pub const PATH_MAX: usize = 4096; pub const NGROUPS_MAX: usize = 65536; +pub const PASS_MAX: usize = 128; diff --git a/src/header/termios/mod.rs b/src/header/termios/mod.rs index e1dc03feb1..7d2ad46ff7 100644 --- a/src/header/termios/mod.rs +++ b/src/header/termios/mod.rs @@ -37,28 +37,28 @@ pub const TCSAFLUSH: c_int = 2; #[cfg(target_os = "linux")] #[repr(C)] -#[derive(Default)] +#[derive(Default, Clone)] pub struct termios { - c_iflag: tcflag_t, - c_oflag: tcflag_t, - c_cflag: tcflag_t, - c_lflag: tcflag_t, - c_line: cc_t, - c_cc: [cc_t; NCCS], - __c_ispeed: speed_t, - __c_ospeed: speed_t, + pub c_iflag: tcflag_t, + pub c_oflag: tcflag_t, + pub c_cflag: tcflag_t, + pub c_lflag: tcflag_t, + pub c_line: cc_t, + pub c_cc: [cc_t; NCCS], + pub __c_ispeed: speed_t, + pub __c_ospeed: speed_t, } // Must match structure in redox_termios #[cfg(target_os = "redox")] #[repr(C)] -#[derive(Default)] +#[derive(Default, Clone)] pub struct termios { - c_iflag: tcflag_t, - c_oflag: tcflag_t, - c_cflag: tcflag_t, - c_lflag: tcflag_t, - c_cc: [cc_t; NCCS], + pub c_iflag: tcflag_t, + pub c_oflag: tcflag_t, + pub c_cflag: tcflag_t, + pub c_lflag: tcflag_t, + pub c_cc: [cc_t; NCCS], } #[no_mangle] diff --git a/src/header/unistd/mod.rs b/src/header/unistd/mod.rs index 6989ee525c..1bc169d951 100644 --- a/src/header/unistd/mod.rs +++ b/src/header/unistd/mod.rs @@ -11,7 +11,9 @@ use crate::{ c_str::CStr, header::{ crypt::{crypt_data, crypt_r}, - errno, fcntl, limits, + errno, fcntl, + limits::{self, PASS_MAX}, + stdio, stdlib::getenv, sys_ioctl, sys_resource, sys_time, sys_utsname, termios, time::timespec, @@ -450,9 +452,51 @@ pub extern "C" fn getpagesize() -> c_int { .expect("page size not representable as type `int`") } -// #[no_mangle] -pub extern "C" fn getpass(prompt: *const c_char) -> *mut c_char { - unimplemented!(); +#[no_mangle] +pub unsafe extern "C" fn getpass(prompt: *const c_char) -> *mut c_char { + let tty = stdio::fopen(c_str!("/dev/tty").as_ptr(), c_str!("w+e").as_ptr()); + + if tty.is_null() { + return ptr::null_mut(); + } + + let fd = stdio::fileno(tty); + + let mut term = termios::termios::default(); + termios::tcgetattr(fd, &mut term as *mut termios::termios); + let old_temr = term.clone(); + + term.c_iflag &= !(termios::IGNCR | termios::INLCR) as u32; + term.c_iflag |= termios::ICRNL as u32; + term.c_lflag &= !(termios::ECHO | termios::ISIG) as u32; + term.c_lflag |= termios::ICANON as u32; + + termios::tcsetattr(fd, termios::TCSAFLUSH, &term as *const termios::termios); + stdio::fputs(prompt, tty); + stdio::fflush(tty); + + static mut PASSBUFF: [c_char; PASS_MAX] = [0; PASS_MAX]; + + let len = read(fd, PASSBUFF.as_mut_ptr() as *const c_void, PASSBUFF.len()); + + if len >= 0 { + let mut l = len as usize; + if PASSBUFF[l - 1] == b'\n' as c_char || PASSBUFF.len() == l { + l -= 1; + } + + PASSBUFF[l] = 0; + } + + termios::tcsetattr(fd, termios::TCSAFLUSH, &old_temr as *const termios::termios); + stdio::fputs(c_str!("\n").as_ptr(), tty); + stdio::fclose(tty); + + if len < 0 { + return ptr::null_mut(); + } + + PASSBUFF.as_mut_ptr() } #[no_mangle] diff --git a/tests/Makefile b/tests/Makefile index 37abe58dbb..636df83cc7 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -196,6 +196,11 @@ NAMES=\ # resource/getrusage # time/times +# Tests run with `expect` (require a .c file and an .exp file +# that takes the produced binary as the first argument) +EXPECT_INPUT_NAMES=\ + unistd/getpass + #TODO: dynamic tests currently broken BINS=$(patsubst %,$(BUILD)/bins_static/%,$(NAMES)) #BINS+=$(patsubst %,bins_dynamic/%,$(NAMES)) @@ -203,6 +208,7 @@ BINS=$(patsubst %,$(BUILD)/bins_static/%,$(NAMES)) EXPECT_BINS=$(patsubst %,$(BUILD)/bins_static/%,$(EXPECT_NAMES)) #EXPECT_BINS+=$(patsubst %,bins_dynamic/%,$(EXPECT_NAMES)) #EXPECT_BINS+=$(patsubst %,bins_dynamic/%,$(DYNAMIC_ONLY_NAMES)) +EXPECT_INPUT_BINS=$(patsubst %,$(BUILD)/bins_expect_input/%,$(EXPECT_INPUT_NAMES)) CARGO_TEST?=cargo TEST_RUNNER?= @@ -214,12 +220,17 @@ all: $(BINS) clean: rm -rf bins_* gen *.out -run: | $(BINS) +run: | $(BINS) $(EXPECT_INPUT_BINS) for bin in $(BINS); \ do \ echo "# $${bin} #"; \ ${TEST_RUNNER} "$${bin}" test args || exit $$?; \ done + for exp in $(EXPECT_INPUT_BINS); \ + do \ + echo "# expect $$(readlink -e $${exp}.exp) $$(readlink -e $${exp}) #"; \ + expect "$$(readlink -e $${exp}.exp)" "$$(readlink -e $${exp})" test args || exit $$?; \ + done expected: | $(EXPECT_BINS) rm -rf expected @@ -289,6 +300,11 @@ $(BUILD)/bins_static/%: %.c $(DEPS) mkdir -p "$$(dirname "$@")" $(CC) "$<" -o "$@" $(FLAGS) $(STATIC_FLAGS) +$(BUILD)/bins_expect_input/%: %.c %.exp $(DEPS) + mkdir -p "$$(dirname "$@")" + $(CC) "$<" -o "$@" $(FLAGS) $(STATIC_FLAGS) + cp $(word 2, "$^") $(addsuffix .exp,"$@") + $(BUILD)/bins_dynamic/%.so: %.c $(DEPS) mkdir -p "$$(dirname "$@")" $(CC) "$<" -o "$@" -shared -fpic $(FLAGS) $(DYNAMIC_FLAGS) diff --git a/tests/unistd/getpass.c b/tests/unistd/getpass.c new file mode 100644 index 0000000000..07c8e5c9a5 --- /dev/null +++ b/tests/unistd/getpass.c @@ -0,0 +1,39 @@ +#include +#include +#include +#include + +// #include "test_helpers.h" + +int main(void) +{ + const char *pass = "pass"; + const char *prompt = "Enter password: "; + + char *result = getpass(prompt); + + if(strcmp(pass, result)) { + printf("incorrect password\n"); + exit(EXIT_FAILURE); + } + + const char *pass_127_chars = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + result = getpass(prompt); + + if(strcmp(pass_127_chars, result)) { + printf("incorrect password\n"); + exit(EXIT_FAILURE); + } + + const char *pass_empty = ""; + result = getpass(prompt); + + if(strcmp(pass_empty, result)) { + printf("incorrect password\n"); + exit(EXIT_FAILURE); + } + + printf("matching passwords\n", result); + + return 0; +} \ No newline at end of file diff --git a/tests/unistd/getpass.exp b/tests/unistd/getpass.exp new file mode 100644 index 0000000000..a707a04655 --- /dev/null +++ b/tests/unistd/getpass.exp @@ -0,0 +1,18 @@ +#!/usr/bin/expect + +set testgetpass [lindex $argv 0]; + +spawn $testgetpass +expect "Enter password: " +send -- "pass\r" + +expect "Enter password: " +send -- "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\r" + +expect "Enter password: " +send -- "\r" + +expect { + "incorrect password" { exit 123 } + eof +} \ No newline at end of file From 545e9e7b2925b369a7c5125a812b6ebc26f59ff2 Mon Sep 17 00:00:00 2001 From: Agoston Szepessy Date: Tue, 23 Jul 2024 16:06:14 +0200 Subject: [PATCH 2/4] Implement getpass using File API Returns a Result for better error handling. --- src/header/unistd/getpass.rs | 81 ++++++++++++++++++++++++++++++++++++ src/header/unistd/mod.rs | 55 ++---------------------- 2 files changed, 85 insertions(+), 51 deletions(-) create mode 100644 src/header/unistd/getpass.rs diff --git a/src/header/unistd/getpass.rs b/src/header/unistd/getpass.rs new file mode 100644 index 0000000000..24e7c14cec --- /dev/null +++ b/src/header/unistd/getpass.rs @@ -0,0 +1,81 @@ +use core::ptr; + +use alloc::vec::Vec; + +use crate::{ + fs::File, + header::{ + fcntl::{O_CLOEXEC, O_RDWR}, + limits::PASS_MAX, + stdio, termios, + }, + io::{self, Read}, +}; + +use crate::platform::types::*; + +#[derive(Debug)] +enum Error { + Io(io::Error), + CannotConvertFd, +} + +impl From for Error { + fn from(value: io::Error) -> Self { + Error::Io(value) + } +} + +unsafe fn getpass_rs(prompt: *const c_char) -> Result<*mut c_char, Error> { + let mut f = File::open(c_str!("/dev/tty"), O_RDWR | O_CLOEXEC)?; + + let mut term = termios::termios::default(); + termios::tcgetattr(f.fd, &mut term as *mut termios::termios); + let old_term = term.clone(); + + term.c_iflag &= !(termios::IGNCR | termios::INLCR) as u32; + term.c_iflag |= termios::ICRNL as u32; + term.c_lflag &= !(termios::ECHO | termios::ISIG) as u32; + term.c_lflag |= termios::ICANON as u32; + + let cfile = stdio::fdopen(f.fd, c_str!("w+e").as_ptr()); + + if cfile.is_null() { + return Err(Error::CannotConvertFd); + } + + termios::tcsetattr(f.fd, termios::TCSAFLUSH, &term as *const termios::termios); + stdio::fputs(prompt, cfile); + stdio::fflush(cfile); + + let mut buff = Vec::new(); + let mut len = f.read(&mut buff)?; + + static mut PASSBUFF: [c_char; PASS_MAX] = [0; PASS_MAX]; + + for (dst, src) in PASSBUFF.iter_mut().zip(&buff) { + *dst = *src as c_char; + } + + if len > 0 { + if PASSBUFF[len - 1] == b'\n' as c_char || PASSBUFF.len() == len { + len -= 1; + } + } + + PASSBUFF[len] = 0; + + termios::tcsetattr( + f.fd, + termios::TCSAFLUSH, + &old_term as *const termios::termios, + ); + stdio::fputs(c_str!("\n").as_ptr(), cfile); + + Ok(PASSBUFF.as_mut_ptr()) +} + +#[no_mangle] +pub extern "C" fn getpass(prompt: *const c_char) -> *mut c_char { + unsafe { getpass_rs(prompt).unwrap_or(ptr::null_mut()) } +} diff --git a/src/header/unistd/mod.rs b/src/header/unistd/mod.rs index 1bc169d951..da7977e93a 100644 --- a/src/header/unistd/mod.rs +++ b/src/header/unistd/mod.rs @@ -11,9 +11,7 @@ use crate::{ c_str::CStr, header::{ crypt::{crypt_data, crypt_r}, - errno, fcntl, - limits::{self, PASS_MAX}, - stdio, + errno, fcntl, limits, stdlib::getenv, sys_ioctl, sys_resource, sys_time, sys_utsname, termios, time::timespec, @@ -21,14 +19,16 @@ use crate::{ platform::{self, types::*, Pal, Sys}, pthread::ResultExt, }; + use alloc::collections::LinkedList; -pub use self::{brk::*, getopt::*, pathconf::*, sysconf::*}; +pub use self::{brk::*, getopt::*, getpass::getpass, pathconf::*, sysconf::*}; use super::errno::{E2BIG, ENOMEM}; mod brk; mod getopt; +mod getpass; mod pathconf; mod sysconf; @@ -452,53 +452,6 @@ pub extern "C" fn getpagesize() -> c_int { .expect("page size not representable as type `int`") } -#[no_mangle] -pub unsafe extern "C" fn getpass(prompt: *const c_char) -> *mut c_char { - let tty = stdio::fopen(c_str!("/dev/tty").as_ptr(), c_str!("w+e").as_ptr()); - - if tty.is_null() { - return ptr::null_mut(); - } - - let fd = stdio::fileno(tty); - - let mut term = termios::termios::default(); - termios::tcgetattr(fd, &mut term as *mut termios::termios); - let old_temr = term.clone(); - - term.c_iflag &= !(termios::IGNCR | termios::INLCR) as u32; - term.c_iflag |= termios::ICRNL as u32; - term.c_lflag &= !(termios::ECHO | termios::ISIG) as u32; - term.c_lflag |= termios::ICANON as u32; - - termios::tcsetattr(fd, termios::TCSAFLUSH, &term as *const termios::termios); - stdio::fputs(prompt, tty); - stdio::fflush(tty); - - static mut PASSBUFF: [c_char; PASS_MAX] = [0; PASS_MAX]; - - let len = read(fd, PASSBUFF.as_mut_ptr() as *const c_void, PASSBUFF.len()); - - if len >= 0 { - let mut l = len as usize; - if PASSBUFF[l - 1] == b'\n' as c_char || PASSBUFF.len() == l { - l -= 1; - } - - PASSBUFF[l] = 0; - } - - termios::tcsetattr(fd, termios::TCSAFLUSH, &old_temr as *const termios::termios); - stdio::fputs(c_str!("\n").as_ptr(), tty); - stdio::fclose(tty); - - if len < 0 { - return ptr::null_mut(); - } - - PASSBUFF.as_mut_ptr() -} - #[no_mangle] pub extern "C" fn getpgid(pid: pid_t) -> pid_t { Sys::getpgid(pid) From 99ade44c1213eb07a2236c501198b004eda54cb1 Mon Sep 17 00:00:00 2001 From: Agoston Szepessy Date: Tue, 23 Jul 2024 16:53:56 +0200 Subject: [PATCH 3/4] PR feedback - Remove `stdio` functions and use `File` API completely. - Remove `unsafe` from wrapper function. - Move static buffer to `getpass()` instead of keeping it in wrapper. --- src/header/unistd/getpass.rs | 74 +++++++++++++++--------------------- 1 file changed, 30 insertions(+), 44 deletions(-) diff --git a/src/header/unistd/getpass.rs b/src/header/unistd/getpass.rs index 24e7c14cec..da54c3d317 100644 --- a/src/header/unistd/getpass.rs +++ b/src/header/unistd/getpass.rs @@ -1,36 +1,27 @@ use core::ptr; -use alloc::vec::Vec; - use crate::{ + c_str::CStr, fs::File, header::{ fcntl::{O_CLOEXEC, O_RDWR}, limits::PASS_MAX, - stdio, termios, + termios, }, - io::{self, Read}, + io::{self, Read, Write}, }; use crate::platform::types::*; -#[derive(Debug)] -enum Error { - Io(io::Error), - CannotConvertFd, -} - -impl From for Error { - fn from(value: io::Error) -> Self { - Error::Io(value) - } -} - -unsafe fn getpass_rs(prompt: *const c_char) -> Result<*mut c_char, Error> { +fn getpass_rs(prompt: CStr, passbuff: &mut [c_char]) -> Result<*mut c_char, io::Error> { let mut f = File::open(c_str!("/dev/tty"), O_RDWR | O_CLOEXEC)?; let mut term = termios::termios::default(); - termios::tcgetattr(f.fd, &mut term as *mut termios::termios); + + unsafe { + termios::tcgetattr(f.fd, &mut term as *mut termios::termios); + } + let old_term = term.clone(); term.c_iflag &= !(termios::IGNCR | termios::INLCR) as u32; @@ -38,44 +29,39 @@ unsafe fn getpass_rs(prompt: *const c_char) -> Result<*mut c_char, Error> { term.c_lflag &= !(termios::ECHO | termios::ISIG) as u32; term.c_lflag |= termios::ICANON as u32; - let cfile = stdio::fdopen(f.fd, c_str!("w+e").as_ptr()); - - if cfile.is_null() { - return Err(Error::CannotConvertFd); + unsafe { + termios::tcsetattr(f.fd, termios::TCSAFLUSH, &term as *const termios::termios); } - termios::tcsetattr(f.fd, termios::TCSAFLUSH, &term as *const termios::termios); - stdio::fputs(prompt, cfile); - stdio::fflush(cfile); + f.write(&prompt.to_bytes())?; - let mut buff = Vec::new(); - let mut len = f.read(&mut buff)?; - - static mut PASSBUFF: [c_char; PASS_MAX] = [0; PASS_MAX]; - - for (dst, src) in PASSBUFF.iter_mut().zip(&buff) { - *dst = *src as c_char; - } + let buff = unsafe { &mut *(passbuff as *mut [i8] as *mut [u8]) }; + let mut len = f.read(buff)?; if len > 0 { - if PASSBUFF[len - 1] == b'\n' as c_char || PASSBUFF.len() == len { + if passbuff[len - 1] == b'\n' as c_char || passbuff.len() == len { len -= 1; } } - PASSBUFF[len] = 0; + passbuff[len] = 0; - termios::tcsetattr( - f.fd, - termios::TCSAFLUSH, - &old_term as *const termios::termios, - ); - stdio::fputs(c_str!("\n").as_ptr(), cfile); + unsafe { + termios::tcsetattr( + f.fd, + termios::TCSAFLUSH, + &old_term as *const termios::termios, + ); + } - Ok(PASSBUFF.as_mut_ptr()) + f.write(b"\n")?; + + Ok(passbuff.as_mut_ptr()) } #[no_mangle] -pub extern "C" fn getpass(prompt: *const c_char) -> *mut c_char { - unsafe { getpass_rs(prompt).unwrap_or(ptr::null_mut()) } +pub unsafe extern "C" fn getpass(prompt: *const c_char) -> *mut c_char { + static mut PASSBUFF: [c_char; PASS_MAX] = [0; PASS_MAX]; + + unsafe { getpass_rs(CStr::from_ptr(prompt), &mut PASSBUFF).unwrap_or(ptr::null_mut()) } } From b5f767ea30b4d118658692bd1d19ddcd6e0ac039 Mon Sep 17 00:00:00 2001 From: Agoston Szepessy Date: Tue, 23 Jul 2024 18:07:09 +0200 Subject: [PATCH 4/4] More PR feedback Use u8 internally instead of `c_char`. --- src/header/unistd/getpass.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/header/unistd/getpass.rs b/src/header/unistd/getpass.rs index da54c3d317..6776cad85b 100644 --- a/src/header/unistd/getpass.rs +++ b/src/header/unistd/getpass.rs @@ -13,7 +13,7 @@ use crate::{ use crate::platform::types::*; -fn getpass_rs(prompt: CStr, passbuff: &mut [c_char]) -> Result<*mut c_char, io::Error> { +fn getpass_rs(prompt: CStr, passbuff: &mut [u8]) -> Result<*mut c_char, io::Error> { let mut f = File::open(c_str!("/dev/tty"), O_RDWR | O_CLOEXEC)?; let mut term = termios::termios::default(); @@ -34,12 +34,12 @@ fn getpass_rs(prompt: CStr, passbuff: &mut [c_char]) -> Result<*mut c_char, io:: } f.write(&prompt.to_bytes())?; + f.flush()?; - let buff = unsafe { &mut *(passbuff as *mut [i8] as *mut [u8]) }; - let mut len = f.read(buff)?; + let mut len = f.read(passbuff)?; if len > 0 { - if passbuff[len - 1] == b'\n' as c_char || passbuff.len() == len { + if passbuff[len - 1] == b'\n' || passbuff.len() == len { len -= 1; } } @@ -55,13 +55,14 @@ fn getpass_rs(prompt: CStr, passbuff: &mut [c_char]) -> Result<*mut c_char, io:: } f.write(b"\n")?; + f.flush()?; - Ok(passbuff.as_mut_ptr()) + Ok(passbuff.as_mut_ptr() as *mut c_char) } #[no_mangle] pub unsafe extern "C" fn getpass(prompt: *const c_char) -> *mut c_char { - static mut PASSBUFF: [c_char; PASS_MAX] = [0; PASS_MAX]; + static mut PASSBUFF: [u8; PASS_MAX] = [0; PASS_MAX]; unsafe { getpass_rs(CStr::from_ptr(prompt), &mut PASSBUFF).unwrap_or(ptr::null_mut()) } }