SIPp / sipp

The SIPp testing tool
https://sipp.readthedocs.io
Other
918 stars 380 forks source link

Allow enabling of TLS verification without supplying CRL file #663

Closed IvanRibakov closed 8 months ago

IvanRibakov commented 11 months ago

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 commented 8 months ago

LGTM

IvanRibakov commented 8 months ago

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

wdoekes commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

Fair enough.

wdoekes commented 8 months ago

Thanks! :+1: