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

Add serverCertificateHashes test server #50263

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jgraham
Copy link
Contributor

@jgraham jgraham commented Jan 24, 2025

Add a second webtransport server, in order to test connection with a server, that has a self-signed certificate together with serverCertificateHashes.

See web-platform-tests/rfcs#216

Add a second webtransport server, in order to test connection with a
server, that has a self-signed certificate together with
serverCertificateHashes.

See web-platform-tests/rfcs#216
Copy link
Member

@nidhijaju nidhijaju left a comment

Choose a reason for hiding this comment

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

Took a first pass at this, but looks good overall.

"""
Specify, if the server should generate a certificate or use an existing certificate
USEPREGENERATED: use existing certificate
GENERATEDVALIDSERVERCERTIFICATEHASHCERT: generate a certificate compatible to server cert hashes
Copy link
Member

Choose a reason for hiding this comment

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

minor nit:

Suggested change
GENERATEDVALIDSERVERCERTIFICATEHASHCERT: generate a certificate compatible to server cert hashes
GENERATEDVALIDSERVERCERTIFICATEHASHCERT: generate a certificate compatible with server cert hashes

"""
USEPREGENERATED = 1,
GENERATEDVALIDSERVERCERTIFICATEHASHCERT = 2
# TODO add cases for invalid certificates
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# TODO add cases for invalid certificates
# TODO(jgraham): add cases for invalid certificates

@@ -507,18 +521,31 @@ class WebTransportH3Server:
:param host: Host from which to serve.
:param port: Port from which to serve.
:param doc_root: Document root for serving handlers.
:paran cert_mode: The used certificate mode can be
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
:paran cert_mode: The used certificate mode can be
:param cert_mode: The used certificate mode can be

@@ -499,6 +503,16 @@ def add(self, ticket: SessionTicket) -> None:
def pop(self, label: bytes) -> Optional[SessionTicket]:
return self.tickets.pop(label, None)

class WebTransportCertificateGeneration(Enum):
Copy link
Member

Choose a reason for hiding this comment

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

If we're assigning them integer values, we can use IntEnum and skip the additional import, right?

Suggested change
class WebTransportCertificateGeneration(Enum):
class WebTransportCertificateGeneration(IntEnum):

Comment on lines +509 to +510
USEPREGENERATED: use existing certificate
GENERATEDVALIDSERVERCERTIFICATEHASHCERT: generate a certificate compatible to server cert hashes
Copy link
Member

Choose a reason for hiding this comment

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

This makes it a little bit more readable:

Suggested change
USEPREGENERATED: use existing certificate
GENERATEDVALIDSERVERCERTIFICATEHASHCERT: generate a certificate compatible to server cert hashes
USE_PREGENERATED: use existing certificate
GENERATE_VALID_SERVER_CERTIFICATE_HASH_CERT: generate a certificate compatible to server cert hashes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra serve webtransport wg-s_webtransport wptrunner The automated test runner, commonly called through ./wpt run wptserve
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants