From 09aa807240b9dcde78a919ff712316a1daf0655e Mon Sep 17 00:00:00 2001 From: Jay Satiro Date: Sun, 23 Feb 2020 18:37:09 -0500 Subject: libssh: Fix matching user-specified MD5 hex key Prior to this change a match would never be successful because it was mistakenly coded to compare binary data from libssh to a user-specified hex string (ie CURLOPT_SSH_HOST_PUBLIC_KEY_MD5). Reported-by: fds242@users.noreply.github.com Fixes https://github.com/curl/curl/issues/4971 Closes https://github.com/curl/curl/pull/4974 --- lib/vssh/libssh.c | 20 +++++++++++++++++--- tests/.gitignore | 1 + tests/FILEFORMAT | 1 + tests/data/Makefile.inc | 2 +- tests/data/test664 | 44 ++++++++++++++++++++++++++++++++++++++++++++ tests/data/test665 | 44 ++++++++++++++++++++++++++++++++++++++++++++ tests/runtests.pl | 24 ++++++++++++++++++++++++ tests/sshhelp.pm | 3 +++ tests/sshserver.pl | 28 ++++++++++++++++++++++++---- 9 files changed, 159 insertions(+), 8 deletions(-) create mode 100644 tests/data/test664 create mode 100644 tests/data/test665 diff --git a/lib/vssh/libssh.c b/lib/vssh/libssh.c index 647b4d491..08d9f9e0f 100644 --- a/lib/vssh/libssh.c +++ b/lib/vssh/libssh.c @@ -345,13 +345,27 @@ static int myssh_is_known(struct connectdata *conn) return rc; if(data->set.str[STRING_SSH_HOST_PUBLIC_KEY_MD5]) { + int i; + char md5buffer[33]; + const char *pubkey_md5 = data->set.str[STRING_SSH_HOST_PUBLIC_KEY_MD5]; + rc = ssh_get_publickey_hash(pubkey, SSH_PUBLICKEY_HASH_MD5, &hash, &hlen); - if(rc != SSH_OK) + if(rc != SSH_OK || hlen != 16) { + failf(data, + "Denied establishing ssh session: md5 fingerprint not available"); goto cleanup; + } + + for(i = 0; i < 16; i++) + msnprintf(&md5buffer[i*2], 3, "%02x", (unsigned char)hash[i]); + + infof(data, "SSH MD5 fingerprint: %s\n", md5buffer); - if(hlen != strlen(data->set.str[STRING_SSH_HOST_PUBLIC_KEY_MD5]) || - memcmp(&data->set.str[STRING_SSH_HOST_PUBLIC_KEY_MD5], hash, hlen)) { + if(!strcasecompare(md5buffer, pubkey_md5)) { + failf(data, + "Denied establishing ssh session: mismatch md5 fingerprint. " + "Remote %s is not equal to %s", md5buffer, pubkey_md5); rc = SSH_ERROR; goto cleanup; } diff --git a/tests/.gitignore b/tests/.gitignore index fbbc16485..abf9aed8c 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -6,6 +6,7 @@ curl_client_key.pub curl_client_knownhosts curl_host_rsa_key curl_host_rsa_key.pub +curl_host_rsa_key.pub_md5 curl_sftp_cmds curl_sftp_config curl_ssh_config diff --git a/tests/FILEFORMAT b/tests/FILEFORMAT index 42ebe2871..16e7d3092 100644 --- a/tests/FILEFORMAT +++ b/tests/FILEFORMAT @@ -366,6 +366,7 @@ Available substitute variables include: %FILE_PWD - Current directory, on windows prefixed with a slash %RTSP6PORT - IPv6 port number of the RTSP server %RTSPPORT - Port number of the RTSP server +%SSHSRVMD5 - MD5 of SSH server's public key %SMTP6PORT - IPv6 port number of the SMTP server %SMTPPORT - Port number of the SMTP server %SOCKSPORT - Port number of the SOCKS4/5 server diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc index dfc74320e..9afbf7437 100644 --- a/tests/data/Makefile.inc +++ b/tests/data/Makefile.inc @@ -85,7 +85,7 @@ test626 test627 test628 test629 test630 test631 test632 test633 test634 \ test635 test636 test637 test638 test639 test640 test641 test642 \ test643 test644 test645 test646 test647 test648 test649 test650 test651 \ test652 test653 test654 test655 test656 test658 test659 test660 test661 \ -test662 test663 \ +test662 test663 test664 test665 \ \ test700 test701 test702 test703 test704 test705 test706 test707 test708 \ test709 test710 test711 test712 test713 test714 test715 test716 test717 \ diff --git a/tests/data/test664 b/tests/data/test664 new file mode 100644 index 000000000..cb73b248b --- /dev/null +++ b/tests/data/test664 @@ -0,0 +1,44 @@ + + + +SFTP +server key check + + + +# +# Server-side + + +test + + + +# +# Client-side + + +sftp + + +SFTP correct host key + + +--hostpubmd5 %SSHSRVMD5 --key curl_client_key --pubkey curl_client_key.pub -u %USER: sftp://%HOSTIP:%SSHPORT%POSIX_PWD/log/file664.txt + + +test + + + +# +# Verify data after the test has been "shot" + + +0 + + +disable + + + diff --git a/tests/data/test665 b/tests/data/test665 new file mode 100644 index 000000000..830adb8f6 --- /dev/null +++ b/tests/data/test665 @@ -0,0 +1,44 @@ + + + +SCP +server key check + + + +# +# Server-side + + +test + + + +# +# Client-side + + +scp + + +SCP correct host key + + +--hostpubmd5 %SSHSRVMD5 --key curl_client_key --pubkey curl_client_key.pub -u %USER: scp://%HOSTIP:%SSHPORT%POSIX_PWD/log/file665.txt + + +test + + + +# +# Verify data after the test has been "shot" + + +0 + + +disable + + + diff --git a/tests/runtests.pl b/tests/runtests.pl index 827fdec50..08d9f9f3a 100755 --- a/tests/runtests.pl +++ b/tests/runtests.pl @@ -152,6 +152,8 @@ my $SMBPORT; # SMB server port my $SMBSPORT; # SMBS server port my $NEGTELNETPORT; # TELNET server port with negotiation +my $SSHSRVMD5; # MD5 of ssh server public key + my $srcdir = $ENV{'srcdir'} || '.'; my $CURL="../src/curl".exe_ext('TOOL'); # what curl executable to run on the tests my $VCURL=$CURL; # what curl binary to use to verify the servers with @@ -2139,6 +2141,18 @@ sub runsshserver { return (0,0); } + my $hstpubmd5f = "curl_host_rsa_key.pub_md5"; + if(!open(PUBMD5FILE, "<", $hstpubmd5f) || + (read(PUBMD5FILE, $SSHSRVMD5, 32) != 32) || + !close(PUBMD5FILE) || + ($SSHSRVMD5 !~ /^[a-f0-9]{32}$/i)) + { + my $msg = "Fatal: $srvrname pubkey md5 missing : \"$hstpubmd5f\" : $!"; + logmsg "$msg\n"; + stopservers($verbose); + die $msg; + } + if($verbose) { logmsg "RUN: $srvrname server is now running PID $pid2\n"; } @@ -3158,6 +3172,16 @@ sub subVariables { $$thing =~ s/%SRCDIR/$srcdir/g; $$thing =~ s/%USER/$USER/g; + if($$thing =~ /%SSHSRVMD5/) { + if(!$SSHSRVMD5) { + my $msg = "Fatal: Missing SSH server pubkey MD5. Is server running?"; + logmsg "$msg\n"; + stopservers($verbose); + die $msg; + } + $$thing =~ s/%SSHSRVMD5/$SSHSRVMD5/g; + } + # The purpose of FTPTIME2 and FTPTIME3 is to provide times that can be # used for time-out tests and that would work on most hosts as these # adjust for the startup/check time for this particular host. We needed diff --git a/tests/sshhelp.pm b/tests/sshhelp.pm index 248be67a2..e43f86532 100644 --- a/tests/sshhelp.pm +++ b/tests/sshhelp.pm @@ -50,6 +50,7 @@ use vars qw( $sftpcmds $hstprvkeyf $hstpubkeyf + $hstpubmd5f $cliprvkeyf $clipubkeyf @sftppath @@ -82,6 +83,7 @@ use vars qw( $sftpcmds $hstprvkeyf $hstpubkeyf + $hstpubmd5f $cliprvkeyf $clipubkeyf display_sshdconfig @@ -122,6 +124,7 @@ $sftpcmds = 'curl_sftp_cmds'; # sftp client commands batch file $knownhosts = 'curl_client_knownhosts'; # ssh knownhosts file $hstprvkeyf = 'curl_host_rsa_key'; # host private key file $hstpubkeyf = 'curl_host_rsa_key.pub'; # host public key file +$hstpubmd5f = 'curl_host_rsa_key.pub_md5'; # md5 hash of host public key $cliprvkeyf = 'curl_client_key'; # client private key file $clipubkeyf = 'curl_client_key.pub'; # client public key file diff --git a/tests/sshserver.pl b/tests/sshserver.pl index 197e8b872..4414ca51b 100644 --- a/tests/sshserver.pl +++ b/tests/sshserver.pl @@ -28,6 +28,9 @@ use strict; use warnings; use Cwd; use Cwd 'abs_path'; +use Digest::MD5; +use Digest::MD5 'md5_hex'; +use MIME::Base64; #*************************************************************************** # Variables and subs imported from sshhelp module @@ -48,6 +51,7 @@ use sshhelp qw( $sftpcmds $hstprvkeyf $hstpubkeyf + $hstpubmd5f $cliprvkeyf $clipubkeyf display_sshdconfig @@ -357,10 +361,11 @@ if((($sshid =~ /OpenSSH/) && ($sshvernum < 299)) || # if((! -e $hstprvkeyf) || (! -s $hstprvkeyf) || (! -e $hstpubkeyf) || (! -s $hstpubkeyf) || + (! -e $hstpubmd5f) || (! -s $hstpubmd5f) || (! -e $cliprvkeyf) || (! -s $cliprvkeyf) || (! -e $clipubkeyf) || (! -s $clipubkeyf)) { # Make sure all files are gone so ssh-keygen doesn't complain - unlink($hstprvkeyf, $hstpubkeyf, $cliprvkeyf, $clipubkeyf); + unlink($hstprvkeyf, $hstpubkeyf, $hstpubmd5f, $cliprvkeyf, $clipubkeyf); logmsg 'generating host keys...' if($verbose); if(system "\"$sshkeygen\" -q -t rsa -f $hstprvkeyf -C 'curl test server' -N ''") { logmsg 'Could not generate host key'; @@ -374,6 +379,21 @@ if((! -e $hstprvkeyf) || (! -s $hstprvkeyf) || # Make sure that permissions are restricted so openssh doesn't complain system "chmod 600 $hstprvkeyf"; system "chmod 600 $cliprvkeyf"; + # Save md5 hash of public host key + open(RSAKEYFILE, "<$hstpubkeyf"); + my @rsahostkey = do { local $/ = ' '; }; + close(RSAKEYFILE); + if(!$rsahostkey[1]) { + logmsg 'Failed parsing base64 encoded RSA host key'; + exit 1; + } + open(PUBMD5FILE, ">$hstpubmd5f"); + print PUBMD5FILE md5_hex(decode_base64($rsahostkey[1])); + close(PUBMD5FILE); + if((! -e $hstpubmd5f) || (! -s $hstpubmd5f)) { + logmsg 'Failed writing md5 hash of RSA host key'; + exit 1; + } } @@ -1099,8 +1119,8 @@ elsif($verbose && ($rc >> 8)) { #*************************************************************************** # Clean up once the server has stopped # -unlink($hstprvkeyf, $hstpubkeyf, $cliprvkeyf, $clipubkeyf, $knownhosts); -unlink($sshdconfig, $sshconfig, $sftpconfig); - +unlink($hstprvkeyf, $hstpubkeyf, $hstpubmd5f, + $cliprvkeyf, $clipubkeyf, $knownhosts, + $sshdconfig, $sshconfig, $sftpconfig); exit 0; -- cgit v1.2.3