Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Modernize code2network() #1530

Open
wants to merge 9 commits into
base: 3.2
Choose a base branch
from
Open

Modernize code2network() #1530

wants to merge 9 commits into from

Conversation

drwetter
Copy link
Owner

@drwetter drwetter commented Mar 6, 2020

This is a WIP for making the backend functions for sending bytes over sockets better.

This function had before a mixture of sed and tr commands
which was now replaced by bash internal functions. It makes
the code better, performance gain in the LAN is neglectable (1s).

This brings code2network somewhat in line with socksend(). This
function does basically the same (and thus is probably prone
to extinction ;-) ). Albeit there the good thing is it does
conversion and sending in one shot.
This commit chamges a few functions / calls so that
the hexbyte syntax with leading x was changed to
one without. The calls then need to change from
socksend --> socksend_clienthello .

The goal is basically to remove socksend() at
some point. Also socksend_clienthello()'s use
of NW_STR should be reconsidered.

This PR removes also some blanks, at the right
hand side of some double square brackets and
at some empty lines
This moves the run_ticketbleed function to the socketsend_clienthello.

It is not working yet, see also #1535 why. This is just for the PoC,
I'll explain:
  It has now a function named check_bytestream() which will be called
in debug mode 1 and checks whether the byte stream to be send via
bash sockets is properly formatted. It can detect bugs which otherwise
would be hard to discover.

DO NOT USE IT for anything else than the check

---snip:

code:

check_bytestream() {
     local line=""
     local -i i=0

     # We do a search and replace so that \xaa\x29 becomes
     # _xaa
     # _x29
     #
     # "echo -e" helps us to get a multiline string
     while read -r line; do
          if [[ $i -eq 0 ]]; then
               # first line is empty because this is a LF
               :
          elif [[ ${#line} -ne 4 ]] && [[ $i != 0 ]]; then
               echo "length of byte $i called from $2 is not ok"
          elif [[ ${line:0:1} != _ ]]; then
               echo "char $i called from $2 doesn't start with a \"\\\""
          elif [[ ${line:1:1} != x ]]; then
               echo "char $i called from $2 doesn't have an x in second position"
          elif [[ ${line:2:2} != [0-9a-fA-F][0-9a-fA-F] ]]; then
               echo "byte $i called from $2 is not hex"
          fi
          i+=1
     done < <( echo -e ${1//\\/\\n_})
}

socksend_clienthello() {
     local data=""

     code2network "$1"
     data="$NW_STR"
     if [[ "$DEBUG" -ge 1 ]]; then
          check_bytestream "$data" "${FUNCNAME[1]}"
          [[ "$DEBUG" -ge 4 ]] && echo && echo "\"$data\""
[..]

Result (./testssl.sh -q --debug=1 -U dev.testssl.sh):

Testing vulnerabilities

 Heartbleed (CVE-2014-0160)                not vulnerable (OK), no heartbeat extension
 CCS (CVE-2014-0224)                       not vulnerable (OK)
 Ticketbleed (CVE-2016-9244), experiment.  length of byte 311 called from run_ticketbleed is not ok
length of byte 312 called from run_ticketbleed is not ok
length of byte 313 called from run_ticketbleed is not ok
length of byte 314 called from run_ticketbleed is not ok
length of byte 315 called from run_ticketbleed is not ok
length of byte 316 called from run_ticketbleed is not ok
length of byte 317 called from run_ticketbleed is not ok
[..]

---snap

Besides that:

* dec02hex was corrected (only being used for run_ticketbleed)
* dec04hex is still buggy and part of the problem
* some quotes removed from rhs of [[]]
This commit fixes ticketbleed so that using socksend_clienthello().
can being used.

The function for retrieving the TLS session ticket is now using SNI
and it was renamed to session_ticket_from_openssl() so that this
can be used elsewhere. Also for the sake of better programming
it is using bash only.

In order to ease stripping whitespaces the bash option "extglob"
was IN GENERAL set. This should only add the possibility to do
extended pattern matching when using round brackets:

?(pattern-list)
    Matches zero or one occurrence of the given patterns.
*(pattern-list)
    Matches zero or more occurrences of the given patterns.
+(pattern-list)
    Matches one or more occurrences of the given patterns.
@(pattern-list)
    Matches one of the given patterns.
!(pattern-list)
    Matches anything except one of the given patterns.

... see bash(1). The man page though warns "separate matches against
shorter strings, or using arrays of strings instead of a single long
string, may be faster.". So when using ~100x we should do s.th. else.
It also works under bashv3.

The check_bytestream() function which was previously introduced now
also list the offending string.
run_ticketbleed() has now a check whether there's "$CLIENT_AUTH"
set. If so a warn message is being issued and the test skipped.

Empty replies for other reasons from the s_client connect are
handled better within run_ticketbleed(). Otherwise it would
lead to ugly errors on the console.

Warning messages for vulneribility checks when client x509-based
authentication is encountered are now all the same. CVE/CWE added.
(run_renego(), run_breach() ).
21, # character set
00, 00, 00, 00, 00, 00, 00, 00, # string[23] reserved (all [0])
00, 00, 00, 00, 00, 00, 00, 00,
00, 00, 00, 00, 00, 00, 00"

debugme echo "=== starting mysql STARTTLS dialog ==="
socksend "${starttls_init}" 0 && debugme echo "${debugpad}initiated STARTTLS" &&
Copy link
Owner Author

Choose a reason for hiding this comment

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

Note to self: doesn't this and in mysql() need to be socksend_clienthello?

@drwetter drwetter added the 3.3dev next release label May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.3dev next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant