serilog / serilog-sinks-email

A Serilog sink that writes events to SMTP email
Apache License 2.0
70 stars 68 forks source link

Request for an "Ignore mail server certificate errors" setting in appsettings.config file #37

Closed KoshelevS closed 6 years ago

KoshelevS commented 7 years ago

Hi!

I have the following application setup:

All works fine up until the application logs a critical error. At that time I see the following error in my debug output console:

2017-07-07T12:08:38.6793862Z Failed to send email: System.Security.Authentication.AuthenticationException: The remote certificate is invalid according to the validation procedure.
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Net.Security.SslState.StartSendAuthResetSignal(ProtocolToken message, AsyncProtocolRequest asyncRequest, ExceptionDispatchInfo exception)
   at System.Net.Security.SslState.CheckCompletionBeforeNextReceive(ProtocolToken message, AsyncProtocolRequest asyncRequest)
   at System.Net.Security.SslState.StartSendBlob(Byte[] incoming, Int32 count, AsyncProtocolRequest asyncRequest)
   at System.Net.Security.SslState.ProcessReceivedBlob(Byte[] buffer, Int32 count, AsyncProtocolRequest asyncRequest)
   at System.Net.Security.SslState.ReadFrameCallback(AsyncProtocolRequest asyncRequest)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Net.Security.SslState.InternalEndProcessAuthentication(LazyAsyncResult lazyResult)
   at System.Net.Security.SslState.EndProcessAuthentication(IAsyncResult result)
   at System.Net.Security.SslStream.EndAuthenticateAsClient(IAsyncResult asyncResult)
   at System.Threading.Tasks.TaskFactory`1.FromAsyncCoreLogic(IAsyncResult iar, Func`2 endFunction, Action`1 endAction, Task`1 promise, Boolean requiresSynchronization)
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at MailKit.Net.Smtp.SmtpClient.Connect(String host, Int32 port, SecureSocketOptions options, CancellationToken cancellationToken)
   at MailKit.MailService.Connect(String host, Int32 port, Boolean useSsl, CancellationToken cancellationToken)
   at Serilog.Sinks.Email.EmailSink.OpenConnectedSmtpClient()
   at Serilog.Sinks.Email.EmailSink.<EmitBatchAsync>d__9.MoveNext()

It is obvious that no email message is received after that.

There is a way to ignore certificate errors using the ServerCertificateValidationCallback property of the EmailConnectionInfo class, but I can't go that way because all the sinks of Serilog are configured in appsettings.json file.

In my case it would be really helpful if Serilog.Sinks.Email is able to ignore certificate errors using a flag in the appsettings.json file, like:

  "Serilog": {
    "Using": [ "Serilog.Sinks.Literate", "Serilog.Sinks.Email" ],
    "MinimumLevel": "Information",
    "WriteTo": [
      { "Name": "LiterateConsole" },
      {
        "Name": "Email",
        "Args": {
          "fromEmail": "from@company.com",
          "toEmail": "to@company.com",
          "mailServer": "mail.company.com",
          "restrictedToMinimumLevel": "Fatal",
          "ignoreCertificateErrors":  true
        }
      }
    ]
  }

Is it possible?

Harmonickey commented 6 years ago

I would also like this

nblumhardt commented 6 years ago

Hi @KoshelevS @Harmonickey - thanks for the note and the PR.

We've been hesitant to build in this option in the past, as from a security standpoint it doesn't make a huge amount of sense. An untrusted root certificate means the certificate could have come from anywhere (e.g. MITM) - it's not providing any privacy nor validation of the server's identity whatsoever.

A better solution for self-signed certs is to trust the specific issuer (this can be set up using a short PowerShell script). Is that an option for you?

Cheers, Nick

Harmonickey commented 6 years ago

That does make sense. However, would it just be possible to let the calling client make that decision if they want to be secure or not, it wouldn't be handled by Serilog. Here is my pull-request. https://github.com/serilog/serilog-sinks-email/pull/39

nblumhardt commented 6 years ago

Sorry @Harmonickey - I misread your PR and thought you were adding the Boolean ignoreCertificateErrors option (which I'd like to avoid). Thanks for the reply.

We do already support what's proposed in your PR, don't we? https://github.com/Harmonickey/serilog-sinks-email/blob/0deb846a233c855116238763f0eb39e2faa3ed22/src/Serilog.Sinks.Email/LoggerConfigurationEmailExtensions.cs#L189 passing an EmailConnectionInfo object will allow the validation function to be specified. Did you spot this overload of WriteTo.Email()?

Harmonickey commented 6 years ago

You are right, I didn't catch that overload.

Harmonickey commented 6 years ago

I see what happened. I thought your library didn't include it because I was getting the NuGet package and grabbing the master branch. https://github.com/serilog/serilog-sinks-email/blob/master/src/Serilog.Sinks.Email/Sinks/Email/EmailConnectionInfo.cs

I will need to grab the dev branch I suppose?

Harmonickey commented 6 years ago

@nblumhardt would it be possible to push a release for what you have in dev right now? That way we can use the NuGet package with the extra field included in EmailConnectionInfo.cs

nblumhardt commented 6 years ago

Thanks @Harmonickey - on its way with #40

KoshelevS commented 6 years ago

@nblumhardt, thank you for response.

A better solution for self-signed certs is to trust the specific issuer (this can be set up using a short PowerShell script). Is that an option for you?

Option with a trusted self-signed certificate sounds good enough. I'm closing this issue.

Thank you!

nblumhardt commented 6 years ago

Great, thanks 👍