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

Wrong QOP value in UAC auth_alg #1684

Closed
teotwaki opened this issue Oct 24, 2018 · 5 comments
Closed

Wrong QOP value in UAC auth_alg #1684

teotwaki opened this issue Oct 24, 2018 · 5 comments

Comments

@teotwaki
Copy link
Contributor

teotwaki commented Oct 24, 2018

Description

While trying to setup a handoff to a third party SIP trunk with authentication, I ran into some issues with said authentication. I'm using qop auth on the inbound leg (and consume_credentials()), and then UAC to authenticate against the SIP trunk with different credentials. The third party SIP trunk offers qop auth/auth-int.

I manually tried to verify the digest that Kamailio was sending, but couldn't. I added some logging to the auth_alg.c file, and ran the whole thing again. HA1 is calculated correctly, and so is HA2. However, the final step of the algorithm produced an incorrect value.

After further debugging, I realised that the issue comes from https://github.com/kamailio/kamailio/blob/master/src/modules/uac/auth_alg.c#L151. More specifically, in my case, the value passed on to MD5Update is auth,auth-int, instead of the simple auth I was expecting (uac doesn't support auth-int).

Troubleshooting

SIP Traffic

I have PCAPs demonstrating the issue and the hack/fix below. I'd rather not post them publicly, however, I am happy to share them privately with Kamailio devs.

In essence:

-> INVITE sip:12345@someprovider.org:5060;transport=tcp
<- 401 Unauthorized
    WWW-Authenticate: Digest realm="someprovider.org", qop="auth,auth-int", nonce="5BCF48671749b873534dc63e76d5594f3988555f"
-> INVITE sip:12345@someprovider.org:5060;transport=tcp
    Authorization: Digest username="redacted", realm="someprovider.org", nonce="5BCF48671749b873534dc63e76d5594f3988555f", uri="sip:12345@someprovider.org:5060;transport=tcp", qop=auth, nc=00000001, cnonce="2106889321", response="redacted71d4c9ce119dcf3ec56209e", algorithm=MD5

Possible Solutions

The hack I've used for the time being is to replace:

MD5Update(&Md5Ctx, ":", 1);
MD5Update(&Md5Ctx, auth->qop.s, auth->qop.len);
MD5Update(&Md5Ctx, ":", 1);

with:

MD5Update(&Md5Ctx, ":auth:", 6);

Obviously, I realise this is not the correct fix, but I figured I'd let the experts fix the value of auth->qop.len. I can spend more time on this, if required, but I don't know exactly what the "correct" solution is.

Additional Information

  • Kamailio Version - output of kamailio -v
version: kamailio 5.1.6 (x86_64/linux) 
flags: STATS: Off, USE_TCP, USE_TLS, USE_SCTP, TLS_HOOKS, USE_RAW_SOCKS, DISABLE_NAGLE, USE_MCAST, DNS_IP_HACK, SHM_MEM, SHM_MMAP, PKG_MALLOC, Q_MALLOC, F_MALLOC, TLSF_MALLOC, DBG_SR_MEMORY, USE_FUTEX, FAST_LOCK-ADAPTIVE_WAIT, USE_DNS_CACHE, USE_DNS_FAILOVER, USE_NAPTR, USE_DST_BLACKLIST, HAVE_RESOLV_RES
ADAPTIVE_WAIT_LOOPS=1024, MAX_RECV_BUFFER_SIZE 262144 MAX_URI_SIZE 1024, BUF_SIZE 65535, DEFAULT PKG_SIZE 8MB
poll method support: poll, epoll_lt, epoll_et, sigio_rt, select.
id: unknown 
compiled with gcc 5.3.1
  • Operating System:

Dockerized version of Kamailio on Ubuntu xenial:

kamailio/kamailio:5.1.6-xenial
@miconda
Copy link
Member

miconda commented Oct 26, 2018

I pushed a patch for it (see the reference above) in master branch. Can you test and if all ok, then I will back port to 5.1 branch.

@miconda
Copy link
Member

miconda commented Nov 2, 2018

Any chance to test this one soon, in the near future we will do another 5.1.x release and it would be good to know if works in order to backport.

@teotwaki
Copy link
Contributor Author

teotwaki commented Nov 2, 2018

Hi,

Sorry, I’ll try to test it today and get back to you.

Thanks

@teotwaki
Copy link
Contributor Author

teotwaki commented Nov 6, 2018

@miconda Currently testing this, however, looking at the patch, I guess this assumes that "auth" would always be first? If the server sends auth-int,auth, this would still fail.

I don't know if the standards say anything about the order, however.

@teotwaki
Copy link
Contributor Author

teotwaki commented Nov 6, 2018

@miconda I can confirm that the proposed fix works (for my use-case).

@miconda miconda closed this as completed Nov 19, 2018
miconda added a commit that referenced this issue Dec 4, 2018
miconda added a commit that referenced this issue Apr 3, 2019
- GH #1684

(cherry picked from commit 6e6a2f4)
(cherry picked from commit 64afdf2)
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

No branches or pull requests

2 participants