Skip to content
This repository has been archived by the owner on Jan 6, 2024. It is now read-only.

Port to rustls v0.20 #9

Merged
merged 12 commits into from
Oct 30, 2022
Merged

Port to rustls v0.20 #9

merged 12 commits into from
Oct 30, 2022

Conversation

notgull
Copy link
Member

@notgull notgull commented Aug 22, 2022

Ports the codebase to the new rustls 0.20 version.

cargo test appears to be failing because of expired certificates.

Resolves #8

@taiki-e taiki-e closed this Aug 24, 2022
@taiki-e taiki-e reopened this Aug 24, 2022
@notgull
Copy link
Member Author

notgull commented Aug 24, 2022

Hmm, nope, that's not it.

There are probably a few things I overlooked, let me give this a once-over.

@taiki-e
Copy link
Collaborator

taiki-e commented Aug 24, 2022

cargo test appears to be failing because of expired certificates.

For this failure, I think you probably need to do something like tokio-rs/tls@c2d1fe6.

@flickpp
Copy link

flickpp commented Sep 17, 2022

Is this PR still alive?

@notgull
Copy link
Member Author

notgull commented Sep 17, 2022

Yes, I was just side-tracked. If it's important to you I can probably get this done pretty quickly.

@notgull
Copy link
Member Author

notgull commented Sep 17, 2022

I'm not really familiar with SSL, so I'm not entirely sure why the tests are failing here. I don't see where an accidental EOF condition could arise.

@flickpp
Copy link

flickpp commented Sep 18, 2022

Not urgent no, thanks for your efforts!

I also believe to have found a bug.
I created the simplest possible server with this branch and async-std. I then called it with curl.
When I use server::TlsStream.read I don't get any data.

Without looking into it deeply, my initial guess is that the handshake logic is reading more bytes off the socket than just the handshake. I can create a repo with my noddy server if this helps?

This may be related to the EOF error you're seeing above?
Although more likely a socket is getting dropped somewhere before a clean shutdown has happened.

Let me know if you'd like some help.

@notgull
Copy link
Member Author

notgull commented Sep 19, 2022

Alright, I don't really know what the actual problem is. In this case, I just re-forked from tokio-rustls, and the tests are passing now.

@flickpp
Copy link

flickpp commented Sep 19, 2022

Just to add for the most recent revision, TlsStream<async_std::net::TcpStream> async read is working for me now.

@notgull notgull requested a review from taiki-e October 2, 2022 14:22
@notgull
Copy link
Member Author

notgull commented Oct 29, 2022

@smol-rs/admins Any blockers to this being merged?

Copy link
Member

@fogti fogti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall looks ok (I'm trusting the tests for now), but I have a few nitpicks.

.gitignore Show resolved Hide resolved
Cargo.toml Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
examples/client/Cargo.toml Outdated Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
src/server.rs Show resolved Hide resolved
tests/badssl.rs Show resolved Hide resolved
tests/test.rs Outdated Show resolved Hide resolved
src/common/mod.rs Show resolved Hide resolved
Copy link
Member Author

@notgull notgull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I took care of most of your nitpicks :-)

src/client.rs Show resolved Hide resolved
Poll::Ready(result) => result,
Poll::Pending => Err(io::ErrorKind::WouldBlock.into()),
}
self.poll_with(|io, cx| io.poll_flush(cx))
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a good fit for the io part of futures_lite

src/common/mod.rs Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
@fogti
Copy link
Member

fogti commented Oct 30, 2022

Note: this PR should be squash-merged.

@notgull notgull merged commit e70f938 into smol-rs:master Oct 30, 2022
@notgull notgull deleted the rustls-0-20 branch October 30, 2022 14:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Would a rustls 0.20 PR be welcome?
4 participants