Skip to content

feat: support s/mime encryption #1946

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.

Sign up for GitHub

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

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

feat: support s/mime encryption #1946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions app/dashboard/views/mailbox_detail.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,53 @@ def mailbox_detail_route(mailbox_id):

elif request.form.get("form-name") == "toggle-pgp":
if request.form.get("pgp-enabled") == "on":
if not mailbox.disable_smime:
mailbox.disable_smime = True
flash(f"S/MIME is disabled on {mailbox.email}", "warning")
mailbox.disable_pgp = False
flash(f"PGP is enabled on {mailbox.email}", "success")
else:
mailbox.disable_pgp = True
flash(f"PGP is disabled on {mailbox.email}", "info")

Session.commit()
return redirect(
url_for("dashboard.mailbox_detail_route", mailbox_id=mailbox_id)
)
elif request.form.get("form-name") == "smime":
if request.form.get("action") == "save":
if not current_user.is_premium():
flash("Only premium plan can add S/MIME Key", "warning")
return redirect(
url_for("dashboard.mailbox_detail_route", mailbox_id=mailbox_id)
)

mailbox.smime_public_key = request.form.get("smime")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you:

  • Verify that this is a valid PEM file.
  • Add a check that the key is not bigger than 15KiB? A PEM chain with several certs shouldn't be bigger than that.

Copy link
Author

Choose a reason for hiding this comment

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

These two seem fairly straightforward. Would solution like so make sense?

from pem import parse
from sys import getsizeof

...

raw_smime_cert_input = request.form.get("smime")
if getsizeof(raw_smime_cert_input) > 15360:
    flash("S/MIME certificate is too big! Must be smaller than 15 KiB")
    return redirect(
        url_for("dashboard.mailbox_detail_route", mailbox_id=mailbox_id)
    )
smime_certs = parse(raw_smime_cert_input)
if not smime_certs[0]:
    flash("Unable to validate S/MIME certificate")
    return redirect(
        url_for("dashboard.mailbox_detail_route", mailbox_id=mailbox_id)
    )
mailbox.smime_public_key = str(smime_certs[0])

Session.commit()
flash("Your S/MIME public key is saved successfully", "success")
return redirect(
url_for("dashboard.mailbox_detail_route", mailbox_id=mailbox_id)
)
elif request.form.get("action") == "remove":
# Free user can decide to remove their added S/MIME key
mailbox.smime_public_key = None
mailbox.disable_smime = False
Session.commit()
flash("Your S/MIME public key is removed successfully", "success")
return redirect(
url_for("dashboard.mailbox_detail_route", mailbox_id=mailbox_id)
)
elif request.form.get("form-name") == "toggle-smime":
if request.form.get("smime-enabled") == "on":
if not mailbox.disable_pgp:
mailbox.disable_pgp = True
flash(f"PGP is disabled on {mailbox.email}", "warning")
mailbox.disable_smime = False
flash(f"S/MIME is enabled on {mailbox.email}", "success")
else:
mailbox.disable_smime = True
flash(f"S/MIME is disabled on {mailbox.email}", "info")

Session.commit()
return redirect(
url_for("dashboard.mailbox_detail_route", mailbox_id=mailbox_id)
Expand Down
12 changes: 12 additions & 0 deletions app/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2566,6 +2566,12 @@ class Mailbox(Base, ModelMixin):
sa.Boolean, default=False, nullable=False, server_default="0"
)

# smime
smime_public_key = sa.Column(sa.Text, nullable=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you move this into a different model like MailboxSMimeKey ? We load mailboxes all the time from the db and the lighter they are, the better. We only need to load the key when we're about to use it.

Copy link
Author

Choose a reason for hiding this comment

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

If I understand correctly, this would mean to create new table for S/MIME certificates, and then add the public key contents to the new table instead of as a field in the mailbox. Then, reference the entry when we want to sign the emails so that the cert is not loaded from the DB as often?

Want to clarify as I think it will be a bit more complex logic, so I would need to take more time to understand these models and the DB structure.

disable_smime = sa.Column(
sa.Boolean, default=True, nullable=False, server_default="0"
)

# incremented when a check is failed on the mailbox
# alert when the number exceeds a threshold
# used in sanity_check()
Expand All @@ -2588,6 +2594,12 @@ def pgp_enabled(self) -> bool:

return False

def smime_enabled(self) -> bool:
if self.smime_public_key and not self.disable_smime:
return True

return False

def nb_alias(self):
return (
AliasMailbox.filter_by(mailbox_id=self.id).count()
Expand Down
37 changes: 37 additions & 0 deletions email_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@
"""
import argparse
import email
import smail
import time
import uuid
from asn1crypto import pem, x509
from email import encoders
from email.encoders import encode_noop
from email.message import Message
Expand Down Expand Up @@ -535,6 +537,21 @@ def prepare_pgp_message(
return msg


def prepare_smime_message(orig_msg: Message, public_key: str) -> Message:
# clone orig message to avoid modifying it
clone_msg = copy(orig_msg)

# create certificate object using public key
_, _, der_bytes = pem.unarmor(public_key.encode())
cert = x509.Certificate.load(der_bytes)

# encrypt the message
clone_msg = smail.encrypt_message(clone_msg, [cert])

# return the message
return clone_msg


def sign_msg(msg: Message) -> Message:
container = MIMEMultipart(
"signed", protocol="application/pgp-signature", micalg="pgp-sha256"
Expand Down Expand Up @@ -908,6 +925,26 @@ def forward_email_to_mailbox(
f"""PGP encryption fails with {mailbox.email}'s PGP key""",
)

# create SMIME email if needed
if mailbox.smime_enabled() and user.is_premium():
LOG.d("Encrypt message using S/MIME for mailbox %s", mailbox)

try:
msg = prepare_smime_message(msg, mailbox.smime_public_key)
except Exception as exceptasdf:
LOG.w(
"Cannot S/MIME encrypt message %s -> %s. %s %s",
contact,
alias,
mailbox,
user,
)
LOG.w(exceptasdf)
Comment on lines +934 to +942
Copy link
Author

Choose a reason for hiding this comment

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

Oops, sorry; I can clean up this variable name and logging

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

msg = add_header(
msg,
f"""S/MIME encryption fails with {mailbox.email}'s S/MIME key""",
)

# add custom header
add_or_replace_header(msg, headers.SL_DIRECTION, "Forward")

Expand Down
31 changes: 31 additions & 0 deletions migrations/versions/2023_111521_fb0ab73c1825_.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
"""empty message

Revision ID: fb0ab73c1825
Revises: 4bc54632d9aa
Create Date: 2023-11-15 21:50:40.424160

"""
import sqlalchemy_utils
from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision = 'fb0ab73c1825'
down_revision = '4bc54632d9aa'
branch_labels = None
depends_on = None


def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.add_column('mailbox', sa.Column('disable_smime', sa.Boolean(), server_default='0', nullable=False))
op.add_column('mailbox', sa.Column('smime_public_key', sa.Text(), nullable=True))
# ### end Alembic commands ###


def downgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.drop_column('mailbox', 'smime_public_key')
op.drop_column('mailbox', 'disable_smime')
# ### end Alembic commands ###
47 changes: 45 additions & 2 deletions poetry.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ SQLAlchemy = "1.3.24"
redis = "^4.5.3"
newrelic-telemetry-sdk = "^0.5.0"
aiospamc = "0.10"
python-smail = "^0.9.0"

[tool.poetry.dev-dependencies]
pytest = "^7.0.0"
Expand Down
Loading