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

Missing getters for alpn_protos #4952

Open
tiran opened this issue Dec 18, 2017 · 7 comments
Open

Missing getters for alpn_protos #4952

tiran opened this issue Dec 18, 2017 · 7 comments
Labels
branch: master Merge to master branch triaged: feature The issue/pr requests/adds a feature
Milestone

Comments

@tiran
Copy link
Contributor

tiran commented Dec 18, 2017

OpenSSL 1.1.0 has no getter to retrieve alpn_client_proto_list from SSL* and SSL_CTX*. Getters would be useful for introspection of context or SSL connection, see https://bugs.python.org/issue32359

I can create a PR after we have decided how the API should look like. Would you prefer:

unsigned SSL_CTX_get_alpn_protos(SSL_CTX *ctx, const unsigned char **protos)
{
    *protos = ctx->alpn_client_proto_list;
    return ctx->alpn_client_proto_list_len;
}

or

int SSL_CTX_get_alpn_protos(SSL_CTX *ctx, const unsigned char **protos, unsigned *protos_len)
{
    *protos = ctx->alpn_client_proto_list;
    *protos_len = ctx->alpn_client_proto_list_len;
    return 0;
}
@richsalz
Copy link
Contributor

My preference is for the second.

@ekasper
Copy link
Contributor

ekasper commented Dec 18, 2017

The second also aligns better with SSL_get0_alpn_selected. Use size_t for length instead of unsigned.

If you use an int rather than a void return (which is perhaps prudent to acommodate potential error conditions), please note that the convention (even though old OpenSSL APIs sometimes get it wrong) is 1 for success and 0 for error.

@tiran
Copy link
Contributor Author

tiran commented Dec 18, 2017

SGTM,

do you prefer a paranoid implementation? Or can I safely assume that the caller is not going to pass NULL to the getter and that alpn_client_proto_list_len is always 0 when alpn_client_proto_list is NULL?

int SSL_CTX_get_alpn_protos(SSL_CTX *ctx, const unsigned char **protos, size_t *protos_len)
{
    if (ctx == NULL) {
        *protos = NULL;
        *protos_len = 0;
        return 0;
    }
    *protos = ctx->alpn_client_proto_list;
    if (*protos == NULL)
        *protos_len = 0;
    else
        *protos_len = (size_t)ctx->alpn_client_proto_list_len;
    return 1;
}

@richsalz
Copy link
Contributor

It should be "get0" because you are passing pointers from an internal data structure. And it's simpler to document that both values are required (nudge, documentation). And if both are required, do NOT do the NULL check.

@mattcaswell
Copy link
Member

@tiran - what is the status of this?

@mattcaswell mattcaswell added this to the 1.1.1 milestone Jan 23, 2018
@mdebski
Copy link

mdebski commented Apr 13, 2018

Hi,
any updates?
I'm also interested in that, for TLS-ALPN validation method [0] in ACME protocol (i.e. Let's Encrypt certs)
I may consider implementing it myself if nobody is working on that, and if deemed necessary for my project.

[0] https://github.com/rolandshoemaker/acme-tls-alpn

@mattcaswell
Copy link
Member

Not a 1.1.1 specific issue so removing that milestone.

@mattcaswell mattcaswell modified the milestones: 1.1.1, Assessed Jul 2, 2018
@t8m t8m removed this from the Assessed milestone Jun 7, 2021
@t8m t8m added the triaged: feature The issue/pr requests/adds a feature label Jun 7, 2021
@t8m t8m added this to the Post 3.0.0 milestone Aug 9, 2021
@t8m t8m added the branch: master Merge to master branch label Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch: master Merge to master branch triaged: feature The issue/pr requests/adds a feature
Projects
None yet
Development

No branches or pull requests

6 participants