Enlarge _oid2txt buffer to handle larger OIDs

[frasertweedale]

The OpenSSL manual recommends a buffer size of 80 for OBJ_oid2txt:
https://www.openssl.org/docs/crypto/OBJ_nid2ln.html#return_values.
But OIDs longer than this occur in real life (e.g. Active Directory
makes some very long OIDs). Use a more conservative buffer size of
256 to prevent crashes if the length of the stringified-OID exceeds
80 characters.

[mention-bot]

@frasertweedale, thanks for your PR! By analyzing the history of the files in this pull request, we identified @alex, @reaperhulk and @intgr to be potential reviewers.

[alex]

It looks like you forgot to git add bigoid.pem.

[reaperhulk]

bigoid.pem will also need to be documented in the test-vectors.rst when it's added 😄

[frasertweedale]

@reaperhulk whups, thanks! test-vectors.rst update imminent.

[alex]

@stevepeak we appear to have hit a bug of some sort - the Changes page for this diff isn't showing the actual source, just a blank space:

[stevepeak]

Cool :) Nice edge case. I'll review 👍

[alex]

@stevepeak thanks, we appreciate it!

[frasertweedale]

Don't merge this; better fix coming.

[alex]

Is it possible to call OBJ_obj2txt with a NULL buffer to get the size?

[frasertweedale]

On Sun, May 28, 2017 at 07:18:22PM -0700, Alex Gaynor wrote: Is it possible to call `OBJ_obj2txt` with a `NULL` buffer to get the size?

This works. The downside is that we compute the conversion for the whole OID twice, every single time. With the first run using an 80-byte buffer we compute the conversion only once in the overwhelming majority of cases. Your call.

[alex]

I feel like the correct answer is "let's benchmark!", but that seems like way too much work to ask you to do for what should be simple. @reaperhulk or I will review as-is I think.

[tiran]

The fix is not sufficient. OBJ_obj2txt(buf, buf_len, obj, 1) does not return the actual required buffer size. You have to call the function with NULL buffer to make it calculate the correct size: OBJ_obj2txt(NULL, 0, obj, 1).

[reaperhulk]

(This is not accurate, a later tiran comment shows that it does return required buffer size)

[frasertweedale]

This is maybe not correct. More testing needed. Don't merge.

[reaperhulk]

(Confirmed that this is correct in a later comment by tiran)

[tiran]

Your code was correct. It looks like I screwed up while I was adopting your patch for CPython. The ssl module calls OBJ_obj2txt() in two different functions. The second time it calls the function with no_name=1. I think I mistakenly copied code with no_name=0.

test case

#include <stdio.h>
#include "openssl/x509.h"

int main(int argc, char **argv) {
    char buf[5];
    char largebuf[256];
    int buflen, i;
    ASN1_OBJECT *obj;
    obj = OBJ_nid2obj(NID_server_auth);

    for (i=0; i < 2; i++) {
        buflen = OBJ_obj2txt(NULL, 0, obj, i);
        fprintf(stdout, "NULL buffer\t\tno_name: %d\tbuflen: %d\n", i, buflen);
        buflen = OBJ_obj2txt(buf, sizeof(buf), obj, i);
        fprintf(stdout, "small buffer %d\t\tno_name: %d\tbuflen: %d\n",
                sizeof(buf), i, buflen);
        buflen = OBJ_obj2txt(largebuf, sizeof(largebuf), obj, i);
        fprintf(stdout, "large buffer %d\tno_name: %d\tbuflen: %d\n",
                sizeof(largebuf), i, buflen);
        fprintf(stdout, "%s\n", largebuf);
    }
}

output

$ gcc -lcrypto obj2txt.c -o obj2txt && ./obj2txt 
NULL buffer             no_name: 0      buflen: 29
small buffer 5          no_name: 0      buflen: 29
large buffer 256        no_name: 0      buflen: 29
TLS Web Server Authentication
NULL buffer             no_name: 1      buflen: 17
small buffer 5          no_name: 1      buflen: 17
large buffer 256        no_name: 1      buflen: 17
1.3.6.1.5.5.7.3.1

[frasertweedale]

@tiran thanks for the update 👍

[reaperhulk]

this approach is fine by me

[reaperhulk]

@alex if you have no objections I'll merge this shortly so it can be in 1.9.

[alex]

I haven't reviewed, but if you've reviewed I'm sure it's fine.

…

On Mon, May 29, 2017 at 5:32 PM, Paul Kehrer ***@***.***> wrote: @alex <https://github.com/alex> if you have no objections I'll merge this shortly so it can be in 1.9. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3612 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAADBMeRupZbA_CbhBXPUkiptyAntdDLks5r-zl7gaJpZM4NnHdW> .

-- "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: D1B3 ADC0 E023 8CA6