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

Support custom SSL Context #806

Open
1 of 2 tasks
Tracked by #1808
samypr100 opened this issue Oct 7, 2020 · 19 comments
Open
1 of 2 tasks
Tracked by #1808

Support custom SSL Context #806

samypr100 opened this issue Oct 7, 2020 · 19 comments

Comments

@samypr100
Copy link

samypr100 commented Oct 7, 2020

Checklist

  • There are no similar issues or pull requests for this yet.
  • I discussed this idea on the community chat and feedback is positive.

Is your feature related to a problem? Please describe.

I would like to pass an existing SSL Context to uvicorn.run(). For example, I have a certificate that needs a password to load. Typically I would do that by setting up a context like so:

ssl_context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
# ... customize context even more
ssl_context.load_cert_chain(ssl_crt_path, keyfile=ssl_key_path, password=ssl_key_password)

The current options are limited to these kinds of advance scenarios and I'd like to avoid keep adding/requesting --ssl-xyz options for each of those scenarios. I know I can decrypt the key before loading it into python, but I'm limited on the environment I need to deploy on since I'm given the encrypted key and the password via a secret.

Describe the solution you would like.

Adding the ability to pass a ssl_context to uvicorn.run in python code that supersedes any of the ssl_* settings if provided.

Example changes in uvicorn/config.py:

@property
def is_ssl(self) -> bool:
    return bool(self.ssl_keyfile or self.ssl_certfile)

@property
def is_ssl_context(self) -> bool:
    return isinstance(self.ssl_context, ssl.SSLContext)

# ...

if self.is_ssl and not self.is_ssl_context:
    self.ssl = create_ssl_context(
        keyfile=self.ssl_keyfile,
        certfile=self.ssl_certfile,
        ssl_version=self.ssl_version,
        cert_reqs=self.ssl_cert_reqs,
        ca_certs=self.ssl_ca_certs,
        ciphers=self.ssl_ciphers,
    )
elif self.is_ssl_context:
    self.ssl = self.ssl_context
else:
    self.ssl = None

# ...

Describe alternatives you considered

Searched source code to see if there was a way to pass a custom context to no avail.

Additional context

Since ssl context is createe via python, it would not quite be supported via command line. Unless we want to get fancy. I can attempt to do a PR if permitted. Thanks!

Important

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@euri10
Copy link
Member

euri10 commented Oct 7, 2020

related #776

@euri10
Copy link
Member

euri10 commented Oct 8, 2020

I have some questions @samypr100 :)

  1. how does gunicorn deal with that, I'm not following the project closely enough but I don't think they provide that option
  2. if implemented, how people using uvicorn as a gunicorn worker will do ?
  3. I proposed 2 small PR, one implements the ability pas pass a password to decrypt your ssl key, does that solve your use case ?
  4. I'm afraid I'm not clear on this, could you expand ?

but I'm limited on the environment I need to deploy on since I'm given the encrypted key and the password via a secret.

@samypr100
Copy link
Author

Hi @euri10, thanks for taking time and considering this.

  1. I don't know how gunicorn deals with it since I don't think they support --ssl-password either. I'm not sure since I could not find any ssl.SSLContext usage in their codebase. There seems to be related issues Switch to use ssl.SSLContext in sync worker benoitc/gunicorn#2012 though to add support, so support might come. I've limited my deployments to use uwsgi via nginx in those scenarios instead of gunicorn for my non-asgi deployments.
  2. I briefly mentioned that I'm not sure how this would work via a CLI unless we have an string equivalent interface the same way we load the app. This issue was meant more for the programtic instantiation. Maybe once gunicorn supports more options related to ssl.SSLContext it might fit better.
  3. The ability to pass a password to decrypt your ssl key would solve my immediate issue. This issue was more of a generic catch all if someone has a more advance SSL context object they wanted to pass on. Flask for example has the ability to pass an ssl_context on it's built-in development server.
  4. All I meant by this is that I currently have cases were I receive the encrypted key string and the password via an environment variable. That's all, hopefully it didn't confuse more.

@euri10
Copy link
Member

euri10 commented Oct 12, 2020

Flask for example has the ability to pass an ssl_context on it's built-in development server.

can you point me where, this is interesting.

ok @samypr100 now that #807 and #808 got merged you can use the fixtures and expand on them for you PR should you be fancy providing one,

something like the below should work ?

@pytest.fixture
def ssl_ctx(tls_certificate):
    ssl_ctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
    tls_certificate.configure_cert(ssl_ctx)
    return ssl_ctx

once there is a PR layout then maybe we can discuss more proactively about what to do with the CLI

@samypr100
Copy link
Author

can you point me where, this is interesting.

Absolutely!
Code: https://github.com/pallets/werkzeug/blob/master/src/werkzeug/serving.py#L604
Docs: https://werkzeug.palletsprojects.com/serving#werkzeug.serving.run_simple (see ssl_context param)

Thanks!

@aswanidutt87
Copy link

