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

Allow enabling of TLS verification without supplying CRL file #663

Merged
merged 4 commits into from
Jan 26, 2024

Conversation

IvanRibakov
Copy link
Contributor

Currently CRL file specified via tls_crl_name flag is being loaded even if it wasn't supplied, causing TLS initialisation to fail:

    if (strlen(tls_ca_name) != 0 || strlen(tls_crl_name) != 0) {
        if (sip_tls_load_crls(sip_trp_ssl_ctx, tls_crl_name) == -1) {

This PR ensures that custom behaviour caused by SSL_VERIFY_PEER can be enabled without supplying CRL file.

@lemenkov
Copy link
Member

LGTM

@IvanRibakov
Copy link
Contributor Author

@wdoekes Any chance this can make it into the next patch release?

@wdoekes
Copy link
Member

wdoekes commented Jan 26, 2024

I think it looks good.

No sudden camelCase please though: gotCAFile

And not sure why the "\n" in the WARNING.

Right now I count:

$ wgrep . 'WARNING("[^"]*[^n]",'  | wc -l
87

87 WARNINGs without LF and

$ wgrep . 'WARNING("[^"]*[n]",'  | wc -l
5

5 ones with.

Are all the other WARNINGs without LF bad?

@IvanRibakov
Copy link
Contributor Author

No sudden camelCase please though: gotCAFile

My bad, will fix!

Are all the other WARNINGs without LF bad?

Yes, they cause log messages to concatenate in one long line.

@wdoekes
Copy link
Member

wdoekes commented Jan 26, 2024

Yes, they cause log messages to concatenate in one long line.

Fair enough.

@wdoekes wdoekes merged commit 65cd1fd into SIPp:master Jan 26, 2024
@wdoekes
Copy link
Member

wdoekes commented Jan 26, 2024

Thanks! 👍

@orgads orgads mentioned this pull request Sep 17, 2024
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.

3 participants