Implement SSLContext factory #1815

aswanidutt87 · 2022-12-21T17:32:51Z

No description provided.

euri10 · 2022-12-21T19:56:49Z

uvicorn/config.py

@@ -446,6 +448,9 @@ def load(self) -> None:
                ca_certs=self.ssl_ca_certs,
                ciphers=self.ssl_ciphers,
            )
+
+        elif self.ssl_context:
+            self.ssl = self.ssl_context.custom_ssl_context_factory()  # type: ignore


Suggested change

self.ssl = self.ssl_context.custom_ssl_context_factory() # type: ignore

self.ssl = self.ssl_context()

this should do it no ? I dont get it otherwise

it wont work with out calling the callable, here is my usage

class SSL_Context(object): @classmethod def __call__(self): context = ssl.SSLContext(int(ssl.PROTOCOL_TLS)) context.load_cert_chain(certfile=tls_cert, keyfile=tls_key) if allowed_ciphers: context.set_ciphers(allowed_ciphers) if list_options: for each_option in list_options: context.options |= each_option return context ssl_context = SSL_Context uvicorn.run( "web:app", host="0.0.0.0", port=int(port), reload=True, ssl_context=ssl_context, )

and inside config.py

elif self.ssl_context: self.ssl = self.ssl_context.__call__() # type: ignore

Kludex · 2022-12-25T17:53:33Z

The way I see this feature is something like:

from ssl import SSLContext

import uvicorn

def ssl_context_factory(context: SSLContext) -> SSLContext:
    return context

if __name__ == "__main__":
    uvicorn.run("main:app", ssl_context_factory=ssl_context_factory)

All the SSL parameters we have can be used together with the ssl_context_factory parameter.

If using this feature via python code, then the above is what I imagine. If we also want to use it via CLI, I guess we'd need to use the import_string that we have - but the CLI can wait for the python code part to be ready.

aswanidutt87 · 2022-12-27T15:40:24Z

The way I see this feature is something like:
from ssl import SSLContext

import uvicorn

def ssl_context_factory(context: SSLContext) -> SSLContext:
    return context

if __name__ == "__main__":
    uvicorn.run("main:app", ssl_context_factory=ssl_context_factory)
All the SSL parameters we have can be used together with the ssl_context_factory parameter.

If using this feature via python code, then the above is what I imagine. If we also want to use it via CLI, I guess we'd need to use the import_string that we have - but the CLI can wait for the python code part to be ready.

got it updated with your suggestions as follows.

def ssl_context() -> ssl.SSLContext:
    context = ssl.SSLContext(int(ssl.PROTOCOL_TLS))
    context.load_cert_chain(certfile=tls_cert, keyfile=tls_key)
    if allowed_ciphers:
        context.set_ciphers(allowed_ciphers)
    if list_options:
        for each_option in list_options:
            context.options |= each_option
    return context

    uvicorn.run(
        "web:app",
        host="0.0.0.0",
        port=int(port),
        reload=True,
        ssl_context_factory=ssl_context,
    )

in config.py
        elif self.ssl_context_factory:
            self.ssl = self.ssl_context_factory()

aswanidutt87 · 2023-01-04T17:37:57Z

@Kludex / @euri10 please review, I have modified per your suggestions, i think this should work

Kludex · 2023-01-04T21:21:08Z

uvicorn/main.py

@@ -319,6 +319,13 @@ def print_version(ctx: click.Context, param: click.Parameter, value: bool) -> No
    help="Ciphers to use (see stdlib ssl module's)",
    show_default=True,
 )
+@click.option(
+    "--ssl-context-factory",
+    type=typing.Callable,


This can't be a callable. What I said was to load the function object from the path... Which I'm not sure if we really should make this available via CLI.

There are messages I've written here that you ignored. I'm not reviewing this again without them addressed.

The code changes here don't fully take in consideration my previous comments.

Kludex · 2023-01-04T21:22:07Z

uvicorn/config.py

@@ -446,6 +448,9 @@ def load(self) -> None:
                ca_certs=self.ssl_ca_certs,
                ciphers=self.ssl_ciphers,
            )
+
+        elif self.ssl_context_factory:
+            self.ssl = self.ssl_context_factory()


Hmm... Maybe we shouldn't ignore the previous parameters passed? 🤔

What I propose was for the factory function to receive a SSLContext, which would be the one that uvicorn creates on the lines above.

Kludex · 2023-02-16T10:26:12Z

@aswanidutt87 Are you still interested on this?

aswanidutt87 · 2023-02-16T14:32:19Z

@aswanidutt87 Are you still interested on this?

