[squid-dev] [PATCH] splicing resumed sessions
This patch adds the ssl_bump_resuming_sessions directive that controls SslBump behavior when dealing with resuming SSL/TLS sessions. Without these changes, SslBump usually terminates all resuming sessions with an error because such sessions do not include server certificates, preventing Squid from successfully validating the server identity. After these changes, Squid either terminates or splices resuming sessions, depending on configuration. Splicing is the right default because Squid most likely has spliced the original connections that the client and server are trying to resume now. Most likely, the splicing decision would not change now (but the lack of the server certificate information means we cannot repeat the original ACL checks and need a special directive to tell Squid what to do). Also, without SslBump, session resumption would just work, and SslBump default should approach that ideal. In many deployment scenarios, this straightforward splice or terminate resuming sessions implementation is exactly what the admin wants. Future projects may add more complex algorithms, including maintaining an SMP-shared cache of sessions that may be resumed in the future and evaluating client/server attempts to resume a session using that cache. Example: # splice all resuming sessions [this is the default] ssl_bump_resuming_sessions allow all This patch also makes SSL client Hello message parsing more robust and adds an SSL server Hello message parser. This patch also prevents occasional segfaults when dealing with SSL cache_peer negotiation failures. The last two changes should applied to squid-3.5 even if this patch will not go into squid-3.5. Regards, Christos Added ssl_bump_resuming_sessions to control treatment of resuming sessions by SslBump. This patch adds the ssl_bump_resuming_sessions directive that controls SslBump behavior when dealing with resuming SSL/TLS sessions. Without these changes, SslBump usually terminates all resuming sessions with an error because such sessions do not include server certificates, preventing Squid from successfully validating the server identity. After these changes, Squid either terminates or splices resuming sessions, depending on configuration. Splicing is the right default because Squid most likely has spliced the original connections that the client and server are trying to resume now. Most likely, the splicing decision would not change now (but the lack of the server certificate information means we cannot repeat the original ACL checks and need a special directive to tell Squid what to do). Also, without SslBump, session resumption would just work, and SslBump default should approach that ideal. In many deployment scenarios, this straightforward splice or terminate resuming sessions implementation is exactly what the admin wants. Future projects may add more complex algorithms, including maintaining an SMP-shared cache of sessions that may be resumed in the future and evaluating client/server attempts to resume a session using that cache. Example: # splice all resuming sessions [this is the default] ssl_bump_resuming_sessions allow all This patch also makes SSL client Hello message parsing more robust and adds an SSL server Hello message parser. This patch also prevents occasional segfaults when dealing with SSL cache_peer negotiation failures. Technical details - In Peek mode, the old Squid code would forward the client Hello message to the server. If the server tries to resume the previous (spliced) SSL session with the client, then Squid SSL code gets an ssl/PeerConnector.cc ccs received early error (or similar) because the Squid SSL object expects a server certificate and does not know anything about the session being resumed. With this patch, Squid detects session resumption attempts and consults the ssl_bump_resuming_sessions access list to decide whether to splice or honor the error. Honoring the error would usually terminate the client and server connections. Session resumption detection There are two mechanism in SSL/TLS for resuming sessions. The traditional shared session IDs and the TLS ticket extensions: * If Squid detects a shared ID in both client and server Hello messages, then Squid decides whether the session is being resumed by comparing those client and server shared IDs. If (and only if) the IDs are the same, then Squid assumes that it is dealing with a resuming session (using session IDs). * If Squid detects a TLS ticket in the client Hello message and TLS ticket support in the server Hello message as well as a Change Cipher Spec or a New TLS Ticket message (following the server Hello message), then (and only then) Squid assumes that it is dealing with a resuming session (using TLS tickets). The TLS tickets check is not performed if Squid detects a shared session ID in both client and server Hello messages. This is a Measurement Factory
Re: [squid-dev] [PATCH] start workers as root
A patch which solves this problem applied to trunk as rev13984.
The patch is different that the patch I posted here.
It just adds enter_suid() calls after the writePidFile and removePidFile()
inside the watch_child() function.
Regards,
Christos
On 03/09/2015 11:09 AM, Tsantilas Christos wrote:
On 03/08/2015 05:57 AM, Amos Jeffries wrote:
On 8/03/2015 6:34 a.m., Tsantilas Christos wrote:
On 03/07/2015 07:18 AM, Amos Jeffries wrote:
On 7/03/2015 12:18 a.m., Tsantilas Christos wrote:
SMP workers in trunk start without root privileges. This results in
startup failures when workers need to use a privileged port
(e.g., 443)
or other root-only features such as TPROXY.
This bug added with my Moved PID file management from Coordinator to
Master patch.
The problem is inside watch_child function which called after a
enter_suid() call, but the writePidFile() call, inside the
watch_child(), will leave suid mode before exit.
This patch removes the enter_suid/leave_suid cals from the
writePidFile
and make the caller responsible for setting the root privileges if
required.
I think this is wrong approach.
Firstly, what are processes without SUID ability doing writing to
secure
system files?
What do you mean here?
After your patch that makes the master file the one writing the PID file
what are the workers (non-master) doing writing to it?
The workers does not write the PID file.
A single-process squid (eg when started with the -N parameter)
still need to write PID file.
Secondly, I thought the entire point of the earlier patch was to make
the *MASTER* process was the one writing the PID file. Not
low-privileged workers.
Yes the master process writing the pid file, not the workers.
Thirdly, the enter/leave_suid calls mean dangerous security stuff
about
to happen and should only be called if absolutely necessary, AND only
around the (block of) system calls which require them.
I agree.
However the watch_child which is implements the master process, is
designed to run in suid mode. This was not changed by the PID patch.
Just due to a bug added with the PID patch this function leaves the
suid
mode.
I think I understand you there. You mean this process:
enter_suid()
...
watch_child()
...
writePidFile() {
enter_squid() // no-op
...
leave_suid()
}
... something that needs suid. oops.
Yep.
Maybe we want to fix master process to not run in suid mode, but I
believe:
- This is not a scope for this patch
- The master process does not do a lot of thinks. Probably we do not
need to make it run with low privileges. Moreover we may have problems
with the kids. (Is it possible for kids to run with different
cache_effective_user parameter?)
Yes, more than possible - probable that somebody will or is already
doing so. Just the other day I saw someone running workers with
different PID files.
Thats the kind of thing that happens with these directives available in
via squid.conf per-worker.
Your description sounds like some part of the code in worker scope is
using enter_suid doing a lot of Squid stuff - plus incidentally some
root system stuff, then leave_suid. That is broken code. None of the
general Squid stuff are security sentitive system calls needing root
privileges.
No, please forget the workers. This patch does not change anything in a
worker.
Please take a look into the writePidFile function, and into watch_child
function (which implements the master process)
The place your patch is changing are all in the workers code.
Instead of moving the enter_suid() outside of writePidFile() in several
places the master code should call enter only to preserve the part of
code *it* needs suid. eg.
watch_child()
...
writePidFile()
enter_suid(); // writePidFile() uses leave_suid()
...
Much simpler patch, and fewer places overall using enter/leave_suid.
Well, still a writePidFile call in the future may cause similar bug.
The need of calling an enter_suid() after writePidFile, is not
something a developer expects.
However I have no problem for this patch too, it is OK for me.
If you prefer it I will apply this patch instead.
Amos
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
[squid-dev] Build failed in Jenkins: trunk-matrix » clang,j-fbsd-93 #141
See http://build.squid-cache.org/job/trunk-matrix/compiler=clang,label=j-fbsd-93/141/ -- [...truncated 2982 lines...] ccache clang++ -DHAVE_CONFIG_H-I../../../../.. -I../../../../../include -I../../../../../lib -I../../../../../src -I../../../include -I/usr/local/include -I/usr/include -I/usr/include -I../../../../../libltdl -Werror -Qunused-arguments -Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -I/usr/local/include -MT basic_pam_auth.o -MD -MP -MF .deps/basic_pam_auth.Tpo -c -o basic_pam_auth.o ../../../../../helpers/basic_auth/PAM/basic_pam_auth.cc mv -f .deps/basic_pam_auth.Tpo .deps/basic_pam_auth.Po /bin/sh ../../../libtool --tag=CXX--mode=link ccache clang++ -Werror -Qunused-arguments -Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -I/usr/local/include -g -L/usr/local/lib -Wl,-R/usr/local/lib -pthread -o basic_pam_auth basic_pam_auth.o ../../../lib/libmiscencoding.la ../../../compat/libcompat-squid.la -lpam -lm libtool: link: ccache clang++ -Werror -Qunused-arguments -Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -I/usr/local/include -g -Wl,-R/usr/local/lib -pthread -o basic_pam_auth basic_pam_auth.o -L/usr/local/lib ../../../lib/.libs/libmiscencoding.a ../../../compat/.libs/libcompat-squid.a -lpam -lm -pthread Making all in POP3 sed -e 's,[@]PERL[@],/usr/bin/perl,g' ../../../../../helpers/basic_auth/POP3/basic_pop3_auth.pl.in basic_pop3_auth || (/bin/rm -f -f basic_pop3_auth ; exit 1) pod2man basic_pop3_auth basic_pop3_auth.8 Making all in RADIUS ccache clang++ -DHAVE_CONFIG_H-I../../../../.. -I../../../../../include -I../../../../../lib -I../../../../../src -I../../../include -I/usr/local/include -I/usr/include -I/usr/include -I../../../../../libltdl -I../../../../../helpers/basic_auth/RADIUS -Werror -Qunused-arguments -Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -I/usr/local/include -MT radius-util.o -MD -MP -MF .deps/radius-util.Tpo -c -o radius-util.o ../../../../../helpers/basic_auth/RADIUS/radius-util.cc ccache clang++ -DHAVE_CONFIG_H-I../../../../.. -I../../../../../include -I../../../../../lib -I../../../../../src -I../../../include -I/usr/local/include -I/usr/include -I/usr/include -I../../../../../libltdl -I../../../../../helpers/basic_auth/RADIUS -Werror -Qunused-arguments -Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -I/usr/local/include -MT basic_radius_auth.o -MD -MP -MF .deps/basic_radius_auth.Tpo -c -o basic_radius_auth.o ../../../../../helpers/basic_auth/RADIUS/basic_radius_auth.cc mv -f .deps/basic_radius_auth.Tpo .deps/basic_radius_auth.Po mv -f .deps/radius-util.Tpo .deps/radius-util.Po /bin/sh ../../../libtool --tag=CXX--mode=link ccache clang++ -Werror -Qunused-arguments -Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -I/usr/local/include -g -L/usr/local/lib -Wl,-R/usr/local/lib -pthread -o basic_radius_auth basic_radius_auth.o radius-util.o ../../../lib/libmiscencoding.la ../../../compat/libcompat-squid.la -lnettle -lm libtool: link: ccache clang++ -Werror -Qunused-arguments -Wno-deprecated-register -D_REENTRANT -g -O2 -std=c++11 -I/usr/local/include -g -Wl,-R/usr/local/lib -pthread -o basic_radius_auth basic_radius_auth.o radius-util.o -L/usr/local/lib ../../../lib/.libs/libmiscencoding.a ../../../compat/.libs/libcompat-squid.a -lnettle -lm -pthread Making all in SMB ccache clang++ -DHAVE_CONFIG_H -I../../../../.. -I../../../../../include -I../../../../../lib -I../../../../../src -I../../../include -I/usr/local/include -I/usr/include -I/usr/include -I../../../../../libltdl -DHELPERSCRIPT=\/usrhttp://build.squid-cache.org/job/trunk-matrix/compiler=clang,label=j-fbsd-93/ws/btlayer-00-default/squid-4.0.0-BZR/_inst/libexec/basic_smb_auth.sh\; -g -O2 -std=c++11 -I/usr/local/include -MT basic_smb_auth-basic_smb_auth.o -MD -MP -MF .deps/basic_smb_auth-basic_smb_auth.Tpo -c -o basic_smb_auth-basic_smb_auth.o `test -f 'basic_smb_auth.cc' || echo '../../../../../helpers/basic_auth/SMB/'`basic_smb_auth.cc mv -f .deps/basic_smb_auth-basic_smb_auth.Tpo .deps/basic_smb_auth-basic_smb_auth.Po /bin/sh ../../../libtool --tag=CXX--mode=link ccache clang++ -DHELPERSCRIPT=\/usrhttp://build.squid-cache.org/job/trunk-matrix/compiler=clang,label=j-fbsd-93/ws/btlayer-00-default/squid-4.0.0-BZR/_inst/libexec/basic_smb_auth.sh\; -g -O2 -std=c++11 -I/usr/local/include -g -L/usr/local/lib -Wl,-R/usr/local/lib -pthread -o basic_smb_auth basic_smb_auth-basic_smb_auth.o ../../../lib/libmiscencoding.la ../../../compat/libcompat-squid.la -lm libtool: link: ccache clang++ -DHELPERSCRIPT=\/usrhttp://build.squid-cache.org/job/trunk-matrix/compiler=clang,label=j-fbsd-93/ws/btlayer-00-default/squid-4.0.0-BZR/_inst/libexec/basic_smb_auth.sh\; -g -O2 -std=c++11 -I/usr/local/include -g -Wl,-R/usr/local/lib -pthread -o basic_smb_auth basic_smb_auth-basic_smb_auth.o -L/usr/local/lib
[squid-dev] Build failed in Jenkins: trunk-matrix » gcc,j-fbsd-93 #141
See http://build.squid-cache.org/job/trunk-matrix/compiler=gcc,label=j-fbsd-93/141/ -- [...truncated 2910 lines...] cp ../../../libltdl/argz_.h argz.h-t mv argz.h-t argz.h make all-am /bin/sh ./libtool --tag=CC --mode=compile ccache gcc -DHAVE_CONFIG_H -I. -I../../../libltdl -DLT_CONFIG_H='config.h' -DLTDL -I. -I../../../libltdl -Ilibltdl -I../../../libltdl/libltdl -I../../../libltdl/libltdl -g -O2 -MT dlopen.lo -MD -MP -MF .deps/dlopen.Tpo -c -o dlopen.lo `test -f 'loaders/dlopen.c' || echo '../../../libltdl/'`loaders/dlopen.c /bin/sh ./libtool --tag=CC --mode=compile ccache gcc -DHAVE_CONFIG_H -I. -I../../../libltdl -DLTDLOPEN=libltdlc -DLT_CONFIG_H='config.h' -DLTDL -I. -I../../../libltdl -Ilibltdl -I../../../libltdl/libltdl -I../../../libltdl/libltdl -g -O2 -MT libltdlc_la-preopen.lo -MD -MP -MF .deps/libltdlc_la-preopen.Tpo -c -o libltdlc_la-preopen.lo `test -f 'loaders/preopen.c' || echo '../../../libltdl/'`loaders/preopen.c libtool: compile: ccache gcc -DHAVE_CONFIG_H -I. -I../../../libltdl -DLT_CONFIG_H=config.h -DLTDL -I. -I../../../libltdl -Ilibltdl -I../../../libltdl/libltdl -I../../../libltdl/libltdl -g -O2 -MT dlopen.lo -MD -MP -MF .deps/dlopen.Tpo -c ../../../libltdl/loaders/dlopen.c -o dlopen.o libtool: compile: ccache gcc -DHAVE_CONFIG_H -I. -I../../../libltdl -DLTDLOPEN=libltdlc -DLT_CONFIG_H=config.h -DLTDL -I. -I../../../libltdl -Ilibltdl -I../../../libltdl/libltdl -I../../../libltdl/libltdl -g -O2 -MT libltdlc_la-preopen.lo -MD -MP -MF .deps/libltdlc_la-preopen.Tpo -c ../../../libltdl/loaders/preopen.c -o libltdlc_la-preopen.o mv -f .deps/dlopen.Tpo .deps/dlopen.Plo /bin/sh ./libtool --tag=CC --mode=compile ccache gcc -DHAVE_CONFIG_H -I. -I../../../libltdl -DLTDLOPEN=libltdlc -DLT_CONFIG_H='config.h' -DLTDL -I. -I../../../libltdl -Ilibltdl -I../../../libltdl/libltdl -I../../../libltdl/libltdl -g -O2 -MT libltdlc_la-lt__alloc.lo -MD -MP -MF .deps/libltdlc_la-lt__alloc.Tpo -c -o libltdlc_la-lt__alloc.lo `test -f 'lt__alloc.c' || echo '../../../libltdl/'`lt__alloc.c libtool: compile: ccache gcc -DHAVE_CONFIG_H -I. -I../../../libltdl -DLTDLOPEN=libltdlc -DLT_CONFIG_H=config.h -DLTDL -I. -I../../../libltdl -Ilibltdl -I../../../libltdl/libltdl -I../../../libltdl/libltdl -g -O2 -MT libltdlc_la-lt__alloc.lo -MD -MP -MF .deps/libltdlc_la-lt__alloc.Tpo -c ../../../libltdl/lt__alloc.c -o libltdlc_la-lt__alloc.o mv -f .deps/libltdlc_la-preopen.Tpo .deps/libltdlc_la-preopen.Plo /bin/sh ./libtool --tag=CC --mode=compile ccache gcc -DHAVE_CONFIG_H -I. -I../../../libltdl -DLTDLOPEN=libltdlc -DLT_CONFIG_H='config.h' -DLTDL -I. -I../../../libltdl -Ilibltdl -I../../../libltdl/libltdl -I../../../libltdl/libltdl -g -O2 -MT libltdlc_la-lt_dlloader.lo -MD -MP -MF .deps/libltdlc_la-lt_dlloader.Tpo -c -o libltdlc_la-lt_dlloader.lo `test -f 'lt_dlloader.c' || echo '../../../libltdl/'`lt_dlloader.c libtool: compile: ccache gcc -DHAVE_CONFIG_H -I. -I../../../libltdl -DLTDLOPEN=libltdlc -DLT_CONFIG_H=config.h -DLTDL -I. -I../../../libltdl -Ilibltdl -I../../../libltdl/libltdl -I../../../libltdl/libltdl -g -O2 -MT libltdlc_la-lt_dlloader.lo -MD -MP -MF .deps/libltdlc_la-lt_dlloader.Tpo -c ../../../libltdl/lt_dlloader.c -o libltdlc_la-lt_dlloader.o mv -f .deps/libltdlc_la-lt__alloc.Tpo .deps/libltdlc_la-lt__alloc.Plo /bin/sh ./libtool --tag=CC --mode=compile ccache gcc -DHAVE_CONFIG_H -I. -I../../../libltdl -DLTDLOPEN=libltdlc -DLT_CONFIG_H='config.h' -DLTDL -I. -I../../../libltdl -Ilibltdl -I../../../libltdl/libltdl -I../../../libltdl/libltdl -g -O2 -MT libltdlc_la-lt_error.lo -MD -MP -MF .deps/libltdlc_la-lt_error.Tpo -c -o libltdlc_la-lt_error.lo `test -f 'lt_error.c' || echo '../../../libltdl/'`lt_error.c libtool: compile: ccache gcc -DHAVE_CONFIG_H -I. -I../../../libltdl -DLTDLOPEN=libltdlc -DLT_CONFIG_H=config.h -DLTDL -I. -I../../../libltdl -Ilibltdl -I../../../libltdl/libltdl -I../../../libltdl/libltdl -g -O2 -MT libltdlc_la-lt_error.lo -MD -MP -MF .deps/libltdlc_la-lt_error.Tpo -c ../../../libltdl/lt_error.c -o libltdlc_la-lt_error.o mv -f .deps/libltdlc_la-lt_dlloader.Tpo .deps/libltdlc_la-lt_dlloader.Plo /bin/sh ./libtool --tag=CC --mode=compile ccache gcc -DHAVE_CONFIG_H -I. -I../../../libltdl -DLTDLOPEN=libltdlc -DLT_CONFIG_H='config.h' -DLTDL -I. -I../../../libltdl -Ilibltdl -I../../../libltdl/libltdl -I../../../libltdl/libltdl -g -O2 -MT libltdlc_la-ltdl.lo -MD -MP -MF .deps/libltdlc_la-ltdl.Tpo -c -o libltdlc_la-ltdl.lo `test -f 'ltdl.c' || echo '../../../libltdl/'`ltdl.c libtool: compile: ccache gcc -DHAVE_CONFIG_H -I. -I../../../libltdl -DLTDLOPEN=libltdlc -DLT_CONFIG_H=config.h -DLTDL -I. -I../../../libltdl -Ilibltdl -I../../../libltdl/libltdl -I../../../libltdl/libltdl -g -O2 -MT libltdlc_la-ltdl.lo -MD -MP -MF .deps/libltdlc_la-ltdl.Tpo -c ../../../libltdl/ltdl.c -o libltdlc_la-ltdl.o mv -f .deps/libltdlc_la-lt_error.Tpo .deps/libltdlc_la-lt_error.Plo
