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

Fix interoperability with NTPSEC and make compliant to latest draft #37

Merged
merged 2 commits into from Nov 22, 2019

Conversation

@mdavids
Copy link
Contributor

mdavids commented Nov 22, 2019

As discussed with Watson by e-mail.

mdavids added 2 commits Nov 22, 2019
…TPSEC
@@ -269,7 +270,7 @@ pub fn run_nts_ke_client(
}
}
debug!(logger, "saw the end of the response");
stream.shutdown(Shutdown::Both)?;
stream.shutdown(Shutdown::Write)?;

This comment has been minimized.

Copy link
@haxxpop

haxxpop Nov 22, 2019

Collaborator

Why not both?

This comment has been minimized.

Copy link
@mdavids

mdavids Nov 22, 2019

Author Contributor

Because that doesn't seem to work on NTPSEC.

Try: cfnts client ntp1.glypnod.com -p 123

This comment has been minimized.

Copy link
@wbl

wbl Nov 22, 2019

Contributor

What's happening is the close notify comes in after we shut down the reading half, hence triggering a failure.

This comment has been minimized.

Copy link
@haxxpop

haxxpop Nov 22, 2019

Collaborator

I see

@wbl
wbl approved these changes Nov 22, 2019
@haxxpop haxxpop self-requested a review Nov 22, 2019
@haxxpop haxxpop merged commit ad21358 into cloudflare:master Nov 22, 2019
@haxxpop

This comment has been minimized.

Copy link
Collaborator

haxxpop commented Nov 22, 2019

Thank you for your contribution :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.