yes @Kludex , we really need this,
regarding your question "Hmm... Maybe we shouldn't ignore the previous parameters passed? " its either we pass the parameters to build the ssl_context in the uvicorn code or pass the custom created ssl_context directly to uvicorn, it cant be both at the same time.
and for the second question "What I propose was for the factory function to receive a SSLContext, which would be the one that uvicorn creates on the lines above. ", thats the whole problem that we cant pass the SSLContext or any other object for that matter, so passing a ssl_context_factory and return SSLContext wherever we need it.
please advise

aswanidutt87 · 2023-02-16T14:36:51Z

@Kludex - also, for our case or in general adding ssl_options will suffice all the use cases as the uvicorn code already have all the options except the ssl_options, so kindly consider this pr also-#1692

Kludex · 2023-02-17T08:41:01Z

ontext wherever we need it.

The factory is meant to be called only when the SSLContext is needed, not in the Config, that's why it's the same problem as before... That's not the idea. I've pasted a link on how it's intended to be done on Gunicorn, and here would be analogous.

aswanidutt87 · 2023-02-17T23:24:16Z

@Kludex

I have modified the code to incorporate your suggestion of consuming the passed parameters to the custom ssl_context supplied.
please review

Kludex · 2023-03-09T21:43:15Z

Would you mind fixing the linter, and removing the parameter from the CLI parameters?

aswanidutt87 · 2023-03-10T15:01:48Z

Would you mind fixing the linter, and removing the parameter from the CLI parameters?

@Kludex , Linter is happy when I ran in my local

Reg: the removal of parameter from the CLI, then the test_cli is breaking.

Kludex · 2023-03-12T12:28:03Z

@aswanidutt87 You probably need to update your dependencies. Please also run ./scripts/lint.

aswanidutt87 · 2023-03-13T14:48:08Z

@aswanidutt87 You probably need to update your dependencies. Please also run ./scripts/lint.
@Kludex lint is clean I guess

