Skip to content

INT-6557: Allow wildcard pjproject TLS certs#11

Merged
MattMacGregor merged 1 commit intointercom/release-2-13from
mattmacgregor/INT-6557---allow-wildcard-pjproject
Nov 5, 2025
Merged

INT-6557: Allow wildcard pjproject TLS certs#11
MattMacGregor merged 1 commit intointercom/release-2-13from
mattmacgregor/INT-6557---allow-wildcard-pjproject

Conversation

@MattMacGregor
Copy link

Twilio uses a wildcard TLS cert. If we want to use TLS with them and properly verify their cert, we need to allow these certs, despite them technically not being allowed by the SIP RFC.

Regardless, its better than not verifying the cert as a whole, which is the other option since we are stuck with Twilio as a primary SIP provider.

@linear
Copy link

linear bot commented Oct 28, 2025

@MattMacGregor MattMacGregor changed the base branch from pjsip/release-2-13 to intercom/release-2-13 October 28, 2025 19:00
@MattMacGregor MattMacGregor force-pushed the mattmacgregor/INT-6557---allow-wildcard-pjproject branch from 0465c8b to 83e5545 Compare October 28, 2025 19:02
Comment on lines +1928 to +1942
if (cert_name->slen > 2 && cert_name->ptr[0] == '*' && cert_name->ptr[1] == '.') {
if (!wildcard_adjusted_remote_set) {
wildcard_adjusted_remote = *remote_name;
remove_first_subdomain(&wildcard_adjusted_remote);
wildcard_adjusted_remote_set = PJ_TRUE;
}
pj_str_t wildcard_adjusted_cert_name = *cert_name;
wildcard_adjusted_cert_name.ptr += 2;
wildcard_adjusted_cert_name.slen -= 2;

matched = !pj_stricmp(&wildcard_adjusted_remote, &wildcard_adjusted_cert_name);
} else {
matched = !pj_stricmp(remote_name, cert_name);
}
break;
Copy link
Author

Choose a reason for hiding this comment

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

If it starts with *., remove the first subdomain of the remote name. This *. can only match one subdomain. We also are only considering if the cert name and remote name are both longer than 2, so *. and * in the cert we are not considering as valid (neither should happen ever, technically not sure if they are considered valid by the spec but I imagine not).

Comment on lines +1929 to +1933
if (!wildcard_adjusted_remote_set) {
wildcard_adjusted_remote = *remote_name;
remove_first_subdomain(&wildcard_adjusted_remote);
wildcard_adjusted_remote_set = PJ_TRUE;
}
Copy link
Author

Choose a reason for hiding this comment

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

might as well optimize to only do this once.

Comment on lines +1968 to +1981
if (serv_cert->subject.cn.slen > 2 && serv_cert->subject.cn.ptr[0] == '*' && serv_cert->subject.cn.ptr[1] == '.') {
if (!wildcard_adjusted_remote_set) {
wildcard_adjusted_remote = *remote_name;
remove_first_subdomain(&wildcard_adjusted_remote);
wildcard_adjusted_remote_set = PJ_TRUE;
}
pj_str_t wildcard_adjusted_cn = serv_cert->subject.cn;
wildcard_adjusted_cn.ptr += 2;
wildcard_adjusted_cn.slen -= 2;

matched = !pj_stricmp(&wildcard_adjusted_remote, &wildcard_adjusted_cn);
} else {
matched = !pj_stricmp(remote_name, &serv_cert->subject.cn);
}
Copy link
Author

Choose a reason for hiding this comment

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

Repeat the same logic for the CN.

@semgrep-code-verkada
Copy link

Semgrep found 1 dont-call-system finding:

  • pjmedia/src/pjmedia-codec/intercom_codec.c

Don't call system. It's a high-level wrapper that allows for stacking multiple commands. Always prefer a more restrictive API such as calling execve from the exec family.

@MattMacGregor MattMacGregor force-pushed the mattmacgregor/INT-6557---allow-wildcard-pjproject branch from 8a4d796 to 6c6a889 Compare November 5, 2025 02:31
@MattMacGregor MattMacGregor force-pushed the mattmacgregor/INT-6557---allow-wildcard-pjproject branch from 6c6a889 to 5b3664f Compare November 5, 2025 02:31
@MattMacGregor MattMacGregor merged commit 64a3ed2 into intercom/release-2-13 Nov 5, 2025
23 of 55 checks passed
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