Closed jschwartzenberg closed 4 years ago
Thanks for the bug report.
I fixed the issue in the latest commit. Now when you pass a PAC file, a parent proxy is no longer required. Let me know, if it works for you.
Regarding rebasing: Thanks for letting me know. I gonna look into the other fork of Cntlm.
You're welcome! I'll try it soon.
Let me know if you'd like help with the rebasing. I had been searching around a bit to see if there were still any maintained forks of Cntlm and that one was still getting updates. There was also a Kerberos patch on SourceForge which I rebased and it was just merged! It would be great to see all the different patches that are going around come together in one branch :)
I've just pushed the rebased version. I tested it one entire day with no segfault whatsoever. However, I still gonna add some features (automatic fetch/update of PAC files, configuration over conf file) and refactor here and there, but I am not sure when I'll find the time to do it. If that isn't a problem, my patches could be merged into versat's cntlm as well... :)
Wow, that's great!! Yeah I would say just create a PR to him and see what happens :) That will also trigger some checks that might help to make the code better. More feature could always come from later PRs.
I just tried the combination with Kerberos and a PAC file, but it segfaults. Do you have a possibility to test with Kerberos? Let me know if I should arrange debug info!
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff7ff7700 (LWP 19529)]
0x00007ffff73158f7 in __strcpy_sse2_unaligned () from /lib64/libc.so.6
Missing separate debuginfos, use: debuginfo-install glibc-2.17-292.el7.x86_64 keyutils-libs-1.5.8-3.el7.x86_64 krb5-libs-1.15.1-37.el7_7.2.x86_64 libcom_err-1.42.9-16.el7.x86_64 libselinux-2.5-14.1.el7.x86_64 pcre-8.32-17.el7.x86_64
(gdb) bt
#0 0x00007ffff73158f7 in __strcpy_sse2_unaligned () from /lib64/libc.so.6
#1 0x000000000041589d in acquire_kerberos_token ()
#2 0x000000000040f08a in proxy_authenticate ()
#3 0x000000000040f9ac in forward_request ()
#4 0x0000000000410578 in pac_forward_request ()
#5 0x0000000000413262 in proxy_thread ()
#6 0x00007ffff7bc6ea5 in start_thread () from /lib64/libpthread.so.0
#7 0x00007ffff73798cd in clone () from /lib64/libc.so.6
Here's the proper debug output:
(gdb) bt
#0 0x00007ffff73158f7 in __strcpy_sse2_unaligned () from /lib64/libc.so.6
#1 0x000000000041589d in acquire_kerberos_token (proxy=0x0, credentials=credentials@entry=0x7ffff0001f00, buf=buf@entry=0x7ffff00022e0 "") at kerberos.c:274
#2 0x000000000040f08a in proxy_authenticate (sd=0x7ffff7ff7cd0, request=0x7ffff0001600, response=0x7ffff0002270, credentials=0x7ffff0001f00) at forward.c:229
#3 0x000000000040f9ac in forward_request (thread_data=thread_data@entry=0x666dd0, request=0x0, request@entry=0x7ffff00008c0, proxy=proxy@entry=0x7ffff0001b80) at forward.c:666
#4 0x0000000000410578 in pac_forward_request (thread_data=thread_data@entry=0x666dd0, request=0x7ffff00008c0, proxy_list=proxy_list@entry=0x7ffff0001750) at forward.c:97
#5 0x0000000000413262 in proxy_thread (thread_data=0x666dd0) at main.c:493
#6 0x00007ffff7bc6ea5 in start_thread () from /lib64/libpthread.so.0
#7 0x00007ffff73798cd in clone () from /lib64/libc.so.6
It seems I found the issue: https://github.com/jschwartzenberg/cntlm/commit/a0a7379ee05cf23425131260341e178d1b327510
Maybe you want to create a PR to versat's repo? Otherwise I can also do it.
Cool! Sorry, I am bit busy with other stuff. It would be great, if you created a PR. I would love to see it being merged. Thanks.
Sure, no problem! I'll see if I can create a PR that gets merged. I imagine it might be necessary to allow disabling this feature compile time (similar to Kerberos), but I guess I could add that similarly.
In any case, thanks a lot for your work! It is really appreciated!!
Hi @olfryd just letting you know that my PR was merged! Thanks a lot for the work. If you add the e-mail address you used in the commits to your GitHub account, this will also be reflected. It seems we might not be very far apart from one another. I guess the combination of PAC and NTLM is popular around here ;)
Hey @jschwartzenberg. Great news. Really happy to hear that. :) One thing, I just noticed that pushed my commits with my work mail address which I really would like to undo. I already rebased my commits in my repository to reflect this.
Could you do the same with the pull requests / commits in Versats repo? I changed my name and email to "Oliver Diehl oliverdiehl@gmx.de". I would appreciate it.
It seems we might not be very far apart from one another.
Where do you live? In the Netherlands? :) I live in Hamburg.
I guess the combination of PAC and NTLM is popular around here ;)
Happy to hear that others can make use of it too. :)
Could you do the same with the pull requests / commits in Versats repo? I changed my name and email to "Oliver Diehl oliverdiehl@gmx.de". I would appreciate it.
I did this for my own fork now, but this requires a forced push to be able to rewrite history. @versat would you be ok with a forced push on the main branch to replace the e-mail address?
Where do you live? In the Netherlands? :) I live in Hamburg.
Yeah, I'm close to Eindhoven ;)
Could you do the same with the pull requests / commits in Versats repo? I changed my name and email to "Oliver Diehl oliverdiehl@gmx.de". I would appreciate it.
I did this for my own fork now, but this requires a forced push to be able to rewrite history. @versat would you be ok with a forced push on the main branch to replace the e-mail address?
Yes, would be good to fix that 👍
Alright, I'll merge it with my later bugfix too then.
Could you do the same with the pull requests / commits in Versats repo?
Done for versat's repo as well, do the following if you had locally pulled the latest master inbetween:
git checkout master
git reset 1db61869063bc807e8e5bcaeac10bfee45db2a1a --hard
git pull
Otherwise you'll get errors.
Ok, thanks.
When I run it with a PAC file, I get this output:
I would expect the parent proxy to be retrieved from the PAC file?
Btw. would you consider rebasing this against a more maintained version of Cntlm? This fork had a lot of work done on it: https://github.com/versat/cntlm