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

feat: support s/mime encryption #1946

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
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.

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
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.

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
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
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

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
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