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

JAMES-2961 Document outgoing SMTP security #2823

Closed
wants to merge 7 commits into from
Closed

JAMES-2961 Document outgoing SMTP security #2823

wants to merge 7 commits into from

Conversation

chibenwa
Copy link
Member

@chibenwa chibenwa commented Nov 4, 2019

No description provided.

@chibenwa chibenwa added the Documentation Contributes some documentation changes label Nov 4, 2019
@chibenwa chibenwa added this to the polish201910 milestone Nov 4, 2019
@chibenwa chibenwa self-assigned this Nov 4, 2019
@Arsnael
Copy link
Member

Arsnael commented Nov 5, 2019 

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.22.2:test (default-test) on project apache-james-mailbox-event-rabbitmq: There was a timeout or other error in the fork -> [Help 1]

? test this please

@blackheaven
Copy link

Commit name issue

mbaechler
mbaechler approved these changes Nov 5, 2019
View reviewed changes
Copy link

@mbaechler mbaechler left a comment

Choose a reason for hiding this comment

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

Shouldn't we enable starttls and starttls.required by default?

@chibenwa
Copy link
Member Author

chibenwa commented Nov 6, 2019

Commit name issue

I don't understand this cryptic comment.

A bit more context would be appreciated @blackheaven

@chibenwa
Copy link
Member Author

chibenwa commented Nov 6, 2019 •
edited

Shouldn't we enable starttls and starttls.required by default?

This require trust configuration. I fear people might get this wrong and start complain. Wildcard accept would be OK to me but this might deserve a mailing list question.

See https://www.mail-archive.com/server-dev@james.apache.org/msg62852.html

@chibenwa
Copy link
Member Author

chibenwa commented Nov 6, 2019 •
edited

I guess you cannot use ssl and tls at the same time (but it's just a guess, it should be checked)

No idea

I don't have a dockerized SSL SMTP testing server at hand for testing this.

@chibenwa chibenwa added the waiting_merge We are about to merge this! label Nov 6, 2019
@chibenwa
Copy link
Member Author

chibenwa commented Nov 6, 2019

test this please

@mbaechler
Copy link

This require trust configuration

Default Java behavior is not enough? I mean, it does certificate validation with know roots CA already, what would we lack?

@chibenwa
Copy link
Member Author

chibenwa commented Nov 6, 2019

Default Java behavior is not enough? I mean, it does certificate validation with know roots CA already, what would we lack?

I miss knowledge on this I must recognize.

@mbaechler
Copy link

Default Java behavior is not enough? I mean, it does certificate validation with know roots CA already, what would we lack?

I miss knowledge on this I must recognize.

It should work as expected but writing integration test for it is another thing entirely
It would require to add a CA into ssl configuration

@Arsnael
Copy link
Member

Arsnael commented Nov 7, 2019 

[f43fd0de2dd767c59f6c603aeb51027f85a1435a] [ERROR]   MemorySpamAssassinContractTest.deletingSpamMessageShouldNotImpactSpamAssassinState » ConditionTimeout
[f43fd0de2dd767c59f6c603aeb51027f85a1435a] [ERROR]   MemorySpamAssassinContractTest.movingAMailToTrashShouldNotImpactSpamassassinLearning » ConditionTimeout
[f43fd0de2dd767c59f6c603aeb51027f85a1435a] [ERROR]   MemorySpamAssassinContractTest.spamShouldBeDeliveredInSpamMailboxWhenSameMessageHasAlreadyBeenMovedToSpam » ConditionTimeout

test this please

@chibenwa
Copy link
Member Author

chibenwa commented Nov 7, 2019

Merged

@chibenwa chibenwa closed this Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbaechler mbaechler mbaechler approved these changes

@rouazana rouazana rouazana approved these changes

@Arsnael Arsnael Arsnael approved these changes

@trantienduchn trantienduchn trantienduchn approved these changes

Assignees

@chibenwa chibenwa

Labels
Documentation Contributes some documentation changes waiting_merge We are about to merge this!
Projects
None yet
Milestone
polish201910
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@chibenwa @Arsnael @blackheaven @mbaechler @rouazana @trantienduchn