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

Changes for OTP 26 #471

Merged
merged 2 commits into from
Jan 16, 2024
Merged

Changes for OTP 26 #471

merged 2 commits into from
Jan 16, 2024

Conversation

avtobiff
Copy link
Collaborator

  • file:pid2name/1 is removed, update yaws_config:fload functions to pass config file name.

    Fixes build breaks with OTP 27 #467

  • Add OTP 26.0 to the test matrix.

@vinoski
Copy link
Collaborator

vinoski commented Jul 25, 2023

I've had basically the same file:pid2name/1 changes sitting on a local branch in my clone ever since OTP 26 release candidates were published. But I've never had time to figure out what's going wrong with the test failures under OTP 26, which your changes are also hitting.

@avtobiff
Copy link
Collaborator Author

avtobiff commented Jul 27, 2023

I see. I tried bisecting otp between maint-25..OTP-26.0 and digging manually.

The failing test cases:

  • dhfile_SUITE:ssl_with_invalid_dhfile

Fails on OTP-26.0-rc1.

Have not looked into why.

  • ssl_sni_SUITE:sni_enabled
  • ssl_sni_SUITE:sni_strict
  • ssl_sni_SUITE:sni_required_on_vhost
Error: {{assertMatch,
            [{module,ssl_sni_SUITE},
             {line,228},
             {expression,
                 "testsuite : http_get ( Url , [ HostHdr , ConnHdr ] , [ ] , SSLOpts )"},
             {pattern,"{ ok , { { _ , 200 , _ } , _ , Res } }"},
             {value,
                 {error,
                     {tls_alert,
                         {handshake_failure,
                             "TLS client: In state hello received SERVER ALERT: Fatal - Handshake Failure\n"}}}}]},

Fails on OTP-26.0-rc1. Passes on erlang/otp@9e7c030 .

Shows that something is going wrong in the handshake, presumably already in
ssl:handle_options/5 but I haven't found anything conclusive about where or
how SNI handling has changed.

  • websockets_SUITE:secure_socket

Fixed in PR.

OTP changed default verify option in erlang/otp@bb3603d

Adding {verify, verify_none} to websockets_SUITE:sslopen/2 makes the test pass,
which could be ok short term. It is consistent with the previous behavior.

The better long term option is to generate proper TLS data for testing.

@avtobiff
Copy link
Collaborator Author

Regarding {verify, verify_none}, this needs to be changed because
{verify, verify_peer} now doesn't allow {cacerts, undefined} but
requires a trusted cert.

https://www.erlang.org/patches/otp-26.0#ssl-11.0
erlang/otp#5899

@vinoski
Copy link
Collaborator

vinoski commented Dec 27, 2023

Regarding {verify, verify_none}, this needs to be changed because {verify, verify_peer} now doesn't allow {cacerts, undefined} but requires a trusted cert.

Yeah, I've been working on this and found the same. I should probably push my current changes up on a branch, even though they still don't pass all the tests.

@avtobiff
Copy link
Collaborator Author

Changes for OTP 27

* file:pid2name/1 is removed, update yaws_config:fload functions to pass
  config file name.

* Add OTP 26.0, 26.1, and 26.2 to the test matrix.

* Make websockets_SUITE:secure_socket pass

  OTP changed default ssl verify option in

      commit bb3603db8459e13e9e5f27c4fb46ca59ee8e4a39

      ssl: Change client default verify

  Adding {verify, verify_none} to websockets_SUITE:sslopen/2 makes the
  test pass, which could be ok short term. The better long term option
  is to generate proper TLS data for testing.

* Remove unused sni_not_available test

  The test ssl_sni_SUITE:sni_not_available is failing on OTP-26 and it
  has not been relevant since OTP R7.

* Update ssl/mkcert_altname

  * Update openssl mkcert_altname README and config, use SHA-256 and
    2048 bit keys by defualt.

  * alice.sni.example.com-*.pem and yaws.sni.example.com-*.pem

    Used SHA-1 signature algorithm, which OTP ssl doesn't allow anymore.
    This fixes the failing ssl_sni_SUITE tests.

Fixes #467

@avtobiff
Copy link
Collaborator Author

This uncovered a bug in OTP where dhfile was not read from the ssl options at all.
See erlang/otp#7984

When the above OTP PR is merged, Yaws works on OTP-26+. It is tested on both
maint and master (both with the dhfile patch).

@avtobiff
Copy link
Collaborator Author

Perhaps it should be renamed to Changes for OTP-26?

@vinoski vinoski changed the title Changes for OTP 27 Changes for OTP 26 Dec 28, 2023
@vinoski
Copy link
Collaborator

vinoski commented Dec 28, 2023

Perhaps it should be renamed to Changes for OTP-26?

Yes, done.

* file:pid2name/1 is removed, update yaws_config:fload functions to pass
  config file name.

* Add OTP 26.0, 26.1, and 26.2 to the test matrix.

* Make websockets_SUITE:secure_socket pass

  OTP changed default ssl verify option in

      commit bb3603db8459e13e9e5f27c4fb46ca59ee8e4a39

      ssl: Change client default verify

  Adding {verify, verify_none} to websockets_SUITE:sslopen/2 makes the
  test pass, which could be ok short term. The better long term option
  is to generate proper TLS data for testing.

* Remove unused sni_not_available test

  The test ssl_sni_SUITE:sni_not_available is failing on OTP-26 and it
  has not been relevant since OTP R7.

* Update ssl/mkcert_altname

  * Update openssl mkcert_altname README and config, use SHA-256
    signature algorithm and 2048 bit keys by defualt.

  * Regenerate certificates and keys alice.sni.example.com-*.pem and
    yaws.sni.example.com-*.pem.

    Used SHA-1 signature algorithm, which OTP ssl doesn't allow anymore.
    This fixes the failing ssl_sni_SUITE tests.

Fixes erlyaws#467
@vinoski
Copy link
Collaborator

vinoski commented Dec 29, 2023

This uncovered a bug in OTP where dhfile was not read from the ssl options at all. See erlang/otp#7984

Interesting!

When the above OTP PR is merged, Yaws works on OTP-26+. It is tested on both maint and master (both with the dhfile patch).

I guess this means we either have to wait to merge this until the OTP team issues a patch release containing the fix that we can then add to our Github Actions for testing, or we have to adjust the code to not run certain tests on OTP 26 unless we can determine we're running the patched release or newer.

@avtobiff
Copy link
Collaborator Author

Yes I thought about when to merge and possibly handle tests for OTP-26 versions
as well. IMHO we can wait and monitor how the OTP PR progresses and decide later.

@avtobiff
Copy link
Collaborator Author

Pushed a suggestion on how and when to skip the dhfile tests on broken
OTP-26 releases.

The OTP ssl dhfile PR is merged, but I don't know what release it will land in.
Guessing that it will be fixed in the next OTP-26 point release 26.2.2.

Disabling the tests for other OTP-26 releases. Perhaps patch releases
will get the fix as well, if so they can be added to the condition to skip the
dhfile tests.

Guessing that the fix will be released in the next OTP-26 point release,
OTP 26.2.2.
@etnt
Copy link
Collaborator

etnt commented Jan 12, 2024

Looks OK to me.

Copy link
Collaborator

@vinoski vinoski left a comment

Choose a reason for hiding this comment

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

Looks great, thanks so much for getting it all working!

@vinoski vinoski merged commit ef81393 into erlyaws:master Jan 16, 2024
19 checks passed
@avtobiff avtobiff deleted the otp-27 branch January 16, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

build breaks with OTP 27
3 participants