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

Add server_name_indication_host config option #3510

Merged
merged 6 commits into from
Jan 26, 2022
Merged

Add server_name_indication_host config option #3510

merged 6 commits into from
Jan 26, 2022

Conversation

gustawlippa
Copy link
Contributor

@gustawlippa gustawlippa commented Jan 20, 2022

The SNI options are formatted to be useful to OTP's ssl library, so in any place where they are passed down to it in the end, this should work.

This PR makes the option server_name_indication sensible - one can set it to either true OR false. And when using verify_peer SNI is (in most cases) needed, so there needs to be a way to pass the domain in the config. Because of the awkward way ssl expects an atom disable or the domain name, there is a need for a new option server_name_indication_host in TOML, and the subsequent parsing.

In the future could be used by other places where TLS can be set, but it's a can of worms, we have like five different ways to configure TLS in TOML, and quite a few libraries/abstractions, which all expect a little bit different options passed to them.

@mongoose-im
Copy link
Collaborator

mongoose-im commented Jan 20, 2022

small_tests_24 / small_tests / 483b738
Reports root / small


small_tests_23 / small_tests / 483b738
Reports root / small


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 483b738
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 483b738
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / 483b738
Reports root/ big
OK: 2674 / Failed: 0 / User-skipped: 255 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 483b738
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / 483b738
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / 483b738
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / 483b738
Reports root/ big
OK: 1541 / Failed: 0 / User-skipped: 349 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / 483b738
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 483b738
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 483b738
Reports root/ big
OK: 1834 / Failed: 0 / User-skipped: 363 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / 483b738
Reports root/ big
OK: 3073 / Failed: 0 / User-skipped: 252 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / 483b738
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / 483b738
Reports root/ big
OK: 1680 / Failed: 0 / User-skipped: 363 / Auto-skipped: 0

@codecov
Copy link

codecov bot commented Jan 20, 2022

Codecov Report

Merging #3510 (f175761) into master (949e047) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3510      +/-   ##
==========================================
+ Coverage   80.98%   81.00%   +0.01%     
==========================================
  Files         419      419              
  Lines       32312    32319       +7     
==========================================
+ Hits        26168    26179      +11     
+ Misses       6144     6140       -4     
Impacted Files Coverage Δ
src/config/mongoose_config_spec.erl 99.22% <100.00%> (+0.02%) ⬆️
src/metrics/mongoose_metrics.erl 86.62% <0.00%> (-5.82%) ⬇️
src/logger/mongoose_log_filter.erl 78.08% <0.00%> (-1.37%) ⬇️
src/mod_muc_log.erl 78.11% <0.00%> (ø)
src/pubsub/mod_pubsub.erl 73.31% <0.00%> (+0.11%) ⬆️
src/rdbms/mongoose_rdbms.erl 63.29% <0.00%> (+0.37%) ⬆️
src/pubsub/mod_pubsub_db_mnesia.erl 92.85% <0.00%> (+0.42%) ⬆️
src/mam/mod_mam_rdbms_arch_async.erl 97.77% <0.00%> (+1.11%) ⬆️
src/mam/mod_mam_elasticsearch_arch.erl 86.84% <0.00%> (+1.75%) ⬆️
src/cassandra/mongoose_cassandra_worker.erl 68.54% <0.00%> (+2.81%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 949e047...f175761. Read the comment docs.

Copy link
Member

@chrzaszcz chrzaszcz left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of it. I think a small correction is needed.

src/config/mongoose_config_spec.erl Outdated Show resolved Hide resolved
src/config/mongoose_config_spec.erl Outdated Show resolved Hide resolved
Remove process_sni/1, refactor process_tls_sni/1, enable SNI host in more places.
@mongoose-im
Copy link
Collaborator

mongoose-im commented Jan 25, 2022

small_tests_24 / small_tests / 1c17815
Reports root / small


small_tests_23 / small_tests / 1c17815
Reports root / small


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 1c17815
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 1c17815
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 1c17815
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / 1c17815
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / 1c17815
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 1c17815
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / 1c17815
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / 1c17815
Reports root/ big
OK: 1541 / Failed: 0 / User-skipped: 349 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 1c17815
Reports root/ big
OK: 1834 / Failed: 0 / User-skipped: 363 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / 1c17815
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / 1c17815
Reports root/ big
OK: 3073 / Failed: 0 / User-skipped: 252 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / 1c17815
Reports root/ big
OK: 1680 / Failed: 0 / User-skipped: 363 / Auto-skipped: 0

@gustawlippa gustawlippa changed the title Add server_name_indication_host config option for outgoing pools Add server_name_indication_host config option Jan 25, 2022
Copy link
Member

@chrzaszcz chrzaszcz left a comment

Choose a reason for hiding this comment

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

Looks good in general, I only have minor remarks.

src/config/mongoose_config_spec.erl Outdated Show resolved Hide resolved
doc/configuration/outgoing-connections.md Outdated Show resolved Hide resolved
gustawlippa and others added 2 commits January 26, 2022 16:05
Co-authored-by: Paweł Chrząszcz <pawel.chrzaszcz@erlang-solutions.com>
@mongoose-im
Copy link
Collaborator

mongoose-im commented Jan 26, 2022

small_tests_24 / small_tests / 8365480
Reports root / small


small_tests_23 / small_tests / 8365480
Reports root / small


internal_mnesia_24 / internal_mnesia / 8365480
Reports root


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 8365480
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / 8365480
Reports root/ big
OK: 2674 / Failed: 0 / User-skipped: 255 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 8365480
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / 8365480
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / 8365480
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 8365480
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 8365480
Reports root/ big
OK: 1834 / Failed: 0 / User-skipped: 363 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / 8365480
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / 8365480
Reports root/ big
OK: 3073 / Failed: 0 / User-skipped: 252 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / 8365480
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / 8365480
Reports root/ big
OK: 1680 / Failed: 0 / User-skipped: 363 / Auto-skipped: 0

@mongoose-im
Copy link
Collaborator

mongoose-im commented Jan 26, 2022

small_tests_24 / small_tests / f175761
Reports root / small


small_tests_23 / small_tests / f175761
Reports root / small


internal_mnesia_24 / internal_mnesia / f175761
Reports root


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / f175761
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / f175761
Reports root/ big
OK: 2674 / Failed: 0 / User-skipped: 255 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / f175761
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / f175761
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / f175761
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / f175761
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / f175761
Reports root/ big
OK: 1834 / Failed: 0 / User-skipped: 363 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / f175761
Reports root/ big
OK: 3073 / Failed: 0 / User-skipped: 252 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / f175761
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / f175761
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / f175761
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / f175761
Reports root/ big
OK: 1680 / Failed: 0 / User-skipped: 363 / Auto-skipped: 0

Copy link
Member

@chrzaszcz chrzaszcz left a comment

Choose a reason for hiding this comment

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

Looks good! The CI failures are caused by minio (fixed in master) and CircleCI outages.

@chrzaszcz chrzaszcz merged commit 6f72056 into master Jan 26, 2022
@chrzaszcz chrzaszcz deleted the fix-sni branch January 26, 2022 17:34
@Premwoik Premwoik added this to the 5.1.0 milestone May 25, 2022
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.

4 participants