I have submitted a PR related to this issue- added param of ssl.Options list to the config of ssl_context, we are trying to resolve a vulnerability of DoS by setting the ssl.OP_NO_RENEGOTIATION to the ssl_options of ssl_context.
#1692 has the changes

@Kludex
Copy link
Sponsor Member

Kludex commented Oct 29, 2022

Adding the ability to pass a ssl_context to uvicorn.run in python code that supersedes any of the ssl_* settings if provided.

PR welcome for what @samypr100 proposes.

@Kludex Kludex added this to the Version 0.21.0 milestone Oct 29, 2022
@aswanidutt87
Copy link

aswanidutt87 commented Nov 8, 2022

hello @Kludex I have started working on passing ssl_context directly to uvicorn.run via config.py as below.
usage:

    ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
    ctx.load_cert_chain(tls_cert, tls_key, None)
    ctx.verify_mode = ssl.VerifyMode(ssl.CERT_NONE)
    ctx.set_ciphers(allowed_ciphers)
    if list_options:
        for each_option in list_options:
            ctx.options |= each_option
    uvicorn.run(
        "web:app",
        host="0.0.0.0",
        port=int(port),
        reload=True,
        ssl_context=ctx,
    )

in main.py
ssl_context: typing.Optional[ssl.SSLContext],

and in the config.py

  ssl_context: Optional[ssl.SSLContext] = None,


    @property
    def is_ssl_context(self) -> bool:
        return isinstance(self.ssl_context, ssl.SSLContext)




        if self.is_ssl and not self.is_ssl_context:
            assert self.ssl_certfile
            self.ssl: Optional[ssl.SSLContext] = create_ssl_context(
                keyfile=self.ssl_keyfile,
                certfile=self.ssl_certfile,
                password=self.ssl_keyfile_password,
                ssl_version=self.ssl_version,
                cert_reqs=self.ssl_cert_reqs,
                ca_certs=self.ssl_ca_certs,
                ciphers=self.ssl_ciphers,
            )
        elif self.is_ssl_context:
            self.ssl = self.ssl_context

and it didnt like the passing of ssl.SSLContext object into Config, on the server startup its failing to pickle the SSLContext object.

here is stack trace of the server startup error:

 File "/Users/aswani/Documents/projects/project_uvicorn/uvicorn_submodule/uvicorn/main.py", line 575, in run
    ChangeReload(config, target=server.run, sockets=[sock]).run()
  File "/Users/aswani/Documents/projects/project_uvicorn/uvicorn_submodule/uvicorn/supervisors/basereload.py", line 44, in run
    self.startup()
  File "/Users/aswani/Documents/projects/project_uvicorn/uvicorn_submodule/uvicorn/supervisors/basereload.py", line 80, in startup
    self.process.start()
  File "/Users/aswani/.pyenv/versions/3.10.7/lib/python3.10/multiprocessing/process.py", line 121, in start
    self._popen = self._Popen(self)
  File "/Users/aswani/.pyenv/versions/3.10.7/lib/python3.10/multiprocessing/context.py", line 288, in _Popen
    return Popen(process_obj)
  File "/Users/aswani/.pyenv/versions/3.10.7/lib/python3.10/multiprocessing/popen_spawn_posix.py", line 32, in __init__
    super().__init__(process_obj)
  File "/Users/aswani/.pyenv/versions/3.10.7/lib/python3.10/multiprocessing/popen_fork.py", line 19, in __init__
    self._launch(process_obj)
  File "/Users/aswani/.pyenv/versions/3.10.7/lib/python3.10/multiprocessing/popen_spawn_posix.py", line 47, in _launch
    reduction.dump(process_obj, fp)
  File "/Users/aswani/.pyenv/versions/3.10.7/lib/python3.10/multiprocessing/reduction.py", line 65, in dump
    ForkingPickler(file, protocol).dump(obj)
TypeError: cannot pickle 'SSLContext' object

I need to know how we can pass the ssl_context into uvicorn.run if this is not the correct way.. please advise.

@Kludex
Copy link
Sponsor Member

Kludex commented Nov 8, 2022

Can we instead pass a function that returns a SSLContext in a factory away?

def ssl_context_factory() -> SSLContext:
    ...

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

@aswanidutt87
Copy link

aswanidutt87 commented Nov 8, 2022

@Kludex something like this worked not sure if you meant to do this way..
usage:

   uvicorn.run(
        "web:app",
        host="0.0.0.0",
        port=int(port),
        reload=True,
        ssl_context=SSLContext(
            tls_cert,
            tls_key,
            None,
            ssl.PROTOCOL_TLS_SERVER,
            ssl.CERT_NONE,
            None,
            allowed_ciphers,
            list_options,
        ),
        workers=10,
    )

in config.py

