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
Fix client hello #1021
Fix client hello #1021
Conversation
Consider message sequence number also to determine, if a CLIENT_HELLO starts a new handshake. Signed-off-by: Achim Kraus <achim.kraus@bosch-si.com>
timer.schedule(new Runnable() { | ||
@Override | ||
public void run() { | ||
handshaker.getConnection().isStartedByClientHello(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really like the idea about a "isSomethingMethod" which is also a setter.
There is a startByClientHello
method we could use it ?
* Random used by client to start the handshake. | ||
* | ||
* Note: used outside of serial-execution! | ||
* Random used by client to start the handshake. Maybe {@code null}, for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Random CLIENT_HELLO ?
if (startRandom != null) { | ||
return startRandom.equals(clientHello.getRandom()); | ||
if (clientHello == null) { | ||
startingClientHello = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need to set startingClientHello to null ? (see comment above)
startingClientHello = null; | ||
} else if (startingClientHello != null) { | ||
if (startingClientHello.getRandom().equals(clientHello.getRandom())) { | ||
if (startingClientHello.getMessageSeq() >= clientHello.getMessageSeq()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not understand this check ? Why using Random is not enough ? I guess this is maybe something about the initialization of Message Sequence number of handshaker but this is not clear to me :-/
Connection sessionConnection = connections.getConnectionBySessionId(); | ||
if (sessionConnection != null && sessionConnection != connection) { | ||
// don't overwrite | ||
verify = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test should not be done in isClientInControlOfSourceIpAddress
?
If not, I would have remove the boolean and just do :
sendHelloVerify(clientHello, record, null);
return;
Signed-off-by: Achim Kraus <achim.kraus@bosch-si.com>
Signed-off-by: Achim Kraus achim.kraus@bosch-si.com