test_ssl_hostname_verification unit test error

[Duncan McGreggor]

In Python 2.5, 2.6, and 2.7 on Mac OS X with Twisted 11, I get a failing unit test:

===============================================================================
[ERROR]
Traceback (most recent call last):
  File "/Users/oubiwann/lab/txaws/github-txaws/txaws/client/tests/test_client.py", line 189, in test_ssl_hostname_verification
    query.get_page("https://example.com/file")
  File "/Users/oubiwann/lab/txaws/github-txaws/txaws/client/base.py", line 101, in get_page
    contextFactory = VerifyingContextFactory(host)
  File "/Users/oubiwann/lab/txaws/github-txaws/txaws/client/ssl.py", line 23, in __init__
    CertificateOptions.__init__(self, verify=True, caCerts=caCerts)
  File "/Users/oubiwann/lab/txaws/github-txaws/.venv-2.7/lib/python2.7/site-packages/twisted/internet/_sslverify.py", line 672, in __init__
    (not verify)), "Specify client CA certificate information if and only if enabling certificate verification"
exceptions.AssertionError: Specify client CA certificate information if and only if enabling certificate verification

txaws.client.tests.test_client.BaseQueryTestCase.test_ssl_hostname_verification
-------------------------------------------------------------------------------

Perhaps the Twisted (or PyOpenSSL) API has changed since this unit test was first created?

[Duncan McGreggor (oubiwann)]

Thomas, I've assigned to you since you created this test...

[Thomas Herve (therve)]

It's probably because CA are in a different place than /etc/ssl/certs/. I don't have access to an OS X machine, though.

[Duncan McGreggor (oubiwann)]

Cool, thanks for the tip -- I'll re-assign to me and look into it on my machine.

[Duncan McGreggor (oubiwann)]

Thomas,

Looks like Mac OS X puts them in /System/Library/OpenSSL/certs. However, there's nothing in that directory for me. My Ubuntu machines are chock-o-block full of certs in /etc/ssl/certs, so I'm not sure what's going on. Still looking into it.

[Duncan McGreggor (oubiwann)]

So, it looks like the only place where get_ca_certs is called is in get_global_ca_certs (txaws.client.ssl), and get_global_ca_certs doesn't accept any parameters. As such, there is no way that get_ca_certs's files parameter can be changed from "/etc/ssl/certs/*.pem" to something else in a unit test (without monkeypatching, that is).

The only place that get_global_ca_certs is called, seems to be VerifyingContextFactory.

So, an obvious workaround for this would be to add a "files" parameters to the constructor of VerifyingContextFactory. Yet VerifyingContextFactory is only ever constructed in the get_page method, so we'd have to pass "files" there. That has a bad smell.

That being said, passing around parameters just to get inside get_global_ca_certs to properly set files for a given operating system seems like a bad path to go down.

Even once that's done, there will still be the problem of what to do with an empty certs dir. Well, I guess not: we can just have the unit test and/or Makefile generate some certs that can be used in the test.

Regardless, this needs more thought.

[Duncan McGreggor (oubiwann)]

After looking at it for a while, I have these thoughts...

Since there's nothing anywhere that takes advantage of the "files" parameter, the simplest solution for now will probably be:

1) remove the parameter and do a sys.platform check in the get_ca_certs function, and
2) monkey patch get_ca_certs (or the module-level singleton "_ca_certs") for unit tests

[Duncan McGreggor (oubiwann)]

Ah, the perils of late-night pondering. I don't think my last proposition is a very good one; if monkeypatching in tests can be cleanly avoided, it should be.

So here's the next thought:

 1) move the "files" dir variable out of the function and put it at the module-level, DEFAULT_CERTS_PATH, complete with sys.platform checks.
 2) enable support for an environment variable CERTS_PATH; using the env actually makes sense in this case, since the location of certs could really be anywhere, depending upon not only the platform, but the way a given system has been set up.
 3) in the get_ca_certs function, do a check for CERTS_PATH; if no files are present there, use DEFAULT_CERTS_PATH instead.

The use of *_PATH here should evoke an intuitive response on the part of the txAWS user/developer/sysadmin. These variable should be used like the PATH env var: use colons to separate multiple paths.

Point #3 will help:
 * folks who want to just use whatever txAWS has defined as the default
 * folks who want to override that with their own certs path
 * folks who want to use multiple certs directories, and
 * unit tests that want to be able to test the default, an override, or the default + a custom path

This seems like an all-round better approach than my previous suggestion.