and regarding the test_cli fail, since we removed the @click.option(
"--ssl-context", from main.py but still it has ssl_context: typing.Callable, as one of the parameter, the test_cli is failing with below error . not sure what dependencies I need to update so that the test_cli is happy.

aswanidutt87 · 2023-05-18T14:40:42Z

@aswanidutt87 You probably need to update your dependencies. Please also run ./scripts/lint.
@Kludex lint is clean I guess

and regarding the test_cli fail, since we removed the @click.option( "--ssl-context", from main.py but still it has ssl_context: typing.Callable, as one of the parameter, the test_cli is failing with below error . not sure what dependencies I need to update so that the test_cli is happy.

@Kludex , please let me know if we really need to remove the entry for CLI parameter, and if so how to fix the test failure due to the removal of CLI parameter.

aswanidutt87 · 2023-05-25T14:18:55Z

@Kludex / @euri10 please review the comment above and suggest

aswanidutt87 · 2023-07-04T14:19:58Z

@Kludex / @euri10 please review the comment above and suggest

hello @Kludex / @euri10 , could you please review this change and approve/comment. we need this custom ssl_context for our use case.

Kludex · 2023-07-04T14:27:59Z

I've commented already: #1815 (comment)

aswanidutt87 · 2023-07-04T14:33:18Z

I've commented already: #1815 (comment)

@Kludex the latest comment is #1815 (comment) , if I remove the CLI parameter for --ssl-context, the test_cli is failing, I need help here.

Kludex · 2024-02-19T19:15:32Z

uvicorn/main.py

@@ -319,6 +319,13 @@ def print_version(ctx: click.Context, param: click.Parameter, value: bool) -> No
    help="Ciphers to use (see stdlib ssl module's)",
    show_default=True,
 )
+@click.option(
+    "--ssl-context-factory",
+    type=typing.Callable,


There are messages I've written here that you ignored. I'm not reviewing this again without them addressed.

The code changes here don't fully take in consideration my previous comments.

Kludex · 2024-02-19T19:16:09Z

Also, the pipeline is not passing... ???

Kludex · 2024-04-13T08:53:27Z

Closing as stale.

aswanidutt and others added 9 commits October 3, 2022 12:37

adding list of options to ssl_context

6635a74

Merge branch 'encode:master' into master

e3de724

adding unit tests, coverage - 97.71%

33aef83

linting, doc updates, cleanup

9d9069e

removed from load and added in create_ssl_context

33d464e

adding help content

904ecb1

Merge branch 'encode:master' into master

5fd40f8

adding custom ssl_context via factory pattern

264852d

ignoring setup

5da1a3d

aswanidutt87 mentioned this pull request Dec 21, 2022

Support custom SSL Context #806

Open

2 tasks

euri10 reviewed Dec 21, 2022

View reviewed changes

Kludex added tls/ssl waiting author Waiting for author's reply labels Dec 25, 2022

code changes with review comments updated

208e581

aswanidutt87 requested a review from euri10 December 27, 2022 15:40

Kludex reviewed Jan 4, 2023

View reviewed changes

adding ssl_context to update with parameters passed

bf1e78e

Kludex changed the title ~~Issue #806~~ Implement SSLContext factory Mar 9, 2023

aswanidutt and others added 4 commits August 16, 2023 19:50

Merge remote-tracking branch 'upstream/master'

8370271

updating cyrptography due to a cve

264fc9f

Merge remote-tracking branch 'upstream/master'

cf71c25

Merge branch 'master' into issue-encode#806

4abc589

Kludex requested changes Feb 19, 2024

View reviewed changes

aswanidutt added 5 commits February 19, 2024 13:40

Merge remote-tracking branch 'upstream/master'

9948eda

fixing imports

40e58a0

Merge remote-tracking branch 'upstream/master'

e1258d7

merge from master

1a2e7a3

changing to str

bc8adbb

This comment was marked as spam.

Sign in to view

MadsRC added a commit to MadsRC/uvicorn that referenced this pull request Apr 5, 2024

tidied up encode#1815

71f4f5d

MadsRC mentioned this pull request Apr 5, 2024

tidied up #1815 #2297

Open

3 tasks

MadsRC added a commit to MadsRC/uvicorn that referenced this pull request Apr 5, 2024

tidied up encode#1815

6f6107d

Kludex closed this Apr 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement SSLContext factory #1815

Implement SSLContext factory #1815

aswanidutt87 commented Dec 21, 2022

euri10 Dec 21, 2022

aswanidutt87 Dec 22, 2022

Kludex commented Dec 25, 2022 •

edited

Loading

aswanidutt87 commented Dec 27, 2022

aswanidutt87 commented Jan 4, 2023

Kludex Jan 4, 2023

Kludex Feb 19, 2024

Kludex Jan 4, 2023

Kludex commented Feb 16, 2023

aswanidutt87 commented Feb 16, 2023

aswanidutt87 commented Feb 16, 2023 •

edited

Loading

Kludex commented Feb 17, 2023

aswanidutt87 commented Feb 17, 2023 •

edited

Loading

Kludex commented Mar 9, 2023

aswanidutt87 commented Mar 10, 2023

Kludex commented Mar 12, 2023 •

edited

Loading

aswanidutt87 commented Mar 13, 2023 •

edited

Loading

aswanidutt87 commented May 18, 2023

aswanidutt87 commented May 25, 2023 •

edited

Loading

aswanidutt87 commented Jul 4, 2023

Kludex commented Jul 4, 2023

aswanidutt87 commented Jul 4, 2023

Kludex Feb 19, 2024

Kludex commented Feb 19, 2024

This comment was marked as spam.

Kludex commented Apr 13, 2024

	self.ssl = self.ssl_context.custom_ssl_context_factory() # type: ignore
	self.ssl = self.ssl_context()

Implement SSLContext factory #1815

Implement SSLContext factory #1815

Conversation

aswanidutt87 commented Dec 21, 2022

euri10 Dec 21, 2022

Choose a reason for hiding this comment

aswanidutt87 Dec 22, 2022

Choose a reason for hiding this comment

Kludex commented Dec 25, 2022 • edited Loading

aswanidutt87 commented Dec 27, 2022

aswanidutt87 commented Jan 4, 2023

Kludex Jan 4, 2023

Choose a reason for hiding this comment

Kludex Feb 19, 2024

Choose a reason for hiding this comment

Kludex Jan 4, 2023

Choose a reason for hiding this comment

Kludex commented Feb 16, 2023

aswanidutt87 commented Feb 16, 2023

aswanidutt87 commented Feb 16, 2023 • edited Loading

Kludex commented Feb 17, 2023

aswanidutt87 commented Feb 17, 2023 • edited Loading

Kludex commented Mar 9, 2023

aswanidutt87 commented Mar 10, 2023

Kludex commented Mar 12, 2023 • edited Loading

aswanidutt87 commented Mar 13, 2023 • edited Loading

aswanidutt87 commented May 18, 2023

aswanidutt87 commented May 25, 2023 • edited Loading

aswanidutt87 commented Jul 4, 2023

Kludex commented Jul 4, 2023

aswanidutt87 commented Jul 4, 2023

Kludex Feb 19, 2024

Choose a reason for hiding this comment

Kludex commented Feb 19, 2024

This comment was marked as spam.

Kludex commented Apr 13, 2024

Kludex commented Dec 25, 2022 •

edited

Loading

aswanidutt87 commented Feb 16, 2023 •

edited

Loading

aswanidutt87 commented Feb 17, 2023 •

edited

Loading

Kludex commented Mar 12, 2023 •

edited

Loading

aswanidutt87 commented Mar 13, 2023 •

edited

Loading

aswanidutt87 commented May 25, 2023 •

edited

Loading