Default to enabling SSL check server identity#178
Conversation
jroper
force-pushed
the
default-ssl-checkserveridentity
branch
from
a75aa81 to
24c5b89
Compare
| // Results in "mail.smtp.localhost=127.0.0.1" and "mail.smtps.localhost=127.0.0.1" in the JavaMail session. | ||
|
|
||
| // If using SSL, we want to default to verifying that we trust the SSL certificate provided by the server. | ||
| ssl.checkserveridentity = true |
| "validate the configuration" in new WithApplication(applicationWithMoreMailerConfiguration) { | ||
| app.injector.instanceOf(bind[SMTPConfiguration]) must ===(SMTPConfiguration("typesafe.org", 25, user = Some("typesafe"), password = Some("typesafe"))) | ||
| app.injector.instanceOf(bind[SMTPConfiguration]) must ===(SMTPConfiguration("typesafe.org", 25, | ||
| user = Some("typesafe"), password = Some("typesafe"), props = ConfigFactory.parseString("ssl.checkserveridentity=true"))) |
There was a problem hiding this comment.
So when constructing the SMTPConfiguration from code rather than from configuration the default is still off. That might still be a risk.
There was a problem hiding this comment.
Probably the best we can do is add a log WARN when that happens.
case class SMTPConfiguration(host: String,
port: Int,
ssl: Boolean = false,
tls: Boolean = false,
tlsRequired: Boolean = false,
user: Option[String] = None,
password: Option[String] = None,
debugMode: Boolean = false,
timeout: Option[Int] = None,
connectionTimeout: Option[Int] = None,
props: Config = ConfigFactory.empty(),
mock: Boolean = false) {
private val logger: Logger =
LoggerFactory.getLogger(classOf[SMTPConfiguration])
if ((ssl || tls) && !props.getBoolean(
"ssl.checkserveridentity"
))
logger.warn(
"You are using ssl/tls but no hostname verification. Enable hostname verification using play.mailer.props.ssl.checkserveridentity = true"
)
}There was a problem hiding this comment.
I think it's fine as is, no one loads one of this from code, if you're going to all that effort, I don't know why you'd even use this library, it doesn't do much.
ennru
force-pushed
the
default-ssl-checkserveridentity
branch
from
24c5b89 to
0135382
Compare
|
Something touches a file so that my variant of using dynver to get align versions doesn't work https://travis-ci.com/github/playframework/play-mailer/jobs/336132876#L517 |
|
Needs backport to |
|
Users affected by the CVE can just add the setting and restart the service. |
4 participants
I've tested this locally to ensure that SSL connections fail when the certificate presented doesn't match the hostname used, and to ensure that they succeed when it does. Also I tested and verified by manual code inspection of the Java mail library that enabling this when SSL is not in use won't impact anything adversely.