[Django] #31885: add EmalBackend CA parameters #13305
Django
-----------------------------------------+------------------------
Reporter: Luis Saavedra | Owner: nobody
Type: Bug | Status: new
Component: Core (Mail) | Version: master
Severity: Normal | Keywords: SSL
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
set ssl cert and key outside a SSLContext object are deprecated and we
need add CA parameters to make the server authentication
https://github.com/django/django/pull/13305
--
Ticket URL: <https://code.djangoproject.com/ticket/31885>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------+-----------------------------------------
Reporter: Luis Saavedra | Owner: Luis Saavedra
Type: Bug | Status: assigned
Component: Core (Mail) | Version: master
Keywords: SSL | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => assigned
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
* owner: nobody => Luis Saavedra
* needs_docs: 0 => 1
* has_patch: 0 => 1
* stage: Unreviewed => Accepted
Old description:
> set ssl cert and key outside a SSLContext object are deprecated and we
> need add CA parameters to make the server authentication
>
> https://github.com/django/django/pull/13305
New description:
Setting SSL cert and key in
[https://docs.python.org/3.8/library/smtplib.html#smtplib.SMTP.starttls
`SMTP.starttls()`] is deprecated (since Python 3.6) in favour of passing
an SSLContext. Update the SMTP backend to use the newer API.
https://github.com/django/django/pull/13305
--
Comment:
Thanks for the report. Yes, an update here sounds good.
We'll need docs and tests for the changes. You've added new settings,
those will need documenting too.
Then, do we need to deprecate the old settings, as no longer appropriate?
--
Ticket URL: <https://code.djangoproject.com/ticket/31885#comment:1>
Django
-------------------------------+-----------------------------------------
Reporter: Luis Saavedra | Owner: Luis Saavedra
Type: Bug | Status: assigned
Keywords: SSL | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (by Luis Saavedra):
The old settings are good because set the authenticity credentials of a
client, this pull request add the CA parameters to check the authenticity
of the server. A man in the midle attack can pass any certificate to the
client and the client can't check the authenticity because dont have CA
parameters.
--
Ticket URL: <https://code.djangoproject.com/ticket/31885#comment:2>
Django
-------------------------------+-----------------------------------------
Reporter: Luis Saavedra | Owner: Luis Saavedra
Type: Bug | Status: assigned
Keywords: SSL | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
* needs_docs: 1 => 0
Comment:
Updating flags to put back in the review queue
--
Ticket URL: <https://code.djangoproject.com/ticket/31885#comment:3>
Django
-------------------------------+-----------------------------------------
Reporter: Luis Saavedra | Owner: Luis Saavedra
Type: Bug | Status: assigned
Keywords: SSL | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 0 => 1
Comment:
PR looks good. Just needs to add deprecations for the old keyfile and
certfile way of doing things.
--
Ticket URL: <https://code.djangoproject.com/ticket/31885#comment:4>
Django
-------------------------------+-----------------------------------------
Reporter: Luis Saavedra | Owner: Luis Saavedra
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: SSL | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
So
[https://github.com/django/django/pull/13305#pullrequestreview-625047316
conclusion on the PR] is that we should add the option to accept the
parameters for an SSLContext on EmailBackend but **not** add them as
settings. (The default is to use the system CA certs which is what most
people want/need.)
We will then recommend subclassing in the docs for more control.
At the same time we should deprecate EMAIL_SSL_CERTFILE and
EMAIL_SSL_KEYFILE for the same reasons.
--
Ticket URL: <https://code.djangoproject.com/ticket/31885#comment:5>
Django
-------------------------------+-----------------------------------------
Reporter: Luis Saavedra | Owner: Luis Saavedra
Type: Bug | Status: assigned
Component: Core (Mail) | Version: dev
Severity: Normal | Resolution:
Keywords: SSL | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* cc: Carlton Gibson (added)
Comment:
`EmailBackend` uses `SSLContext` since
2848e5d0ce5cf3c31fe87525536093b21d570f69. Users can subclass
`EmailBackend` and override the `ssl_context` property to use custom
cart/key files. Maybe it's enough to consider this ticket as fixed 🤔
--
Ticket URL: <https://code.djangoproject.com/ticket/31885#comment:6>
Django
-------------------------------+-----------------------------------------
Reporter: Luis Saavedra | Owner: Luis Saavedra
Component: Core (Mail) | Version: dev
Keywords: SSL | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: assigned => closed
* resolution: => fixed
Comment:
> ...override the ssl_context property to use custom crt/key files.
The `ssl_context` still uses the settings, so not sure they need to
subclass even.
(I still we expose too much of `smtplib` through the settings here, but
progress there is likely Jacob's proposal from https://groups.google.com/g
/django-developers/c/R8ebGynQjK0/m/kc-zggaxAgAJ)
I agree the narrow ticket here was resolved in
2848e5d0ce5cf3c31fe87525536093b21d570f69. 👍
--
Ticket URL: <https://code.djangoproject.com/ticket/31885#comment:7>