class SSLContext:
    def __init__(
        self,
        certfile: Union[str, os.PathLike],
        keyfile: Optional[Union[str, os.PathLike]],
        password: Optional[str],
        ssl_version: int,
        cert_reqs: int,
        ca_certs: Optional[Union[str, os.PathLike]],
        ciphers: Optional[str],
        options: Optional[List[ssl.Options]],
    ):
        self.certfile = certfile
        self.keyfile = keyfile
        self.password = password
        self.ssl_version = ssl_version
        self.cert_reqs = cert_reqs
        self.ca_certs = ca_certs
        self.ciphers = ciphers
        self.options = options

     def ssl_context_factory(self) -> ssl.SSLContext:
        ctx = ssl.SSLContext(self.ssl_version)
        get_password = (lambda: self.password) if self.password else None
        ctx.load_cert_chain(self.certfile, self.keyfile, get_password)
        ctx.verify_mode = ssl.VerifyMode(self.cert_reqs)
        if self.ca_certs:
            ctx.load_verify_locations(self.ca_certs)
        if self.ciphers:
            ctx.set_ciphers(self.ciphers)
        if self.options:
            for each_option in self.options:
                ctx.options |= each_option
        return ctx



     if self.is_ssl and not self.is_ssl_context:
            assert self.ssl_certfile
            self.ssl: Optional[ssl.SSLContext] = create_ssl_context(
                keyfile=self.ssl_keyfile,
                certfile=self.ssl_certfile,
                password=self.ssl_keyfile_password,
                ssl_version=self.ssl_version,
                cert_reqs=self.ssl_cert_reqs,
                ca_certs=self.ssl_ca_certs,
                ciphers=self.ssl_ciphers,
            )
        elif self.is_ssl_context and self.ssl_context is not None:
            self.ssl = self.ssl_context.ssl_context_factory()
        else:
            self.ssl = None

but its not much of different than passing all parameters that are needed for creating a ssl_context inside config.py, I dont think we can create a custom ssl_context object and pass it into config.py

@Kludex
Copy link
Sponsor Member

Kludex commented Nov 22, 2022

I think the idea is more to call the factory when needed on each worker. Similar to: benoitc/gunicorn#2649

@aswanidutt87
Copy link

thanks for the hint, here is the PR - #1815, please review

@Kludex Kludex mentioned this issue Mar 10, 2023
16 tasks
@Kludex Kludex modified the milestones: Version 0.21.0, Version 0.22.0 Mar 22, 2023
@desean1625
Copy link

In Gunicorn version 21.0+ the code that mapped the CLI flag --ssl-version TLSv1_2 into an int was removed.

This causes the uvicorn workers to always throw errors when trying to set a specific tls version and cyphers.

@aswanidutt87
Copy link

hi @desean1625 currently I guess we are on 0.24.0 Uvicorn and passing ssl_version=int(ssl.PROTOCOL_TLS_SERVER) and ssl_ciphers=allowed_ciphers to the uvicorn.run() for our use case, if we are going to get impacted with new change, what will be the alternative for this change.

@desean1625
Copy link

@aswanidutt87 yes with gunicorn 21.0 and uvicorn 0.24 I am seeing this because a change in gunicorn.

gunicorn --ciphers ... -k uvicorn.workers.UvicornWorker --ssl-version TLSv1_2 causes ssl_version to be TLSv1_2 instead of int(5)

 File "/usr/local/lib/python3.11/site-packages/uvicorn/config.py", line 119, in create_ssl_context
    ctx = ssl.SSLContext(ssl_version)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/ssl.py", line 500, in __new__
    self = _SSLContext.__new__(cls, protocol)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: 'str' object cannot be interpreted as an integer

I tried gunicorn --ciphers ... -k uvicorn.workers.UvicornWorker --ssl-version 5 but still get a TypeError: 'str' object cannot be interpreted as an integer when the worker tries to start.

I suppose my question is would uvicorn add a check to parse --ssl-version 5 as an int until gunicorn completely stops sending that flag, or would allowing the support for custom ssl contexts make this obsolete.

@aswanidutt87
Copy link

This PR has been un attended since a year now, I am trying to add a custom ssl to the uvicorn as we need ssl_options and somebody in future might need something else in ssl_context so thought adding a custom ssl_context pass to uvicorn.run() a good idea, but couldn't go much far. @Kludex helped me until some point reviewing the changes and its been an year.
and Yes, adding custom ssl_context would resolve all this ssl_version and cipher as you bring your own ssl_context and pass to uvicorn.run.
please correct me @Kludex

@cschmutzer
Copy link

I am also interested in the ability to pass an SSL context. my use case is to have HTTPS using PSK instead of certificates

@aswanidutt87
Copy link

@Kludex , the need for passing custom ssl_context to the uvicorn is gaining momentum, please help me close this PR, its been two years that we are struggling to close this.

aswanidutt87 added a commit to aswanidutt87/uvicorn that referenced this issue Feb 19, 2024
@UnshiftedEarth
Copy link

@Kludex Has this feature been added yet? Seems like a pretty reasonable option as users may want to setup their own SSL context instead of being limited by basic parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants