From f8351aef302fa2da6183096fdf0013b2f29900ae Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Fri, 14 Aug 2020 23:35:47 +0200 Subject: upgrade to C++17 --- Makefile | 2 +- ssh-agent-filter.C | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 9c6a8a4..6fbf4e1 100644 --- a/Makefile +++ b/Makefile @@ -19,7 +19,7 @@ CXXFLAGS ?= -g -O2 -Wall -Wold-style-cast CPPFLAGS += -D_FILE_OFFSET_BITS=64 -CXXFLAGS += -std=c++11 +CXXFLAGS += -std=c++17 LDLIBS = -lstdc++ -lboost_program_options -lboost_filesystem -lboost_system -lboost_iostreams -lnettle -lpthread all: ssh-agent-filter.1 afssh.1 ssh-askpass-noinput.1 diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index b6d906b..340c606 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -499,8 +499,7 @@ rfc4251::string handle_request (rfc4251::string const & r) { if (allowed_pubkeys.count(key)) allow = true; else { - auto it = confirmed_pubkeys.find(key); - if (it != confirmed_pubkeys.end()) { + if (auto it = confirmed_pubkeys.find(key); it != confirmed_pubkeys.end()) { string request_description; bool dissect_ok{false}; if (!dissect_ok) -- cgit v1.2.3 From 013ca71f8e07f462266f5c795930739b37c6d5fb Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Mon, 24 Aug 2020 13:42:39 +0200 Subject: use C++17 std::filesystem instead of boost --- Makefile | 2 +- ssh-agent-filter.C | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 6fbf4e1..cd27012 100644 --- a/Makefile +++ b/Makefile @@ -20,7 +20,7 @@ CXXFLAGS ?= -g -O2 -Wall -Wold-style-cast CPPFLAGS += -D_FILE_OFFSET_BITS=64 CXXFLAGS += -std=c++17 -LDLIBS = -lstdc++ -lboost_program_options -lboost_filesystem -lboost_system -lboost_iostreams -lnettle -lpthread +LDLIBS = -lstdc++ -lboost_program_options -lboost_iostreams -lnettle -lpthread all: ssh-agent-filter.1 afssh.1 ssh-askpass-noinput.1 diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index 340c606..e98123c 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -22,8 +22,8 @@ #include namespace po = boost::program_options; -#include -namespace fs = boost::filesystem; +#include +namespace fs = std::filesystem; #include #include -- cgit v1.2.3 From d17162f5c6f7d002bf68fac48bcadd2db0c033e7 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Sun, 18 Apr 2021 00:02:42 +0200 Subject: use C++17 std::optional for dissectors, refactor --- ssh-agent-filter.C | 63 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index e98123c..c513804 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -64,6 +64,8 @@ using std::pair; using std::mutex; using std::lock_guard; +#include + #include #include @@ -326,9 +328,10 @@ bool confirm (string const & question) { } } -bool dissect_auth_data_ssh_cert (rfc4251::string const & data, string & request_description) try { +std::optional dissect_auth_data_ssh_cert (rfc4251::string const & data) try { io::stream datastream{data.data(), data.size()}; arm(datastream); + string request_description{}; // Format specified in https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL.certkeys?annotate=1.13 rfc4251::string keytype{datastream}; @@ -337,10 +340,10 @@ bool dissect_auth_data_ssh_cert (rfc4251::string const & data, string & request_ // check for and remove suffix to get the base keytype std::string const suffix{"-cert-v01@openssh.com"}; if (keytype_str.length() <= suffix.length()) - return false; + return {}; auto suffix_start = keytype_str.end() - suffix.length(); if (!std::equal(suffix.begin(), suffix.end(), suffix_start)) - return false; + return {}; keytype_str.erase(suffix_start, keytype_str.end()); } rfc4251::string nonce{datastream}; @@ -365,7 +368,7 @@ bool dissect_auth_data_ssh_cert (rfc4251::string const & data, string & request_ rfc4251::string pk{datastream}; key_to_be_signed << rfc4251::string{keytype_str} << pk; } else { - return false; + return {}; } rfc4251::uint64 serial{datastream}; rfc4251::uint32 type{datastream}; @@ -380,14 +383,15 @@ bool dissect_auth_data_ssh_cert (rfc4251::string const & data, string & request_ request_description = "The request is for a certificate signature on key " + base64_encode(key_to_be_signed.str()) + "."; - return true; + return request_description; } catch (...) { - return false; + return {}; } -bool dissect_auth_data_ssh (rfc4251::string const & data, string & request_description) try { +std::optional dissect_auth_data_ssh (rfc4251::string const & data) try { io::stream datastream{data.data(), data.size()}; arm(datastream); + string request_description{}; // Format specified in RFC 4252 Section 7 rfc4251::string session_identifier{datastream}; @@ -450,11 +454,25 @@ bool dissect_auth_data_ssh (rfc4251::string const & data, string & request_descr } } catch (...) {} - return true; + return request_description; } catch (...) { - return false; + return {}; +} + +string describe_sign_request (rfc4251::string const & data_to_be_signed) { + auto const dissectors = { + dissect_auth_data_ssh_cert, + dissect_auth_data_ssh, + }; + std::optional request_description; + for (auto const dissector : dissectors) + if (request_description = dissector(data_to_be_signed); request_description) + break; + + return request_description.value_or("The request format is unknown."); } + rfc4251::string handle_request (rfc4251::string const & r) { io::stream request{r.data(), r.size()}; rfc4251::string ret; @@ -492,30 +510,19 @@ rfc4251::string handle_request (rfc4251::string const & r) { case SSH2_AGENTC_SIGN_REQUEST: { rfc4251::string key{request}; - rfc4251::string data{request}; + rfc4251::string data_to_be_signed{request}; rfc4251::uint32 flags{request}; bool allow{false}; if (allowed_pubkeys.count(key)) allow = true; - else { - if (auto it = confirmed_pubkeys.find(key); it != confirmed_pubkeys.end()) { - string request_description; - bool dissect_ok{false}; - if (!dissect_ok) - dissect_ok = dissect_auth_data_ssh_cert(data, request_description); - if (!dissect_ok) - dissect_ok = dissect_auth_data_ssh(data, request_description); - if (!dissect_ok) - request_description = "The request format is unknown."; - - string question = "Something behind the ssh-agent-filter"; - if (saf_name.length()) - question += " named '" + saf_name + "'"; - question += " requested use of the key named '" + it->second + "'.\n"; - question += request_description; - allow = confirm(question); - } + else if (auto it = confirmed_pubkeys.find(key); it != confirmed_pubkeys.end()) { + string question = "Something behind the ssh-agent-filter"; + if (saf_name.length()) + question += " named '" + saf_name + "'"; + question += " requested use of the key named '" + it->second + "'.\n"; + question += describe_sign_request(data_to_be_signed); + allow = confirm(question); } if (allow) { -- cgit v1.2.3 From df63d26366b39ad8eb1c1ba5876b57b6a15208ed Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Thu, 29 Apr 2021 17:28:15 +0200 Subject: use C++17 constructor template argument deduction (CTAD) --- ssh-agent-filter.C | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index c513804..639e55c 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -61,8 +61,6 @@ using std::pair; #include #include -using std::mutex; -using std::lock_guard; #include @@ -103,7 +101,7 @@ bool debug{false}; bool all_confirmed{false}; string saf_name; fs::path path; -mutex fd_fork_mutex; +std::mutex fd_fork_mutex; string md5_hex (string const & s) { @@ -141,7 +139,7 @@ int make_upstream_agent_conn () { throw invalid_argument("no $SSH_AUTH_SOCK"); { - lock_guard lock{fd_fork_mutex}; + std::lock_guard lock{fd_fork_mutex}; if ((sock = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0)) == -1) throw system_error(errno, system_category(), "socket"); cloexec(sock); @@ -165,7 +163,7 @@ int make_listen_sock () { struct sockaddr_un addr; { - lock_guard lock{fd_fork_mutex}; + std::lock_guard lock{fd_fork_mutex}; if ((sock = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0)) == -1) throw system_error(errno, system_category(), "socket"); cloexec(sock); @@ -310,7 +308,7 @@ bool confirm (string const & question) { sap = "ssh-askpass"; pid_t pid; { - lock_guard lock{fd_fork_mutex}; + std::lock_guard lock{fd_fork_mutex}; pid = fork(); } if (pid < 0) @@ -636,7 +634,7 @@ int main (int const argc, char const * const * const argv) { select(listen_sock + 1, &fds, nullptr, nullptr, nullptr); int client_sock; { - lock_guard lock{fd_fork_mutex}; + std::lock_guard lock{fd_fork_mutex}; if ((client_sock = accept(listen_sock, nullptr, nullptr)) == -1) { if (errno == EAGAIN || errno == EWOULDBLOCK) continue; -- cgit v1.2.3 From 664ad8e38fd0f077e1fba5e69325fb3a36a75e19 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Sun, 18 Apr 2021 00:03:31 +0200 Subject: add a test for cert signature dissection --- tests | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests b/tests index 0e7e1e7..d222e7e 100755 --- a/tests +++ b/tests @@ -97,7 +97,8 @@ test_confirmation () { EOT chmod +x "$SHUNIT_TMPDIR/sap" assertTrue 'export SSH_ASKPASS="$SHUNIT_TMPDIR/sap"; sign_key_with_key_filtered key0 key1 --comment-confirmed key1' - assertSame "Something behind the ssh-agent-filter requested use of the key named 'key1'." "$(head -n1 "$SHUNIT_TMPDIR/sap_out")" + assertSame "Something behind the ssh-agent-filter requested use of the key named 'key1'." "$(sed 1!d "$SHUNIT_TMPDIR/sap_out")" + assertSame "The request is for a certificate signature on key " "$(sed '2!d;s/AAAA.*$//' "$SHUNIT_TMPDIR/sap_out")" } . shunit2 -- cgit v1.2.3 From b3989076c699ee2d3e01377d17edec9a8bd0b2c3 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Wed, 21 Apr 2021 13:26:03 +0200 Subject: add .gitignore --- .gitignore | 9 +++++++++ Makefile | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 .gitignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..e0ca177 --- /dev/null +++ b/.gitignore @@ -0,0 +1,9 @@ +*.1 +*.gcda +*.gcno +*.gcov +*.o +*.plist +*.swp +compile_commands.json +ssh-agent-filter diff --git a/Makefile b/Makefile index cd27012..6b0c42d 100644 --- a/Makefile +++ b/Makefile @@ -43,7 +43,7 @@ version.h: test ! -d .git || git describe | sed 's/^.*$$/#define SSH_AGENT_FILTER_VERSION "ssh-agent-filter \0"/' > $@ clean: - $(RM) *.1 ssh-agent-filter *.o + $(RM) *.1 ssh-agent-filter *.o *.gcda *.gcno *.gcov test ! -d .git || git checkout -f -- version.h .PHONY: version.h -- cgit v1.2.3 From 9d25142d506d1559b134d465ff0434ffbde7dca8 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Sun, 18 Apr 2021 00:04:19 +0200 Subject: rfc4251.H: use template for fixed-length-types --- rfc4251.H | 160 +++++++++++++++++++------------------------------------------- 1 file changed, 48 insertions(+), 112 deletions(-) diff --git a/rfc4251.H b/rfc4251.H index 7d4c0d3..fe8ccd4 100644 --- a/rfc4251.H +++ b/rfc4251.H @@ -10,7 +10,7 @@ * those structs contain the objects in their RFC 4251 representation, * conversions are provided via constructors and cast operators * - * Copyright (C) 2013-2015 Timo Weingärtner + * Copyright (C) 2013-2015,2021 Timo Weingärtner * * This file is part of ssh-agent-filter. * @@ -28,132 +28,69 @@ * along with ssh-agent-filter. If not, see . */ +#include +#include +#include #include #include #include #include #include -#include // ntohl() / htonl() #include #include namespace rfc4251 { -struct byte { - union { - uint8_t value; - char buf[1]; - }; - - byte () = default; - explicit byte (uint8_t v) : value(v) {} - inline explicit byte (std::istream &); - - operator uint8_t () const { return value; } -}; - -inline std::istream & operator>> (std::istream & is, byte & x) { - return is.read(x.buf, sizeof(x.buf)); -} - -inline std::ostream & operator<< (std::ostream & os, byte const & x) { - return os.write(x.buf, sizeof(x.buf)); -} - -inline byte::byte (std::istream & is) { - is >> *this; -} - -struct boolean { - union { - bool value; - char buf[1]; - }; - - boolean () = default; - explicit boolean (uint8_t v) : value(v) {} - inline explicit boolean (std::istream &); - - operator uint8_t () const { return value; } -}; - -inline std::istream & operator>> (std::istream & is, boolean & x) { - return is.read(x.buf, sizeof(x.buf)); -} - -inline std::ostream & operator<< (std::ostream & os, boolean const & x) { - return os.write(x.buf, sizeof(x.buf)); -} - -inline boolean::boolean (std::istream & is) { - is >> *this; -} +namespace internal { + template + constexpr inline auto host_to_network (T value) noexcept { + static_assert(std::is_unsigned_v, "shift is only supported on unsigned"); + + std::array ret; + for (auto it{rbegin(ret)}; it != rend(ret); ++it) { + *it = value & 0xff; + if constexpr (sizeof(T) > 1) value >>= 8; + } + return ret; + } -struct uint32 { - union { - uint32_t value; - char buf[4]; - }; + template + constexpr inline auto network_to_host (std::array const buf) noexcept { + static_assert(std::is_unsigned_v, "shift is only supported on unsigned"); - uint32 () = default; - explicit uint32 (uint32_t v) { value = htonl(v); } - inline explicit uint32 (std::istream &); + T ret{0}; + for (auto it{cbegin(buf)}; it != cend(buf); ++it) { + if constexpr (sizeof(T) > 1) ret <<= 8; + ret |= static_cast(*it); + } + return ret; + } - operator uint32_t () const { return ntohl(value); } -}; + template + struct fixed_length { + static_assert(std::is_unsigned_v, "support for signed types might need more work"); -inline std::istream & operator>> (std::istream & is, uint32 & x) { - return is.read(x.buf, sizeof(x.buf)); -} + std::array buf{}; -inline std::ostream & operator<< (std::ostream & os, uint32 const & x) { - return os.write(x.buf, sizeof(x.buf)); -} + constexpr fixed_length () noexcept = default; + constexpr explicit fixed_length (T const v) noexcept : buf{host_to_network(v)} {} + explicit fixed_length (std::istream & is) { is >> *this; } -inline uint32::uint32 (std::istream & is) { - is >> *this; -} + constexpr operator T () const noexcept { return network_to_host(buf); } -struct uint64 { - union { - uint64_t value; - char buf[8]; + friend std::istream & operator>> (std::istream & is, fixed_length & x) { + return is.read(x.buf.data(), x.buf.size()); + } + friend std::ostream & operator<< (std::ostream & os, fixed_length const & x) { + return os.write(x.buf.data(), x.buf.size()); + } }; - - uint64 () = default; - inline explicit uint64 (uint64_t v); - inline explicit uint64 (std::istream &); - - inline explicit operator uint64_t () const; -}; - -inline uint64::uint64 (uint64_t v) { - for (int_fast8_t i{7}; i >= 0; --i) { - buf[i] = v & 0xff; - v >>= 8; - } -} - -inline uint64::operator uint64_t () const { - uint64_t ret{0}; - for (uint_fast8_t i{0}; i < 8; ++i) { - ret <<= 8; - ret |= static_cast(buf[i]); - } - return ret; } -inline std::istream & operator>> (std::istream & is, uint64 & x) { - return is.read(x.buf, sizeof(x.buf)); -} - -inline std::ostream & operator<< (std::ostream & os, uint64 const & x) { - return os.write(x.buf, sizeof(x.buf)); -} - -inline uint64::uint64 (std::istream & is) { - is >> *this; -} +using byte = internal::fixed_length; +using boolean = internal::fixed_length; +using uint32 = internal::fixed_length; +using uint64 = internal::fixed_length; struct string : boost::totally_ordered { std::vector value; @@ -166,9 +103,9 @@ struct string : boost::totally_ordered { explicit string (mpz_class const & x) : string{x.get_mpz_t()} {} inline explicit string (std::istream &); - size_t size () const { return value.size(); } - char const * data () const { return value.data(); } - char * data () { return value.data(); } + size_t size () const noexcept { return value.size(); } + char const * data () const noexcept { return value.data(); } + char * data () noexcept { return value.data(); } operator std::string () const { return {value.begin(), value.end()}; } operator std::vector () const; @@ -183,8 +120,7 @@ inline string::string (char const * s, size_t l) { inline std::istream & operator>> (std::istream & is, string & s) { s.value.clear(); - uint32 len; - if (is >> len) { + if (uint32_t const len{uint32{is}}; is) { s.value.resize(len); is.read(s.value.data(), len); } -- cgit v1.2.3 From b929b048a1839c3da1cadf7f116b0f767f40267e Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Thu, 22 Apr 2021 23:29:10 +0200 Subject: implement separate rfc4251::(string|mpint|name_list) types --- rfc4251.C | 41 +++++++++---------- rfc4251.H | 116 +++++++++++++++++++++++++++++++---------------------- rfc4251_gmp.C | 51 +++++++++++------------ ssh-agent-filter.C | 14 +++---- 4 files changed, 120 insertions(+), 102 deletions(-) diff --git a/rfc4251.C b/rfc4251.C index 5ea0a93..05ef8b8 100644 --- a/rfc4251.C +++ b/rfc4251.C @@ -1,10 +1,10 @@ /* * rfc4251.C -- support for name-list type from RFC 4251, section 5 * - * These are the conversions between an rfc4251::string containing a name-list - * and vector. + * These are the conversions between an rfc4251::name_list and + * std::vector. * - * Copyright (C) 2013,2015 Timo Weingärtner + * Copyright (C) 2013,2015,2021 Timo Weingärtner * * This file is part of ssh-agent-filter. * @@ -26,34 +26,33 @@ namespace rfc4251 { -string::string (std::vector const & v) { +name_list::name_list (std::vector const & v) { for (auto it = v.begin(); it != v.end();) { if (it->size() == 0) throw std::length_error{"name of zero length"}; - if (value.size() + it->size() > std::numeric_limits::max()) - throw std::length_error{"32-bit limit for rfc4251::string exceeded"}; - value.insert(value.end(), it->data(), it->data() + it->size()); + check_length_against_limit(buf.size() + it->size()); + buf.insert(buf.end(), it->data(), it->data() + it->size()); ++it; if (it == v.end()) break; - value.push_back(','); + buf.push_back(','); } } -string::operator std::vector () const { - std::vector ret; - auto name_start = value.begin(); - if (name_start != value.end()) - for (auto it = name_start; ; ++it) { - if (it == value.end() or *it == ',') { - if (it == name_start) - throw std::length_error{"name of zero length"}; - ret.emplace_back(name_start, it); - name_start = it + 1; - } - if (it == value.end()) - break; +name_list::operator std::vector () const { + std::vector ret{}; + if (buf.empty()) return ret; + auto name_start = buf.begin(); + for (auto it = name_start; ; ++it) { + if (it == buf.end() or *it == ',') { + if (it == name_start) + throw std::length_error{"name of zero length"}; + ret.emplace_back(name_start, it); + name_start = it + 1; } + if (it == buf.end()) + break; + } return ret; } diff --git a/rfc4251.H b/rfc4251.H index fe8ccd4..6647d15 100644 --- a/rfc4251.H +++ b/rfc4251.H @@ -5,7 +5,9 @@ * rfc4251::boolean boolean * rfc4251::uint32 uint32 * rfc4251::uint64 uint64 - * rfc4251::string string, incl. mpint and name-list + * rfc4251::string string + * rfc4251::mpint mpint + * rfc4251::name_list name-list * * those structs contain the objects in their RFC 4251 representation, * conversions are provided via constructors and cast operators @@ -85,6 +87,48 @@ namespace internal { return os.write(x.buf.data(), x.buf.size()); } }; + + template + struct variable_length : boost::totally_ordered> { + using length_type = Length_Type; + static_assert(std::is_unsigned_v, "negative lengths are not supported"); + + std::vector buf; + + variable_length () = default; + explicit variable_length (std::istream & is) { is >> *this; } + + size_t size () const noexcept { return buf.size(); } + char const * data () const noexcept { return buf.data(); } + char * data () noexcept { return buf.data(); } + + friend std::istream & operator>> (std::istream & is, variable_length & v) { + if (length_type const len{fixed_length{is}}; is) { + v.buf.clear(); + v.buf.resize(len); + is.read(v.buf.data(), len); + } + return is; + } + friend std::ostream & operator<< (std::ostream & os, variable_length const & v) { + check_length_against_limit(v.buf.size()); + if (os << fixed_length{static_cast(v.buf.size())}) + os.write(v.buf.data(), v.buf.size()); + return os; + } + friend bool operator== (variable_length const & l, variable_length const & r) { + return l.buf == r.buf; + } + friend bool operator< (variable_length const & l, variable_length const & r) { + return l.buf < r.buf; + } + + constexpr static void check_length_against_limit(size_t const length) { + if (length > std::numeric_limits::max()) + throw std::length_error{"numeric limit for length field in rfc4251::variable_length type exceeded"}; + } + + }; } using byte = internal::fixed_length; @@ -92,59 +136,33 @@ using boolean = internal::fixed_length; using uint32 = internal::fixed_length; using uint64 = internal::fixed_length; -struct string : boost::totally_ordered { - std::vector value; - - string () = default; - inline explicit string (char const *, size_t); +struct string : internal::variable_length { + using variable_length::variable_length; + + inline explicit string (char const * s, size_t l) { + check_length_against_limit(l); + buf.insert(buf.end(), s, s + l); + } explicit string (std::string const & s) : string{s.data(), s.size()} {} - explicit string (std::vector const &); - explicit string (mpz_srcptr); - explicit string (mpz_class const & x) : string{x.get_mpz_t()} {} - inline explicit string (std::istream &); - size_t size () const noexcept { return value.size(); } - char const * data () const noexcept { return value.data(); } - char * data () noexcept { return value.data(); } + operator std::string () const { return {buf.cbegin(), buf.cend()}; } +}; - operator std::string () const { return {value.begin(), value.end()}; } +struct name_list : internal::variable_length { + using variable_length::variable_length; + + explicit name_list (std::vector const &); + operator std::vector () const; - operator mpz_class () const; }; -inline string::string (char const * s, size_t l) { - if (l > std::numeric_limits::max()) - throw std::length_error{"32-bit limit for rfc4251::string exceeded"}; - value.insert(value.end(), s, s + l); -} - -inline std::istream & operator>> (std::istream & is, string & s) { - s.value.clear(); - if (uint32_t const len{uint32{is}}; is) { - s.value.resize(len); - is.read(s.value.data(), len); - } - return is; -} - -inline std::ostream & operator<< (std::ostream & os, string const & s) { - if (s.value.size() > std::numeric_limits::max()) - throw std::length_error{"32-bit limit for rfc4251::string exceeded"}; - if (os << uint32{static_cast(s.value.size())}) - os.write(s.value.data(), s.value.size()); - return os; -} - -inline string::string (std::istream & is) { - is >> *this; -} - -inline bool operator== (string const & l, string const & r) { - return l.value == r.value; -} - -inline bool operator< (string const & l, string const & r) { - return l.value < r.value; -} +struct mpint : internal::variable_length { + using variable_length::variable_length; + + explicit mpint (mpz_srcptr); + explicit mpint (mpz_class const & x) : mpint{x.get_mpz_t()} {} + + operator mpz_class () const; +}; } diff --git a/rfc4251_gmp.C b/rfc4251_gmp.C index b4c369b..06b6cf6 100644 --- a/rfc4251_gmp.C +++ b/rfc4251_gmp.C @@ -1,9 +1,9 @@ /* - * rfc4251_gmp.C -- implements mpint/gmp conversions for rfc4251::string + * rfc4251_gmp.C -- implements mpint/mpz conversions for rfc4251::mpint * * these functions need linking against libgmp * - * Copyright (C) 2013,2015 Timo Weingärtner + * Copyright (C) 2013,2015,2021 Timo Weingärtner * * This file is part of ssh-agent-filter. * @@ -25,36 +25,37 @@ namespace rfc4251 { -string::string (mpz_srcptr x) { - if (mpz_sgn(x) == 0) - return; - - auto const import_positive = [] (mpz_srcptr x, std::vector & value) { +mpint::mpint (mpz_srcptr x) { + auto const import_positive = [] (mpz_srcptr x, std::vector & buf) { size_t bits{mpz_sizeinbase(x, 2)}; size_t bytes{(bits + 7) / 8}; size_t extrabyte{(bits % 8) == 0}; // need extra byte if MSB is 1 to keep it non-negative - if (bytes + extrabyte > std::numeric_limits::max()) - throw std::length_error{"32-bit limit for rfc4251::string exceeded"}; - value.resize(bytes + extrabyte); - value[0] = 0; - mpz_export(value.data() + extrabyte, nullptr, 1, 1, 1, 0, x); + check_length_against_limit(bytes + extrabyte); + buf.resize(bytes + extrabyte); + buf[0] = 0; + mpz_export(buf.data() + extrabyte, nullptr, 1, 1, 1, 0, x); }; - if (mpz_sgn(x) == 1) - import_positive(x, value); - else { - // handle two's complement: add 1, invert all bits - mpz_class tmp{x}; - tmp += 1; - import_positive(tmp.get_mpz_t(), value); - for (auto & i : value) - i ^= 0xff; + + switch (mpz_sgn(x)) { + case 0: + return; + case 1: + import_positive(x, buf); + return; + default: + // handle two's complement: add 1, invert all bits + mpz_class tmp{x}; + tmp += 1; + import_positive(tmp.get_mpz_t(), buf); + for (auto & i : buf) + i ^= 0xff; } } -string::operator mpz_class () const { - mpz_class ret; - mpz_import(ret.get_mpz_t(), value.size(), 1, 1, 1, 0, value.data()); - if (mpz_sizeinbase(ret.get_mpz_t(), 2) == value.size() * 8) { // negative +mpint::operator mpz_class () const { + mpz_class ret{}; + mpz_import(ret.get_mpz_t(), buf.size(), 1, 1, 1, 0, buf.data()); + if (mpz_sizeinbase(ret.get_mpz_t(), 2) == buf.size() * 8) { // negative mpz_com(ret.get_mpz_t(), ret.get_mpz_t()); ret += 1; mpz_neg(ret.get_mpz_t(), ret.get_mpz_t()); diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index 639e55c..c658b62 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -347,14 +347,14 @@ std::optional dissect_auth_data_ssh_cert (rfc4251::string const & data) rfc4251::string nonce{datastream}; std::ostringstream key_to_be_signed{}; if (keytype_str == "ssh-rsa") { - rfc4251::string e{datastream}; - rfc4251::string n{datastream}; + rfc4251::mpint e{datastream}; + rfc4251::mpint n{datastream}; key_to_be_signed << rfc4251::string{keytype_str} << e << n; } else if (keytype_str == "ssh-dss") { - rfc4251::string p{datastream}; - rfc4251::string q{datastream}; - rfc4251::string g{datastream}; - rfc4251::string y{datastream}; + rfc4251::mpint p{datastream}; + rfc4251::mpint q{datastream}; + rfc4251::mpint g{datastream}; + rfc4251::mpint y{datastream}; key_to_be_signed << rfc4251::string{keytype_str} << p << q << g << y; } else if (keytype_str == "ecdsa-sha2-nistp256" || keytype_str == "ecdsa-sha2-nistp384" @@ -474,7 +474,7 @@ string describe_sign_request (rfc4251::string const & data_to_be_signed) { rfc4251::string handle_request (rfc4251::string const & r) { io::stream request{r.data(), r.size()}; rfc4251::string ret; - io::stream>> answer{ret.value}; + io::stream>> answer{ret.buf}; arm(request); arm(answer); rfc4251::byte request_code{request}; -- cgit v1.2.3 From fc0e196e4fd302d681725c9e5549e34b93a91075 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Thu, 22 Apr 2021 23:31:12 +0200 Subject: use std:: containers instead of C-style arrays --- ssh-agent-filter.C | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index c658b62..cede337 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -108,17 +108,17 @@ string md5_hex (string const & s) { struct md5_ctx ctx; md5_init(&ctx); md5_update(&ctx, s.size(), reinterpret_cast(s.data())); - uint8_t bin[MD5_DIGEST_SIZE]; - md5_digest(&ctx, MD5_DIGEST_SIZE, bin); - char hex[BASE16_ENCODE_LENGTH(MD5_DIGEST_SIZE)]; - base16_encode_update(hex, MD5_DIGEST_SIZE, bin); - return {hex, sizeof(hex)}; + std::array bin; + md5_digest(&ctx, MD5_DIGEST_SIZE, bin.data()); + std::array hex; + base16_encode_update(hex.data(), MD5_DIGEST_SIZE, bin.data()); + return {begin(hex), end(hex)}; } string base64_encode (string const & s) { - char b64[BASE64_ENCODE_RAW_LENGTH(s.size())]; - base64_encode_raw(b64, s.size(), reinterpret_cast(s.data())); - return {b64, sizeof(b64)}; + std::vector b64(BASE64_ENCODE_RAW_LENGTH(s.size())); + base64_encode_raw(b64.data(), s.size(), reinterpret_cast(s.data())); + return {begin(b64), end(b64)}; } void cloexec (int fd) { @@ -315,9 +315,9 @@ bool confirm (string const & question) { throw runtime_error("fork()"); if (pid == 0) { // child - char const * args[3] = { sap, question.c_str(), nullptr }; + std::array const args{ sap, question.c_str(), nullptr }; // see execvp(3p) for cast rationale - execvp(sap, const_cast(args)); + execvp(sap, const_cast(args.data())); throw system_error(errno, system_category(), "exec"); } else { // parent -- cgit v1.2.3 From 145c64e6c4e4151e869104e12da71786b8c31932 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Thu, 22 Apr 2021 23:33:14 +0200 Subject: replace strcpy() with something clang-tidy doesn't complain about --- ssh-agent-filter.C | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index cede337..175fba2 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -132,12 +132,11 @@ void arm(std::ios & stream) { int make_upstream_agent_conn () { char const * path; - int sock; - struct sockaddr_un addr; if (!(path = getenv("SSH_AUTH_SOCK"))) throw invalid_argument("no $SSH_AUTH_SOCK"); + int sock; { std::lock_guard lock{fd_fork_mutex}; if ((sock = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0)) == -1) @@ -145,13 +144,13 @@ int make_upstream_agent_conn () { cloexec(sock); } - addr.sun_family = AF_UNIX; + struct sockaddr_un addr{AF_UNIX, {}}; - if (strlen(path) >= sizeof(addr.sun_path)) + if (auto len = strlen(path); len < sizeof(addr.sun_path)) + std::copy(path, path + len, addr.sun_path); + else throw length_error("$SSH_AUTH_SOCK too long"); - strcpy(addr.sun_path, path); - if (connect(sock, reinterpret_cast(&addr), sizeof(addr))) throw system_error(errno, system_category(), "connect"); @@ -160,8 +159,6 @@ int make_upstream_agent_conn () { int make_listen_sock () { int sock; - struct sockaddr_un addr; - { std::lock_guard lock{fd_fork_mutex}; if ((sock = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0)) == -1) @@ -172,13 +169,13 @@ int make_listen_sock () { if (fcntl(sock, F_SETFL, fcntl(sock, F_GETFL) | O_NONBLOCK)) throw system_error(errno, system_category(), "fcntl"); - addr.sun_family = AF_UNIX; + struct sockaddr_un addr{AF_UNIX, {}}; - if (path.native().length() >= sizeof(addr.sun_path)) + if (path.native().length() < sizeof(addr.sun_path)) + std::copy(path.native().begin(), path.native().end(), addr.sun_path); + else throw length_error("path for listen socket too long"); - strcpy(addr.sun_path, path.c_str()); - if (bind(sock, reinterpret_cast(&addr), sizeof(addr))) throw system_error(errno, system_category(), "bind"); -- cgit v1.2.3 From 1ed4729f59343b8041387b8802610c25de599085 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Sun, 25 Apr 2021 17:45:45 +0200 Subject: add const to most rfc4251:: variables --- ssh-agent-filter.C | 102 ++++++++++++++++++++++++++--------------------------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index 175fba2..750ae60 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -236,13 +236,13 @@ void setup_filters () { arm(agent); agent << rfc4251::string{string{SSH2_AGENTC_REQUEST_IDENTITIES}}; - rfc4251::string answer{agent}; + rfc4251::string const answer{agent}; io::stream answer_iss{answer.data(), answer.size()}; arm(answer_iss); - rfc4251::byte resp_code{answer_iss}; + rfc4251::byte const resp_code{answer_iss}; if (resp_code != SSH2_AGENT_IDENTITIES_ANSWER) throw runtime_error{"unexpected answer from ssh-agent"}; - rfc4251::uint32 keycount{answer_iss}; + rfc4251::uint32 const keycount{answer_iss}; for (uint32_t i = keycount; i; --i) { rfc4251::string key{answer_iss}; rfc4251::string comment{answer_iss}; @@ -329,7 +329,7 @@ std::optional dissect_auth_data_ssh_cert (rfc4251::string const & data) string request_description{}; // Format specified in https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL.certkeys?annotate=1.13 - rfc4251::string keytype{datastream}; + rfc4251::string const keytype{datastream}; std::string keytype_str{keytype}; { // check for and remove suffix to get the base keytype @@ -341,40 +341,40 @@ std::optional dissect_auth_data_ssh_cert (rfc4251::string const & data) return {}; keytype_str.erase(suffix_start, keytype_str.end()); } - rfc4251::string nonce{datastream}; + rfc4251::string const nonce{datastream}; std::ostringstream key_to_be_signed{}; if (keytype_str == "ssh-rsa") { - rfc4251::mpint e{datastream}; - rfc4251::mpint n{datastream}; + rfc4251::mpint const e{datastream}; + rfc4251::mpint const n{datastream}; key_to_be_signed << rfc4251::string{keytype_str} << e << n; } else if (keytype_str == "ssh-dss") { - rfc4251::mpint p{datastream}; - rfc4251::mpint q{datastream}; - rfc4251::mpint g{datastream}; - rfc4251::mpint y{datastream}; + rfc4251::mpint const p{datastream}; + rfc4251::mpint const q{datastream}; + rfc4251::mpint const g{datastream}; + rfc4251::mpint const y{datastream}; key_to_be_signed << rfc4251::string{keytype_str} << p << q << g << y; } else if (keytype_str == "ecdsa-sha2-nistp256" || keytype_str == "ecdsa-sha2-nistp384" || keytype_str == "ecdsa-sha2-nistp521") { - rfc4251::string curve{datastream}; - rfc4251::string public_key{datastream}; + rfc4251::string const curve{datastream}; + rfc4251::string const public_key{datastream}; key_to_be_signed << rfc4251::string{keytype_str} << curve << public_key; } else if (keytype_str == "ssh-ed25519") { - rfc4251::string pk{datastream}; + rfc4251::string const pk{datastream}; key_to_be_signed << rfc4251::string{keytype_str} << pk; } else { return {}; } - rfc4251::uint64 serial{datastream}; - rfc4251::uint32 type{datastream}; - rfc4251::string key_id{datastream}; - rfc4251::string valid_principals{datastream}; - rfc4251::uint64 valid_after{datastream}; - rfc4251::uint64 valid_before{datastream}; - rfc4251::string critical_options{datastream}; - rfc4251::string extensions{datastream}; - rfc4251::string reserved{datastream}; - rfc4251::string signature_key{datastream}; + rfc4251::uint64 const serial{datastream}; + rfc4251::uint32 const type{datastream}; + rfc4251::string const key_id{datastream}; + rfc4251::string const valid_principals{datastream}; + rfc4251::uint64 const valid_after{datastream}; + rfc4251::uint64 const valid_before{datastream}; + rfc4251::string const critical_options{datastream}; + rfc4251::string const extensions{datastream}; + rfc4251::string const reserved{datastream}; + rfc4251::string const signature_key{datastream}; request_description = "The request is for a certificate signature on key " + base64_encode(key_to_be_signed.str()) + "."; @@ -389,14 +389,14 @@ std::optional dissect_auth_data_ssh (rfc4251::string const & data) try { string request_description{}; // Format specified in RFC 4252 Section 7 - rfc4251::string session_identifier{datastream}; - rfc4251::byte requesttype{datastream}; - rfc4251::string username{datastream}; - rfc4251::string servicename{datastream}; - rfc4251::string publickeystring{datastream}; - rfc4251::boolean shouldbetrue{datastream}; - rfc4251::string publickeyalgorithm{datastream}; - rfc4251::string publickey{datastream}; + rfc4251::string const session_identifier{datastream}; + rfc4251::byte const requesttype{datastream}; + rfc4251::string const username{datastream}; + rfc4251::string const servicename{datastream}; + rfc4251::string const publickeystring{datastream}; + rfc4251::boolean const shouldbetrue{datastream}; + rfc4251::string const publickeyalgorithm{datastream}; + rfc4251::string const publickey{datastream}; request_description = "The request is for an ssh connection as user '" + string{username} + "' with service name '" + string{servicename} + "'."; @@ -405,17 +405,17 @@ std::optional dissect_auth_data_ssh (rfc4251::string const & data) try { io::stream idstream{session_identifier.data(), session_identifier.size()}; arm(idstream); - rfc4251::uint32 type{idstream}; + rfc4251::uint32 const type{idstream}; if (type == 101) { // PAM_SSH_AGENT_AUTH_REQUESTv1 - rfc4251::string cookie{idstream}; - rfc4251::string user{idstream}; - rfc4251::string ruser{idstream}; - rfc4251::string pam_service{idstream}; - rfc4251::string pwd{idstream}; - rfc4251::string action{idstream}; - rfc4251::string hostname{idstream}; - rfc4251::uint64 timestamp{idstream}; + rfc4251::string const cookie{idstream}; + rfc4251::string const user{idstream}; + rfc4251::string const ruser{idstream}; + rfc4251::string const pam_service{idstream}; + rfc4251::string const pwd{idstream}; + rfc4251::string const action{idstream}; + rfc4251::string const hostname{idstream}; + rfc4251::uint64 const timestamp{idstream}; string singleuser{user}; if (user != ruser) @@ -428,12 +428,12 @@ std::optional dissect_auth_data_ssh (rfc4251::string const & data) try { io::stream actionstream{action.data(), action.size()}; arm(actionstream); - rfc4251::uint32 argc{actionstream}; + rfc4251::uint32 const argc{actionstream}; if (argc) { additional += " to run"; for (uint32_t i = argc; i; --i) { - rfc4251::string argv{actionstream}; + rfc4251::string const argv{actionstream}; additional += ' ' + string{argv}; } } @@ -474,7 +474,7 @@ rfc4251::string handle_request (rfc4251::string const & r) { io::stream>> answer{ret.buf}; arm(request); arm(answer); - rfc4251::byte request_code{request}; + rfc4251::byte const request_code{request}; switch (request_code) { case SSH2_AGENTC_REQUEST_IDENTITIES: { @@ -483,14 +483,14 @@ rfc4251::string handle_request (rfc4251::string const & r) { agent << rfc4251::string{string{SSH2_AGENTC_REQUEST_IDENTITIES}}; // temp to test key filtering when signing //return rfc4251::string{agent}; - rfc4251::string agent_answer{agent}; + rfc4251::string const agent_answer{agent}; io::stream agent_answer_iss{agent_answer.data(), agent_answer.size()}; arm(agent_answer_iss); - rfc4251::byte answer_code{agent_answer_iss}; - rfc4251::uint32 keycount{agent_answer_iss}; + rfc4251::byte const answer_code{agent_answer_iss}; + rfc4251::uint32 const keycount{agent_answer_iss}; if (answer_code != SSH2_AGENT_IDENTITIES_ANSWER) throw runtime_error{"unexpected answer from ssh-agent"}; - vector> keys; + vector> keys; for (uint32_t i = keycount; i; --i) { rfc4251::string key{agent_answer_iss}; rfc4251::string comment{agent_answer_iss}; @@ -504,9 +504,9 @@ rfc4251::string handle_request (rfc4251::string const & r) { break; case SSH2_AGENTC_SIGN_REQUEST: { - rfc4251::string key{request}; - rfc4251::string data_to_be_signed{request}; - rfc4251::uint32 flags{request}; + rfc4251::string const key{request}; + rfc4251::string const data_to_be_signed{request}; + rfc4251::uint32 const flags{request}; bool allow{false}; if (allowed_pubkeys.count(key)) -- cgit v1.2.3 From 34781d9036c6e564209780154f781b04cd040805 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Sun, 25 Apr 2021 17:52:03 +0200 Subject: factor out socket creation with CLOEXEC --- ssh-agent-filter.C | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index 750ae60..e94425b 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -126,7 +126,16 @@ void cloexec (int fd) { throw system_error(errno, system_category(), "fcntl"); } -void arm(std::ios & stream) { +int make_cloexec_socket (int const address_family, int const type, int const protocol) { + int sock; + std::lock_guard lock{fd_fork_mutex}; + if ((sock = socket(address_family, type | SOCK_CLOEXEC, protocol)) == -1) + throw system_error(errno, system_category(), "socket"); + cloexec(sock); + return sock; +} + +void arm (std::ios & stream) { stream.exceptions(stream.badbit | stream.failbit); } @@ -136,13 +145,7 @@ int make_upstream_agent_conn () { if (!(path = getenv("SSH_AUTH_SOCK"))) throw invalid_argument("no $SSH_AUTH_SOCK"); - int sock; - { - std::lock_guard lock{fd_fork_mutex}; - if ((sock = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0)) == -1) - throw system_error(errno, system_category(), "socket"); - cloexec(sock); - } + int const sock = make_cloexec_socket(AF_UNIX, SOCK_STREAM, 0); struct sockaddr_un addr{AF_UNIX, {}}; @@ -158,13 +161,7 @@ int make_upstream_agent_conn () { } int make_listen_sock () { - int sock; - { - std::lock_guard lock{fd_fork_mutex}; - if ((sock = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0)) == -1) - throw system_error(errno, system_category(), "socket"); - cloexec(sock); - } + int const sock = make_cloexec_socket(AF_UNIX, SOCK_STREAM, 0); if (fcntl(sock, F_SETFL, fcntl(sock, F_GETFL) | O_NONBLOCK)) throw system_error(errno, system_category(), "fcntl"); -- cgit v1.2.3 From b8b65de84dff999f11bcb3379b576c47faa020b5 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Sun, 25 Apr 2021 22:06:21 +0200 Subject: rfc4251::string: allow construction from std::initializer_list --- rfc4251.H | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rfc4251.H b/rfc4251.H index 6647d15..41d466f 100644 --- a/rfc4251.H +++ b/rfc4251.H @@ -144,6 +144,10 @@ struct string : internal::variable_length { buf.insert(buf.end(), s, s + l); } explicit string (std::string const & s) : string{s.data(), s.size()} {} + string (std::initializer_list init) { + check_length_against_limit(init.size()); + buf = init; + } operator std::string () const { return {buf.cbegin(), buf.cend()}; } }; -- cgit v1.2.3 From fcdfa65579518c2d9d9fa74231455a08a0e783f3 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Tue, 27 Apr 2021 12:40:17 +0200 Subject: factor out communication with the upstream agent --- ssh-agent-filter.C | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index e94425b..d22ced1 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -182,6 +182,14 @@ int make_listen_sock () { return sock; } +rfc4251::string ask_upstream_agent (rfc4251::string const & request) { + io::stream agent{make_upstream_agent_conn(), io::close_handle}; + arm(agent); + + agent << request; + return rfc4251::string{agent}; +} + void parse_cmdline (int const argc, char const * const * const argv) { po::options_description opts{"Options"}; opts.add_options() @@ -229,11 +237,7 @@ void parse_cmdline (int const argc, char const * const * const argv) { } void setup_filters () { - io::stream agent{make_upstream_agent_conn(), io::close_handle}; - arm(agent); - - agent << rfc4251::string{string{SSH2_AGENTC_REQUEST_IDENTITIES}}; - rfc4251::string const answer{agent}; + auto const answer = ask_upstream_agent({SSH2_AGENTC_REQUEST_IDENTITIES}); io::stream answer_iss{answer.data(), answer.size()}; arm(answer_iss); rfc4251::byte const resp_code{answer_iss}; @@ -475,12 +479,9 @@ rfc4251::string handle_request (rfc4251::string const & r) { switch (request_code) { case SSH2_AGENTC_REQUEST_IDENTITIES: { - io::stream agent{make_upstream_agent_conn(), io::close_handle}; - arm(agent); - agent << rfc4251::string{string{SSH2_AGENTC_REQUEST_IDENTITIES}}; + auto const agent_answer = ask_upstream_agent({SSH2_AGENTC_REQUEST_IDENTITIES}); // temp to test key filtering when signing - //return rfc4251::string{agent}; - rfc4251::string const agent_answer{agent}; + //return agent_answer; io::stream agent_answer_iss{agent_answer.data(), agent_answer.size()}; arm(agent_answer_iss); rfc4251::byte const answer_code{agent_answer_iss}; @@ -518,12 +519,7 @@ rfc4251::string handle_request (rfc4251::string const & r) { } if (allow) { - io::stream agent{make_upstream_agent_conn(), io::close_handle}; - arm(agent); - rfc4251::string agent_answer; - - agent << r; - return rfc4251::string{agent}; + return ask_upstream_agent(r); } else answer << rfc4251::byte{SSH_AGENT_FAILURE}; } -- cgit v1.2.3 From daf69a87ec7d01673362c1c9bbdbcb61c71ab83e Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Tue, 27 Apr 2021 12:41:34 +0200 Subject: s/arm/arm_exceptions/ --- ssh-agent-filter.C | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index d22ced1..252e017 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -135,7 +135,7 @@ int make_cloexec_socket (int const address_family, int const type, int const pro return sock; } -void arm (std::ios & stream) { +void arm_exceptions (std::ios & stream) { stream.exceptions(stream.badbit | stream.failbit); } @@ -184,7 +184,7 @@ int make_listen_sock () { rfc4251::string ask_upstream_agent (rfc4251::string const & request) { io::stream agent{make_upstream_agent_conn(), io::close_handle}; - arm(agent); + arm_exceptions(agent); agent << request; return rfc4251::string{agent}; @@ -239,7 +239,7 @@ void parse_cmdline (int const argc, char const * const * const argv) { void setup_filters () { auto const answer = ask_upstream_agent({SSH2_AGENTC_REQUEST_IDENTITIES}); io::stream answer_iss{answer.data(), answer.size()}; - arm(answer_iss); + arm_exceptions(answer_iss); rfc4251::byte const resp_code{answer_iss}; if (resp_code != SSH2_AGENT_IDENTITIES_ANSWER) throw runtime_error{"unexpected answer from ssh-agent"}; @@ -326,7 +326,7 @@ bool confirm (string const & question) { std::optional dissect_auth_data_ssh_cert (rfc4251::string const & data) try { io::stream datastream{data.data(), data.size()}; - arm(datastream); + arm_exceptions(datastream); string request_description{}; // Format specified in https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL.certkeys?annotate=1.13 @@ -386,7 +386,7 @@ std::optional dissect_auth_data_ssh_cert (rfc4251::string const & data) std::optional dissect_auth_data_ssh (rfc4251::string const & data) try { io::stream datastream{data.data(), data.size()}; - arm(datastream); + arm_exceptions(datastream); string request_description{}; // Format specified in RFC 4252 Section 7 @@ -404,7 +404,7 @@ std::optional dissect_auth_data_ssh (rfc4251::string const & data) try { if (string{servicename} == "pam_ssh_agent_auth") try { clog << base64_encode(session_identifier) << endl; io::stream idstream{session_identifier.data(), session_identifier.size()}; - arm(idstream); + arm_exceptions(idstream); rfc4251::uint32 const type{idstream}; if (type == 101) { @@ -427,7 +427,7 @@ std::optional dissect_auth_data_ssh (rfc4251::string const & data) try { additional += "' in '" + string{pwd}; io::stream actionstream{action.data(), action.size()}; - arm(actionstream); + arm_exceptions(actionstream); rfc4251::uint32 const argc{actionstream}; @@ -473,8 +473,8 @@ rfc4251::string handle_request (rfc4251::string const & r) { io::stream request{r.data(), r.size()}; rfc4251::string ret; io::stream>> answer{ret.buf}; - arm(request); - arm(answer); + arm_exceptions(request); + arm_exceptions(answer); rfc4251::byte const request_code{request}; switch (request_code) { case SSH2_AGENTC_REQUEST_IDENTITIES: @@ -483,7 +483,7 @@ rfc4251::string handle_request (rfc4251::string const & r) { // temp to test key filtering when signing //return agent_answer; io::stream agent_answer_iss{agent_answer.data(), agent_answer.size()}; - arm(agent_answer_iss); + arm_exceptions(agent_answer_iss); rfc4251::byte const answer_code{agent_answer_iss}; rfc4251::uint32 const keycount{agent_answer_iss}; if (answer_code != SSH2_AGENT_IDENTITIES_ANSWER) @@ -563,7 +563,7 @@ void handle_client (int const sock) try { // fails with ESPIPE and causes an exception io::stream client_in{sock, io::close_handle}; io::stream client_out{sock, io::never_close_handle}; - arm(client_out); + arm_exceptions(client_out); for (;;) client_out << handle_request(rfc4251::string{client_in}) << flush; -- cgit v1.2.3 From 56ced7e19f4bce304f567aea3d6cd67769bb89e3 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Tue, 27 Apr 2021 12:50:51 +0200 Subject: handle some edge cases when dup2()ing /dev/null --- ssh-agent-filter.C | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index 252e017..b806a85 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -602,11 +602,13 @@ int main (int const argc, char const * const * const argv) { // the following stuff is optional, so we don't do error checking setsid(); static_cast(chdir("/")); - int devnull = open("/dev/null", O_RDWR); - dup2(devnull, 0); - dup2(devnull, 1); - dup2(devnull, 2); - close(devnull); + if (int devnull = open("/dev/null", O_RDWR); devnull != -1) { + dup2(devnull, 0); + dup2(devnull, 1); + dup2(devnull, 2); + if (devnull > 2) + close(devnull); + } } else { cout << "copy this to another terminal:" << endl; cout << "SSH_AUTH_SOCK='" << path.native() << "'; export SSH_AUTH_SOCK;" << endl; -- cgit v1.2.3 From 74b15a1fe3be3bd549e5fdbfbaf0b64db48a7a76 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Tue, 11 May 2021 13:38:09 +0200 Subject: apply some more const --- ssh-agent-filter.C | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index b806a85..fbeb85f 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -245,16 +245,16 @@ void setup_filters () { throw runtime_error{"unexpected answer from ssh-agent"}; rfc4251::uint32 const keycount{answer_iss}; for (uint32_t i = keycount; i; --i) { - rfc4251::string key{answer_iss}; - rfc4251::string comment{answer_iss}; + rfc4251::string key{answer_iss}; + rfc4251::string const comment{answer_iss}; - auto b64 = base64_encode(key); + auto const b64 = base64_encode(key); if (debug) clog << b64 << endl; - auto md5 = md5_hex(key); + auto const md5 = md5_hex(key); if (debug) clog << md5 << endl; - string comm(comment); + string comm{comment}; if (debug) clog << comm << endl; bool allow{false}; -- cgit v1.2.3 From 2927e78b226ede4df9d6eee9701485ed32c8d48a Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Tue, 11 May 2021 13:41:06 +0200 Subject: add some more documentation: dissected requests for confirmation, threat model --- README.md | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 05ca21d..c5326c1 100644 --- a/README.md +++ b/README.md @@ -35,12 +35,17 @@ confirmation You can use the `--*-confirmed` options (e.g.`--comment-confirmed`) to add keys for which you want to be asked on each use through the filter. The confirmation is done in the same way as when you `ssh-add -c` a key to your `ssh-agent`, but the question will contain some additional information extracted from the sign request. +These types of sign requests are dissected: +* ssh connections +* authentications with `libpam-ssh-agent-auth` +* ssh certificates + how it works ------------ -ssh-agent-filter provides a socket interface identical to that of a normal ssh-agent. -We don't keep private key material, but delegate requests to the upstream ssh-agent after checking if the key is allowed. +`ssh-agent-filter` provides a socket interface identical to that of a normal `ssh-agent`. +We don't keep private key material, but delegate requests to the upstream `ssh-agent` after checking if the key is allowed. The following requests are implemented: * `SSH2_AGENTC_REQUEST_IDENTITIES`: @@ -58,3 +63,15 @@ The following requests are implemented: * success is returned without doing anything Requests to add or remove keys and to (un)lock the agent are refused + + +threat model +------------ + +We assume trusted: +* the user invoking our software +* the upstream ssh-agent +* the user giving or declining confirmation + +We assume untrusted: +* any connection from clients on our listening socket -- cgit v1.2.3 From 3088eda994ea0812e4aebf414334304723108671 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Mon, 17 May 2021 12:37:22 +0200 Subject: factor out alter_fd_flags() this also saves one syscall each time the flags are already as they should be --- ssh-agent-filter.C | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index fbeb85f..61255a0 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -121,8 +121,19 @@ string base64_encode (string const & s) { return {begin(b64), end(b64)}; } -void cloexec (int fd) { - if (fcntl(fd, F_SETFD, fcntl(fd, F_GETFD) | FD_CLOEXEC)) +template +void alter_fd_flags (int const fd) { + static_assert(write_cmd == F_SETFD || write_cmd == F_SETFL, "unsupported fcntl() operation"); + constexpr int read_cmd = (write_cmd == F_SETFD) ? F_GETFD : F_GETFL; + static_assert((set_flags & clear_flags) == 0, "overlap in set and clear"); + + int const current_flags = fcntl(fd, read_cmd); + if (current_flags == -1) + throw system_error(errno, system_category(), "fcntl"); + int const new_flags = (current_flags | set_flags) & ~clear_flags; + if (current_flags == new_flags) + return; + if (fcntl(fd, write_cmd, new_flags)) throw system_error(errno, system_category(), "fcntl"); } @@ -131,7 +142,7 @@ int make_cloexec_socket (int const address_family, int const type, int const pro std::lock_guard lock{fd_fork_mutex}; if ((sock = socket(address_family, type | SOCK_CLOEXEC, protocol)) == -1) throw system_error(errno, system_category(), "socket"); - cloexec(sock); + alter_fd_flags(sock); return sock; } @@ -163,8 +174,7 @@ int make_upstream_agent_conn () { int make_listen_sock () { int const sock = make_cloexec_socket(AF_UNIX, SOCK_STREAM, 0); - if (fcntl(sock, F_SETFL, fcntl(sock, F_GETFL) | O_NONBLOCK)) - throw system_error(errno, system_category(), "fcntl"); + alter_fd_flags(sock); struct sockaddr_un addr{AF_UNIX, {}}; @@ -555,8 +565,7 @@ rfc4251::string handle_request (rfc4251::string const & r) { } void handle_client (int const sock) try { - if (fcntl(sock, F_SETFL, fcntl(sock, F_GETFL) & ~O_NONBLOCK)) - throw system_error(errno, system_category(), "fcntl"); + alter_fd_flags(sock); // we could use only one streambuf and iostream but when // switching from read to write an lseek call is made that @@ -633,7 +642,7 @@ int main (int const argc, char const * const * const argv) { else break; } - cloexec(client_sock); + alter_fd_flags(client_sock); } std::thread t{handle_client, client_sock}; t.detach(); -- cgit v1.2.3 From adbbb81f7a82829363b41f6de8fc403caece3f33 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Mon, 17 May 2021 13:10:24 +0200 Subject: add SOCK_NONBLOCK on listen socket creation to possibly save a syscall --- ssh-agent-filter.C | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index 61255a0..cadfd1d 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -88,6 +88,9 @@ using std::pair; #ifndef SOCK_CLOEXEC #define SOCK_CLOEXEC 0 #endif +#ifndef SOCK_NONBLOCK +#define SOCK_NONBLOCK 0 +#endif vector allowed_b64; vector allowed_md5; @@ -172,7 +175,7 @@ int make_upstream_agent_conn () { } int make_listen_sock () { - int const sock = make_cloexec_socket(AF_UNIX, SOCK_STREAM, 0); + int const sock = make_cloexec_socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0); alter_fd_flags(sock); -- cgit v1.2.3 From 7fc3abd4b250ceae28c2b0705625d708260cfc38 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Mon, 17 May 2021 15:12:31 +0200 Subject: fix typo: puposes purposes --- ssh-agent-filter.C | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index cadfd1d..3fe4801 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -215,7 +215,7 @@ void parse_cmdline (int const argc, char const * const * const argv) { ("help,h", "print this help message") ("key,k", po::value(&allowed_b64), "key specified by base64-encoded pubkey") ("key-confirmed,K", po::value(&confirmed_b64), "key specified by base64-encoded pubkey, with confirmation") - ("name,n", po::value(&saf_name), "name for this instance of ssh-agent-filter, for confirmation puposes") + ("name,n", po::value(&saf_name), "name for this instance of ssh-agent-filter, for confirmation purposes") ("version,V", "print version information") ; po::variables_map config; -- cgit v1.2.3 From 19304907075d2cc69805dae81493f28483be305b Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Thu, 5 Mar 2026 19:22:39 +0100 Subject: tests: work around https://github.com/kward/shunit2/issues/112 --- tests | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests b/tests index d222e7e..415be38 100755 --- a/tests +++ b/tests @@ -38,7 +38,7 @@ oneTimeSetUp () { } oneTimeTearDown () { - [ -z "$SSH_AGENT_PID" ] || kill "$SSH_AGENT_PID" + [ -z "$SSH_AGENT_PID" ] || { kill "$SSH_AGENT_PID"; unset SSH_AGENT_PID; } } with_saf_in_tmp () { -- cgit v1.2.3 From 60b84f1d9e64254c817bfa1b9a54a1a723af0712 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Sat, 7 Mar 2026 18:06:40 +0100 Subject: tests: use empty $HOME --- tests | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests b/tests index 415be38..f01c70b 100755 --- a/tests +++ b/tests @@ -22,6 +22,11 @@ oneTimeSetUp () { set -e + # openssh tools require an at least existing $HOME and + # are probably sensitive to it, create an empty one + mkdir "${SHUNIT_TMPDIR}/home" + export HOME="${SHUNIT_TMPDIR}/home" + # prepare keys ( cd "$SHUNIT_TMPDIR"; ssh-keygen -q -t ed25519 -N '' -C key0 -f key0 ) ( cd "$SHUNIT_TMPDIR"; ssh-keygen -q -t ed25519 -N '' -C key1 -f key1 ) -- cgit v1.2.3 From c212ae94dce33e150bc384db54a3c4d471c75bd7 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Sat, 7 Mar 2026 18:44:02 +0100 Subject: follow API break of nettle 4 --- ssh-agent-filter.C | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ssh-agent-filter.C b/ssh-agent-filter.C index 3fe4801..8a2772d 100644 --- a/ssh-agent-filter.C +++ b/ssh-agent-filter.C @@ -77,6 +77,7 @@ using std::pair; #include #include #include +#include #include #include #include @@ -112,7 +113,11 @@ string md5_hex (string const & s) { md5_init(&ctx); md5_update(&ctx, s.size(), reinterpret_cast(s.data())); std::array bin; +#if (NETTLE_VERSION_MAJOR >= 4) + md5_digest(&ctx, bin.data()); +#else md5_digest(&ctx, MD5_DIGEST_SIZE, bin.data()); +#endif std::array hex; base16_encode_update(hex.data(), MD5_DIGEST_SIZE, bin.data()); return {begin(hex), end(hex)}; -- cgit v1.2.3 From 8bd8f61f2ab2978a0a1bf7aa6f19a6352b9769c9 Mon Sep 17 00:00:00 2001 From: Timo Weingärtner Date: Sat, 7 Mar 2026 22:24:24 +0100 Subject: release 0.5.3 --- changelog | 162 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- version.h | 2 +- 2 files changed, 161 insertions(+), 3 deletions(-) diff --git a/changelog b/changelog index dba174e..6b24c0e 100644 --- a/changelog +++ b/changelog @@ -1,4 +1,162 @@ -commit 87f2de93a6522bbcf17d1960e78641df8ecd85d3 (HEAD -> master) +commit 267e770d08ffcddb17c09710a50ed65d10e9ad63 +Author: Timo Weingärtner +Date: 2026-03-07 22:24:24 +0100 + + release 0.5.3 + +commit c212ae94dce33e150bc384db54a3c4d471c75bd7 +Author: Timo Weingärtner +Date: 2026-03-07 18:44:02 +0100 + + follow API break of nettle 4 + +commit 60b84f1d9e64254c817bfa1b9a54a1a723af0712 +Author: Timo Weingärtner +Date: 2026-03-07 18:06:40 +0100 + + tests: use empty $HOME + +commit 19304907075d2cc69805dae81493f28483be305b +Author: Timo Weingärtner +Date: 2026-03-05 19:22:39 +0100 + + tests: work around https://github.com/kward/shunit2/issues/112 + +commit 7fc3abd4b250ceae28c2b0705625d708260cfc38 +Author: Timo Weingärtner +Date: 2021-05-17 15:12:31 +0200 + + fix typo: puposes purposes + +commit adbbb81f7a82829363b41f6de8fc403caece3f33 +Author: Timo Weingärtner +Date: 2021-05-17 13:10:24 +0200 + + add SOCK_NONBLOCK on listen socket creation to possibly save a syscall + +commit 3088eda994ea0812e4aebf414334304723108671 +Author: Timo Weingärtner +Date: 2021-05-17 12:37:22 +0200 + + factor out alter_fd_flags() + + this also saves one syscall each time the flags are already as they should be + +commit 2927e78b226ede4df9d6eee9701485ed32c8d48a +Author: Timo Weingärtner +Date: 2021-05-11 13:41:06 +0200 + + add some more documentation: dissected requests for confirmation, threat model + +commit 74b15a1fe3be3bd549e5fdbfbaf0b64db48a7a76 +Author: Timo Weingärtner +Date: 2021-05-11 13:38:09 +0200 + + apply some more const + +commit 56ced7e19f4bce304f567aea3d6cd67769bb89e3 +Author: Timo Weingärtner +Date: 2021-04-27 12:50:51 +0200 + + handle some edge cases when dup2()ing /dev/null + +commit daf69a87ec7d01673362c1c9bbdbcb61c71ab83e +Author: Timo Weingärtner +Date: 2021-04-27 12:41:34 +0200 + + s/arm/arm_exceptions/ + +commit fcdfa65579518c2d9d9fa74231455a08a0e783f3 +Author: Timo Weingärtner +Date: 2021-04-27 12:40:17 +0200 + + factor out communication with the upstream agent + +commit b8b65de84dff999f11bcb3379b576c47faa020b5 +Author: Timo Weingärtner +Date: 2021-04-25 22:06:21 +0200 + + rfc4251::string: allow construction from std::initializer_list + +commit 34781d9036c6e564209780154f781b04cd040805 +Author: Timo Weingärtner +Date: 2021-04-25 17:52:03 +0200 + + factor out socket creation with CLOEXEC + +commit 1ed4729f59343b8041387b8802610c25de599085 +Author: Timo Weingärtner +Date: 2021-04-25 17:45:45 +0200 + + add const to most rfc4251:: variables + +commit 145c64e6c4e4151e869104e12da71786b8c31932 +Author: Timo Weingärtner +Date: 2021-04-22 23:33:14 +0200 + + replace strcpy() with something clang-tidy doesn't complain about + +commit fc0e196e4fd302d681725c9e5549e34b93a91075 +Author: Timo Weingärtner +Date: 2021-04-22 23:31:12 +0200 + + use std:: containers instead of C-style arrays + +commit b929b048a1839c3da1cadf7f116b0f767f40267e +Author: Timo Weingärtner +Date: 2021-04-22 23:29:10 +0200 + + implement separate rfc4251::(string|mpint|name_list) types + +commit 9d25142d506d1559b134d465ff0434ffbde7dca8 +Author: Timo Weingärtner +Date: 2021-04-18 00:04:19 +0200 + + rfc4251.H: use template for fixed-length-types + +commit b3989076c699ee2d3e01377d17edec9a8bd0b2c3 +Author: Timo Weingärtner +Date: 2021-04-21 13:26:03 +0200 + + add .gitignore + +commit 664ad8e38fd0f077e1fba5e69325fb3a36a75e19 +Author: Timo Weingärtner +Date: 2021-04-18 00:03:31 +0200 + + add a test for cert signature dissection + +commit df63d26366b39ad8eb1c1ba5876b57b6a15208ed +Author: Timo Weingärtner +Date: 2021-04-29 17:28:15 +0200 + + use C++17 constructor template argument deduction (CTAD) + +commit d17162f5c6f7d002bf68fac48bcadd2db0c033e7 +Author: Timo Weingärtner +Date: 2021-04-18 00:02:42 +0200 + + use C++17 std::optional for dissectors, refactor + +commit 013ca71f8e07f462266f5c795930739b37c6d5fb +Author: Timo Weingärtner +Date: 2020-08-24 13:42:39 +0200 + + use C++17 std::filesystem instead of boost + +commit f8351aef302fa2da6183096fdf0013b2f29900ae +Author: Timo Weingärtner +Date: 2020-08-14 23:35:47 +0200 + + upgrade to C++17 + +commit 6036ef52c956d8de14279b6f160ece60ae596eae (tag: 0.5.2) +Author: Timo Weingärtner +Date: 2018-11-23 23:20:30 +0100 + + release 0.5.2 + +commit 87f2de93a6522bbcf17d1960e78641df8ecd85d3 Author: Timo Weingärtner Date: 2018-11-19 21:27:42 +0100 @@ -12,7 +170,7 @@ Date: 2018-11-18 16:42:06 +0100 tests: describe the asserts -commit cc7b883c67b78021c13df453abeb35d8d9055c35 (tag: 0.5.1, tiwe/master) +commit cc7b883c67b78021c13df453abeb35d8d9055c35 (tag: 0.5.1) Author: Timo Weingärtner Date: 2018-07-18 20:42:58 +0200 diff --git a/version.h b/version.h index 923bc6f..671a9f6 100644 --- a/version.h +++ b/version.h @@ -1 +1 @@ -#define SSH_AGENT_FILTER_VERSION "ssh-agent-filter 0.5.2" +#define SSH_AGENT_FILTER_VERSION "ssh-agent-filter 0.5.3" -- cgit v1.2.3