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.

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

load_client_ca does nothing #387

Closed
glyph opened this issue Nov 11, 2015 · 10 comments
Closed

load_client_ca does nothing #387

glyph opened this issue Nov 11, 2015 · 10 comments
Milestone

Comments

@glyph
Copy link
Contributor

glyph commented Nov 11, 2015

The method body of Context.load_client_ca is empty. That seems like it is probably not what callers expect.

@Lukasa
Copy link
Member

Lukasa commented Nov 11, 2015

Uh...yes. That seems...wrong?

@hynek
Copy link
Contributor

hynek commented Jan 31, 2016

pyOpenSSL 0.13 does this:

static char ssl_Context_load_client_ca_doc[] = "\n\
Load the trusted certificates that will be sent to the client (basically\n\
telling the client \"These are the guys I trust\").  Does not actually\n\
imply any of the certificates are trusted; that must be configured\n\
separately.\n\
\n\
:param cafile: The name of the certificates file\n\
:return: None\n\
";
static PyObject *
ssl_Context_load_client_ca(ssl_ContextObj *self, PyObject *args)
{
    char *cafile;

    if (!PyArg_ParseTuple(args, "s:load_client_ca", &cafile))
        return NULL;

    SSL_CTX_set_client_CA_list(self->ctx, SSL_load_client_CA_file(cafile));

    Py_INCREF(Py_None);
    return Py_None;
}

Unfortunately, SSL_load_client_CA_file isn’t bound in cryptography. Maybe there’s another way? Pinning to higher cryptography options is sadly not an option due to the CFFI/PyPy/Travis situation.

@alex
Copy link
Member

alex commented Jan 31, 2016

I'll go ahead and send a PR to cryptography anyways, so that it's at least
there in the future.

On Sun, Jan 31, 2016 at 9:59 AM, Hynek Schlawack [email protected]
wrote:

pyOpenSSL 0.13 does this:

static char ssl_Context_load_client_ca_doc[] = "\n\Load the trusted certificates that will be sent to the client (basically\n\telling the client "These are the guys I trust"). Does not actually\n\imply any of the certificates are trusted; that must be configured\n\separately.\n\n:param cafile: The name of the certificates file\n:return: None\n";static PyObject *ssl_Context_load_client_ca(ssl_ContextObj *self, PyObject *args)
{
char *cafile;

if (!PyArg_ParseTuple(args, "s:load_client_ca", &cafile))
    return NULL;

SSL_CTX_set_client_CA_list(self->ctx, SSL_load_client_CA_file(cafile));

Py_INCREF(Py_None);
return Py_None;

}

Unfortunately, SSL_load_client_CA_file isn’t bound in cryptography. Maybe
there’s another way? Pinning to higher cryptography options is sadly not an
option due to the CFFI/PyPy/Travis situation.


Reply to this email directly or view it on GitHub
#387 (comment).

"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: 125F 5C67 DFE9 4084

@glyph
Copy link
Contributor Author

glyph commented Jan 31, 2016

Pinning to higher cryptography options is sadly not an option due to the CFFI/PyPy/Travis situation.

I'm curious as to why you say this. https://github.com/rackerlabs/mimic/ uses pyenv to get a current pypy and it's been working better for us than trying to work around travis's ancient and busted version…

@hynek
Copy link
Contributor

hynek commented Jan 31, 2016

I don't care about our own CI; I care about the wave of anger and frustration pouring over me if we released a pyOpenSSL that renders every depending project untestable on Travis without jumping through pyenv hoops…

@glyph
Copy link
Contributor Author

glyph commented Jan 31, 2016

Ah, gotcha.

@mithrandi
Copy link

Maybe we could conditionally fall back to doing nothing if the necessary binding isn't importable? This is terrible, of course, but I don't think it's any less terrible than the current behaviour of always doing nothing.

@mithrandi
Copy link

The cryptography PR is pyca/cryptography#2703 btw.

@hynek
Copy link
Contributor

hynek commented Feb 1, 2016

I have thought about opening this can of worms but it adds a new dimension of complexity I would rather avoid.

@hynek
Copy link
Contributor

hynek commented Feb 1, 2016

(but it's possible that we won't be able to get around that anyway: #419 :|)

@hynek hynek added this to the 16.0.0 milestone Feb 14, 2016
@hynek hynek mentioned this issue Feb 27, 2016
1 task
@hynek hynek closed this as completed Mar 19, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

5 participants