Fix various ssh key exchange problems #715
Conversation
* bjorn/use-monotonic-time: supervisor: Correct restart handling test_server: Use erlang:monotonic_time/0 compile: Teach 'time' option to show three significant decimals timer: Use monotonic_time/0 in tc/1,2,3
Common pattern seen in SSL: move y x | move r x -> move2 move r x | move y x -> move2 Common pattern seen in SSL and Compiler: move x r | move x x -> move2
Seen on SSL application where substraction with x registers were prevalent: * i_minus specialization on x registers * i_plus specialization on x registers
We cannot use erlang:unique_integer([positive]) here since this code in run on older test releases as well.
* derek121/gen_server_doc_grammar: Fix grammar in docs for multi_call/*
* sverk/maximmai-pr640-autoconf/OTP-12646: Update config.guess and config.sub to latest versions
* hans/ssh/improve_docs: ssh: broken doc links to file functions fixed
* x0id/jinterface_transport_factory: jinterface: transport factory implementation fix typo error from recent merge OTP-12686
* erland/OTP18/snmp/OTP-12452: snmp: Remove deprecated warning for erlang:now in snmp_verbority snmp: Change to random use crypto. Remove use of erlang:now
* egil/opt-instructions/OTP-12690: erts: Specialize minus and plus instruction erts: Add move2 specialization for common move patterns erts: Specialize rem instruction for common case erts: Specialize band instruction for common case erts: Batch loads and stores for move_window erts: Fix loader increment from minus instruction erts: Add move window instruction erts: Add instruction move3 for xy and xx erts: Specialize compare instructions kernel: Add instruction_count helper to erts_debug
set_type_y/3 is far too complicated. Note that we don't need to check the #st.numy field, because we will detect the error anyway because the information for the y register will be missing in the #st.y gb_tree. There is also a clause that would never match because of a spelling error (the first "n" was missing in "uninitialized"). That clause is not needed because the default clause will do fine. Furthermore, we can break out the special case for handling catch_end and similar instructions into a new function.
The run-time system stopped paying attention the 'aligned' flag in bit syntax construction and matching when bitstrings were introduced in language. The beam_asm compiler pass will crash if the 'aligned' flag is given in bit syntax instructions. beam_validator still validates the 'aligned' flag. Before 912fea0 (which removed the possibility to validate existing BEAM files), the 'aligned' flag could actually be encountered when validating a BEAM file. Since the validation of 'aligned' no longer serves any useful purpose, remove the validation code.
beam_utils used to be overly conservative about liveness for exit instructions such as: call_ext erlang:exit/1 beam_utils would consider all y registers to be used, to avoid overwriting a catch or try tag. That does not seem to be a real risk. However, we miss opportunities for stack trimming if we consider y registers used by an exit instruction.
Understanding get_map_elements improves the stack trimming done by beam_trim.
86fbd6d strengthened type optimization in lets. As a result of the stronger optimizations, special care had to be taken to suppress false warnings. It turns out that false warnings can still slip through. Slapping on a 'compiler_generated' annotation at the top-level of a complex term such as #c_tuple{} may not suppress all warnings. We will need to go deeper into the term to eliminate all warnings.
Using 'try'...'catch' simplifies the code and improves coverage because we don't have to re-throw accidentally caught errors.
The test suites generates listing files, so we can slightly speed up running of test suites (especially when running 'cover') by optimizing writing of .S files.
It is not necessary to compile the compile three times. After the second compilation, we compare the generated .beam files with the .beam files that were used when compiling them. Doing one more round will not find more bugs. While we are it, remove the ?line macros and the unused make_current/1 function.
The misc_SUITE:integer_encoding/1 test case is annoyingly slow. Rewrite the encoding of integers in beam_asm to use the binary:encode_unsigned/1 BIF. Also tweak the test case itself. Scale the down the maximum size of the numbers being generated, but also add test of numbers around boundaries of power of two (which are the numbers most likely to expose bugs in the encoding).
In 8470558, the drop_labels/1 function was added to beam_utils as a minor optimization. Since the function is already available, we might as well use it in index_label/3 too.
The intention of the comparison is to avoid unnecessary updates of the
">=" instead of ">". With the ">" comparison, typically every line
instruction would cause the #asm{} record to be updated.
Simplify the uniq/0 function by using erlang:unique_integer/1.
* bjorn/compiler/misc:
test_lib: Simplify uniq/0
beam_dict: Correct comparison in opcode/2
beam_utils: Re-use the local helper function drop_labels/1
beam_asm: Speed up encoding of large numbers
compilation_SUITE: Speed up the self_compile test cases
beam_listing: Optimize writing of .S files
v3_core, v3_codegen: Eliminate old-style catches
cerl_inline: Replace old-style 'catch' with 'try'...'catch'
sys_core_fold: Suppress warnings better
beam_utils: Teach check_liveness/3 to understand get_map_elements
Teach beam_trim to handle map instructions
beam_utils: Be less conservative about liveness for exit instructions
beam_validator: Stop validating the 'aligned' flag for binaries
beam_validator: Clean up updating of types for y register
beam_validator: Remove support for removed BIF fault/1,2
beam_validator: Correct merging of states
beam_validator: Correct merging of y registers
sys_pre_expand: Remove unused fields in #expand{} record
This commit adds a new function, ssl:connection_information/[1,2] to retrive the connection information from a SSLSocket. And also, this deprecates a function ssl:connection_info/1, and reimplements connection_info/1 with the new function.
This commit adds tests for SNI server support in: * ssl_sni_SUITE.erl * ssl_to_openssl_SUITE.erl And some more modifications: * make_certs also makes two certs for SNI, and adds extra options for SNI.
The newly added function sni_fun allows dynamic update of SSL options like keys and certificates depending on different SNI hostname, rather than a predefined rules of SSL options.
Dialyzer warned about the incorrect match of Packets. Code was refactored and the problem avoided in the process. Dialyzer warned that the empty tuple is not a function as the contract said it should be. Changed the handling of the sni_fun default value to be undefined and added it to the contract.
* rickard/timer-optimization/OTP-12650: Timer fixes, documentation, and test cases Conflicts: erts/emulator/beam/erl_hl_timer.c
|
Patch has passed first testings and has been assigned to be reviewed I am a script, I am not human |
This reverts commit e09dd66.
|
Great job! |
|
Should I rebase on master then? Otherwise the tests will fail on maint without d4cf4d4 |
dotsimon
force-pushed
the
ssh_key_exchange
branch
from
e1d9641
to
313c867
Compare
|
I pushed the fixed tests and I'll await your comment if you want this on master. |
|
Patch has passed first testings and has been assigned to be reviewed I am a script, I am not human |
|
Looks good - master would be perfect! |
When in the connected state, an received KEXINIT message MUST be responded to with KEXINIT. After that, the client may continue with KEXDH_INIT (or similar). See the first paragraph on RFC 4253 sec. 9.
In RFC 4253, sections 7.1 & 9 describe rekeying with special attention to the protocol messages that may be received and may not be sent during rekeying. This patch fixes a number of problems during rekeying caused by data & requests received from the network, and/or data & requests sent by the user.
dotsimon
force-pushed
the
ssh_key_exchange
branch
from
313c867
to
39a21d2
Compare
|
I rebased this on master without the rekey data volume commit. |
|
The summary line of the commit message is too long and/or ends with a "." Bad message: beam_utils: Teach check_liveness/3 to understand get_map_elements I am a script, I am not human |
|
Unfortunately github is missing the feature to switch target for a pull request. This is aimed towards "maint" and you want it towards "master" Feel free to submit a request for this feature to github (they have some kind of vote based backlog system) |
|
Ok, but how do you want to handle this pull request now? Should I close this one and make a new one? Or can you just take the commits directly? I think "we" all agree the changes are needed. |
|
Yes we all agree! I'll take care of taking your commits. Just leave the pull request as it is. Thank you very much for your work! From: Simon Cornish [notifications@github.com] Ok, but how do you want to handle this pull request now? Should I close this one and make a new one? Or can you just take the commits directly? I think "we" all agree the changes are needed. — |
|
Merged to master. |
This pull request addresses a number of issues related to key exchange and re-exchange.
The bugs were:
These are fixed and tests added around functionality