On 25/01/2017 08:24 μμ, Alex Rousskov wrote:
On 01/16/2017 04:38 AM, Christos Tsantilas wrote:
On 13/01/2017 07:04 μμ, Alex Rousskov wrote:
The dependency here is that clientHelloMessage comes from our parser. We
can substitute OpenSSL-generated ClientHello with client-sent
ClientHello because/if we successfully parsed and stored the latter. I
think we should validate clientHelloMessage/parsing state before using
clientHelloMessage.
+ if (bumpMode_ == Ssl::bumpPeek) {
+ // client-sent ClientHello must be available for peek
+ Must(!clientSentHello.isEmpty());
+ if (adjustSSL(ssl, clientTlsDetails, clientSentHello))
allowBump = true;
+ allowSplice = true;
+ // Replace OpenSSL-generated ClientHello with client-sent one.
+ helloMsg.append(clientSentHello);
+ debugs(83, 7, "FD " << fd_ << ": Using client-sent ClientHello for
peek mode");
What happens if that new Must() throws?
May Squid reach that Must() and will that Must() throw if Squid receives
a valid TLS Hello encapsulated into ancient SSLv2 records?
The must will throw when Squid will be somewhere inside
Security::PeerConnector::NegotiateSsl asyncJob call or similar code and
the AsyncJob code will handle this exception. Actually will abort the
connection.
This is should be enough.
Our [fixed?] handling of situations like this is still not clear to me.
My concern does not imply that your code is wrong. It is just not clear
to me whether "client-sent ClientHello must be available for peek"
really means
* A client-sent ClientHello is required for peeking, but we might get
here without it in some unusual cases. Throw so that FooBar will see the
problem and handle these unusual cases.
We do not have the ClientHello here only in the case of a squid-bug.
or
* A client-sent ClientHello is required for peeking. The calling code
must ensure that we never get here without it. Throw if our calling code
is buggy.
This is the correct.
Please note that by "calling code" I do not mean some direct
ServerBio::write() caller. I mean any Squid code that sets up the BIO
object and allows it to proceed to write() under said circumstances.
Actually without a client hello message we should not proceed to step3
peek or stare. We should abort in an earlier step.
The ServerBio::write is called during step3 (from step2 to step3).
and for stare mode allow adjustSSL only if clientSentHello is not null.
That code/case is clear to me.
Thank you,
Alex.
_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
