Skip to content

Allow :ca_path or :ca_file to be passed in as settings, for SSL context#399

Merged
jeremy merged 1 commit into
mikel:masterfrom
ndbroadbent:configure_ca_path_or_file
Jan 23, 2013
Merged

Allow :ca_path or :ca_file to be passed in as settings, for SSL context#399
jeremy merged 1 commit into
mikel:masterfrom
ndbroadbent:configure_ca_path_or_file

Conversation

@ndbroadbent

Copy link
Copy Markdown
Contributor

RE: Issue #345

Also refactored SSL context code in Mail::SMTP to use a shared private method: #ssl_context.

@ffa

ffa commented May 22, 2012

Copy link
Copy Markdown

I support this pull request. This pull request adds the setting for validating ca certificates. Without this, SSL/TLS may be much more easily spoofed and therefore renders SSL/TLS considerable less secure. Furthermore, the default setting is for ca_path or ca_file to be used.

@mikel

mikel commented Nov 17, 2012

Copy link
Copy Markdown
Owner

Hi there, looks like a good idea. Though making the default to require ca_path and ca_file is going to cause a lot of grief. Please change the PR to make current behaviour the default and I'll merge in.

@mikel

mikel commented Nov 17, 2012

Copy link
Copy Markdown
Owner

Hi there, just took another look at your pull request, and seems like you are defaulting to using the dummy context requiring the ca stuff by default which is good.... however, we recently merged a pull request in to make SSL work on Ruby 1.8.7 and this now conflicts.

Could you rebase against master?

@jeremy

jeremy commented Jan 19, 2013

Copy link
Copy Markdown
Collaborator

Ping! @ndbroadbent up for rebasing?

@ndbroadbent

Copy link
Copy Markdown
Contributor Author

Rebased!

jeremy added a commit that referenced this pull request Jan 23, 2013
Allow :ca_path or :ca_file to be passed in as settings, for SSL context
@jeremy jeremy merged commit 1e31ef4 into mikel:master Jan 23, 2013
jeremy added a commit that referenced this pull request Jan 23, 2013
@jeremy

jeremy commented Jan 23, 2013

Copy link
Copy Markdown
Collaborator

Thanks @ndbroadbent!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants