Skip to content

fix tls_mgm release domain issue#3629

Merged
liviuchircu merged 1 commit intoOpenSIPS:masterfrom
36952362:master
Apr 17, 2025
Merged

fix tls_mgm release domain issue#3629
liviuchircu merged 1 commit intoOpenSIPS:masterfrom
36952362:master

Conversation

@36952362
Copy link
Copy Markdown
Contributor

@36952362 36952362 commented Apr 16, 2025

Summary
This PR fixes the issue where the contents in client_dom_matching or server_dom_matching are not fully released after executing the tls_reload command.

Details
Assuming the content of tls_mgm config file which used by tls_mgm module as follows, where 127.0.0.1 and 127.0.0.2 are associated with dom1 via match_sip_domain.

id(int,auto) domain(string) match_ip_address(string,null) match_sip_domain(string,null) type(int) method(string,null) verify_cert(int,null) require_cert(int,null) certificate(blob,null) private_key(blob,null) crl_check_all(int,null) crl_dir(string,null) ca_list(blob,null) ca_dir(string,null) cipher_list(string,null) dh_params(blob,null) ec_curve(string,null) 

1:dom1::127.0.0.1,127.0.0.2:1::1:1:/tmp/cert.pem:/tmp/key.pem:0::::::
2:dom2::127.0.0.3:1::1:1:/tmp/cert.pem:/tmp/key.pem:0::::::

After the tls_mgm module has been loaded, the dom_filt_array in client_dom_matching is stored in the following format like:

doms_array->size = 3

doms_array->arr[0].dom_link ->dom1

doms_array->arr[1].dom_link ->dom1

doms_array->arr[2].dom_link ->dom2

When performing the tls_reload operation, it is necessary to call tls_free_db_domains to release the old domain data, including the data in client_dom_matching.

reload_data -> tls_free_db_domains -> map_remove_tls_dom
However, during the loop iteration in map_remove_tls_dom, the removal is incomplete. In the example mentioned above, the data associated with 127.0.0.2 would not be deleted. This can lead to an even worse situation — executing tls_reload again may cause OpenSIPS to crash.

Solution
Adjust iteration logic in map_remove_tls_dom to make sure the data can be release completely

Compatibility
This is a change-specific and isolated in tls_mgm module - it's 100% retro-compatible with the other versions.

Closing issues

Copy link
Copy Markdown
Member

@liviuchircu liviuchircu left a comment

Choose a reason for hiding this comment

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

LGTM and a very good catch!!

PS: An alternate fix would be to just i--; after the doms_array->size--;, but your approach is completely fine too.

@liviuchircu liviuchircu self-assigned this Apr 17, 2025
@liviuchircu liviuchircu added this to the 3.4.12 milestone Apr 17, 2025
@liviuchircu liviuchircu merged commit f0054a2 into OpenSIPS:master Apr 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants