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.

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

Jump to bottom

Bug/notifications/email: Content needs <html>...</html> tags #1316

Closed
Athanasius opened this issue Mar 5, 2022 · 15 comments
Closed

Bug/notifications/email: Content needs <html>...</html> tags #1316

Athanasius opened this issue Mar 5, 2022 · 15 comments
Labels
kind/bug Something isn't working

Comments

@Athanasius
Copy link

Describe the bug
The default config for email notifications can trigger a high-scoring Spamassassin rules due to bare HTML without <html>...</html> enclosing tags.

To Reproduce
Steps to reproduce the behavior:

  1. Set up email notifications, with minimal edits to the default notifications/email.yaml
  2. Trigger an email
  3. Check the content of the solitary text/html attachment

Expected behavior
All reasonable attempts should be made for these emails to not look like spam.

Technical Information (please complete the following information):

  • OS: Debian buster (currently oldstable)
  • Version: crowdsec 1.3.2 from the APT repository

Additional context
Spamassassin reports the following on crowdsec notification emails:

        *  3.8 HTML_MIME_NO_HTML_TAG HTML-only message, but there is no HTML
        *      tag

and indeed the only part of a crowdsec notification email starts with:

<a href=...

Now, obviously, I've gone and whitelisted (won't even go through Spamassassin processing) the crowdsec emails in question now, and I can tweak my local config file to add the missing tags (presumably also <body>), but this is a small improvement that could be made to the defaults.
@Athanasius Athanasius added the kind/bug Something isn't working label Mar 5, 2022
@Athanasius
Copy link
Author

I note this also affects the registration emails for the console:

        *  3.8 HTML_MIME_NO_HTML_TAG HTML-only message, but there is no HTML
        *      tag

buixor added a commit that referenced this issue Mar 10, 2022
@buixor
fix #1316 : add html body
e1c388a
@buixor
Copy link
Contributor

buixor commented Mar 10, 2022

@Athanasius although naive, would this do trick : #1339 ?

@Athanasius
Copy link
Author

Athanasius commented Mar 10, 2022 •
edited

@Athanasius although naive, would this do trick : #1339 ?

Yes, that's exactly the sort of thing I've applied in my local version of the config:

# The output goes in the email message body
format: |
  {{range . -}}
    {{$alert := . -}}
    {{range .Decisions -}}
      <html><body><a href=https://www.whois.com/whois/{{.Value}}>{{.Value}}</a> will get <b>{{.Type}}</b> for next <b>{{.Duration}}</b> for triggering <b>{{.Scenario}}</b> on machine <b>{{$alert.MachineID}}</b>. <a href=https://www.shodan.io/host/{{.Value}}>Shodan</a></html></body>
    {{end -}}
  {{end -}}

neomutt is happy to display it, and ... ah, well now Spamassassin (I stopped the 'live' messages from going through it) has another hit on its SCC_BODY_URI_ONLY rule. Let me check into that and get back to you so we can fashion the best format for this in one go.

@Athanasius
Copy link
Author

OK, it appears that SCC_BODY_URI_ONLY just wants more than one rendered line in the HTML So, something like:

# The output goes in the email message body
format: |
  {{range . -}}
    {{$alert := . -}}
    {{range .Decisions -}}
      <html><body><p><a href=https://www.whois.com/whois/{{.Value}}>{{.Value}}</a> will get <b>{{.Type}}</b> for next <b>{{.Duration}}</b> for triggering <b>{{.Scenario}}</b> on machine <b>{{$alert.MachineID}}</b>.</p> <p><a href=https://www.shodan.io/host/{{.Value}}>Shodan {{.Value}}</a></p></body></html>
    {{end -}}
  {{end -}}

I've added the IP into the Shodan link text so it's more obvious it's a link for the IP, not a general Shodan one.

The full SpamAssassin score in my setup is now:

 pts rule name              description
---- ---------------------- --------------------------------------------------
-1.0 ALL_TRUSTED            Passed through trusted hosts only via SMTP
 1.5 BAYES_50               BODY: Bayes spam probability is 40 to 60%
                            [score: 0.4978]
 0.1 MIME_HTML_ONLY         BODY: Message only has text/html MIME parts
 0.0 HTML_MESSAGE           BODY: HTML included in message
 0.0 MIME_QP_LONG_LINE      RAW: Quoted-printable line longer than 76
                            chars
-0.0 T_SCC_BODY_TEXT_LINE   No description available.
 1.2 MISSING_MID            Missing Message-Id: header
 1.0 HK_NAME_FROM           No description available.

@Athanasius
Copy link
Author

HK_NAME_FROM will go away when a version that allows setting other than harcoded 'From' sender is released.

If you can convince the golang Mail code that's being used to add a Message-Id heade then MISSING_MID will go away.

@buixor
Copy link
Contributor

buixor commented Mar 10, 2022

if you have more than one decision/alert, won't spamassassin complain about the multiple opening/closing html/body tags ?

@Athanasius
Copy link
Author

if you have more than one decision/alert, won't spamassassin complain about the multiple opening/closing html/body tags ?

I've not yet seen an email with multiple decisions in it. All I can say is that the lack of them at all in this single alert case is an issue.

A quick test shows SA not caring about multiple <html><body>...</body></html> in the same attachment. I can't speak for any other anti-spam detection.

@buixor
Copy link
Contributor

buixor commented Mar 10, 2022

thanks, updated the PR, let me know if it sounds good to you and we merge

@Athanasius
Copy link
Author

thanks, updated the PR, let me know if it sounds good to you and we merge

I'm still seeing only the two commits there. Forgot to push ?

@Athanasius
Copy link
Author

Ah, the second one just reformatted. You're missing the <p>...</p> tags that will actually make it two rendered lines to avoid that SCC_BODY_URI_ONLY rule triggering.

@buixor
Copy link
Contributor

buixor commented Mar 10, 2022

ah thanks, missed that one, should be fixed now :)

@Athanasius
Copy link
Author

No, it needs to be TWO HTML paragraphs. That's why I moved the 'Shodan' link into its own.

@buixor
Copy link
Contributor

buixor commented Mar 10, 2022

ah yes, slow day 😅

@Athanasius
Copy link
Author

OK, looks good now (I still like the extra {{.Value}} in the Shodan link text, but I'm editing my local active config anyway of course).

@buixor
Copy link
Contributor

buixor commented Mar 10, 2022

Yes, I think we need to revamp this, I think we might now want to include a link to the console's CTI for extra info.

I will close this one after approval/merge of the PR and keep further modifications on the template in a separate one !

@buixor buixor closed this as completed in 6f68f40 Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Athanasius @buixor