Skip to content

Specify certfile to verify SSL certs against in tests #166

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.

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

Merged
merged 4 commits into from
Dec 10, 2014

Conversation

mtodd
Copy link
Member

@mtodd mtodd commented Dec 1, 2014

#162 added a TLS Start test but didn't assert verified certs because the tls_options hadn't been wired up yet. #161 changed this, but hadn't been tested against the additional test in #162 before getting merged, resulting in master failing.

It's easy enough to disable certificate verification, but it's not necessary. Instead, we can also set the :ca_file TLS options to verify against.

cc @jch @schaary @sonOfRa @tarcieri

cc @jch as I'm unsure if we want to go this route or make the cert verifiable instead
@mtodd mtodd changed the title Skip SSL certificate verification for test Specify certfile to verify SSL certs against in tests Dec 1, 2014
@mtodd
Copy link
Member Author

mtodd commented Dec 1, 2014

Updated title to match the changed approach.

else
File.expand_path("fixtures/cacert.pem", File.dirname(__FILE__))
end
end
Copy link
Member Author

Choose a reason for hiding this comment

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

Alternatively, we could generate a CA file if none is found instead of sticking one in test fixtures. Unsure if that'd be any better, though.

@mtodd mtodd mentioned this pull request Dec 1, 2014
@@ -0,0 +1,20 @@
-----BEGIN CERTIFICATE-----
Copy link

Choose a reason for hiding this comment

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

You might consider generating certificates (with e.g. certificate_authority or OpenSSL itself) rather than checking one into the repo

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, considered this, though that creates more friction for running tests locally, or at least running the integration tests locally.

In any case, I don't think we should block this fix on figuring out generated CA certs. That can come in a followup PR. Thoughts?

mtodd added a commit that referenced this pull request Dec 10, 2014
Specify certfile to verify SSL certs against in tests
@mtodd mtodd merged commit 14abcaf into master Dec 10, 2014
@mtodd mtodd deleted the starttls-test-verify-none branch December 10, 2014 00:40
@zmajstor
Copy link

there is a failing test:

Error: test_bind_tls_with_cafile(TestBindIntegration)
: OpenSSL::SSL::SSLError: SSL_connect returned=1 errno=0 state=SSLv3 read server certificate B: certificate verify failed

I think that the reason is that install-openldap creates self-signed CA certificate on VM setup, so
in order to make it pass, content of fixtures/cacert.pem have to be replaced with this one:

$ vagrant ssh

vagrant@rubyldap:~$ cat /etc/ssl/certs/cacert.pem

I guess there is a more elegant way to handle this ... but this works as a quick fix

@tarcieri
Copy link

@zmajstor see my original notes on the PR: certificates should be generated for tests, not checked in.

I'm a big fan of the certificate_authority gem for this. Otherwise, there's r509, but it's MRI-specific which kinda sucks.

astratto pushed a commit to astratto/ruby-net-ldap that referenced this pull request Dec 18, 2015
…none

Specify certfile to verify SSL certs against in tests
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