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

[Bug]: Potential Regression in Parsing Invalid Certificates #7919

Open
rumpelsepp opened this issue Aug 29, 2024 · 5 comments
Open

[Bug]: Potential Regression in Parsing Invalid Certificates #7919

rumpelsepp opened this issue Aug 29, 2024 · 5 comments
Assignees
Labels

Comments

@rumpelsepp
Copy link

rumpelsepp commented Aug 29, 2024

CC

@peckto

Version

bd15538

Description

Hi! I am opening this issue to share an conspicuousness I found during my research work. We did some tests with a lot of publicly available X.509 certificates, see our paper here. After evaluating the data, we came up with a difference in parsing between wolfssl 5.5 und 5.7 (see Table 6c in the paper).

Recently, I was finally able to bisect the issue to commit bd15538. Before this commit, the certificates in the attachement could not be parsed using wolfSSL_X509_d2i(). After this commit 14 out of 17 can be parsed by wolfssl. I am not sure which case is the most correct one. However, Go fails to parse all these certificates with an "x509: invalid ECDSA parameter" error; zlint is not able to parse them, too. Assuming that Go's behaviour is correct, then this could be a wolfssl regression. If wolfssl parses correctly, it's a Go bug.

I would be thankful if you could shed some light on this.

wolfssl-5.5.4-certs-parsed.txt

Reproduction steps

$ ./configure \
        --enable-all \
        --enable-reproducible-build \
        --enable-pkcs11 \
        --enable-writedup \
        --enable-base64encode \
        --enable-pkcs11 \
        --enable-writedup \
        --enable-base64encode \
        --enable-bigcache \
        --enable-sp=yes,asm \
        --enable-sp-math-all \
        --enable-harden \
        --enable-intelasm \
        --enable-aesni

My relevant C snippet is as follows; I omitted the wrapping code of my test tool.

int test_certificate(unsigned char *data, size_t data_len, struct test_result *res) {
    WOLFSSL_X509 *cert;
    cert = wolfSSL_X509_d2i(&cert, data, data_len);
    if (!cert) {
        res->success = false;
        wolfSSL_ERR_error_string_n(wolfSSL_ERR_get_error(), res->error_msg, sizeof(res->error_msg)-1);
        return 0;
    }

    wolfSSL_X509_free(cert);
    res->success = true;
    return 0;
}

Relevant log output

Logfile (of my testsuite) with error messages after bd155389e205f662d66f3bdcd262d32f9d70ef51

----
$ cat c/c-wolfssl-1724853348440292-1469481-results.json  | jq -c 'select(.success == false)'
{"id":1,"sha256_fingerprint":"V57Pqx9qvWQ7LnAVnzpfgBJmU+qK3YWed6TJ8AOwzVw=","start_time":1724853348440412,"end_time":1724853348443259,"error_msg":"Setting Cert Public Key error","success":false}
{"id":7,"sha256_fingerprint":"iwDWtGcdMav5UXeSSDTbU8ERV80EgR90j3UgA3B9Sss=","start_time":1724853348443413,"end_time":1724853348443439,"error_msg":"Setting Cert Public Key error","success":false}
{"id":13,"sha256_fingerprint":"Hc3z577W37+RBMq5llvOFkQTKMOcElhrrYwInrSTPtI=","start_time":1724853348443546,"end_time":1724853348443566,"error_msg":"Setting Cert Public Key error","success":false}
----


Logfile (of my testsuite) with error messages before bd155389e205f662d66f3bdcd262d32f9d70ef51

----
$ cat c/c-wolfssl-1724935339261878-1731052-results.json  | jq -c 'select(.success == false) '
{"id":1,"sha256_fingerprint":"V57Pqx9qvWQ7LnAVnzpfgBJmU+qK3YWed6TJ8AOwzVw=","start_time":1724935339261981,"end_time":1724935339263476,"error_msg":"ok","success":false}
{"id":2,"sha256_fingerprint":"GkD75ohk2t2WMUUfkGRIJfpAn9sOEpxlmmL8lghND9c=","start_time":1724935339263503,"end_time":1724935339263506,"error_msg":"ok","success":false}
{"id":3,"sha256_fingerprint":"pJOGCBF/DwgJqA8s/nEDZib1FcCXc9tiaQHHKFPmMfM=","start_time":1724935339263510,"end_time":1724935339263512,"error_msg":"ok","success":false}
{"id":4,"sha256_fingerprint":"yeZp+p/tKh4A7osoTsZQhHmrgS3ttYZubb4CqEVuYdc=","start_time":1724935339263515,"end_time":1724935339263518,"error_msg":"ok","success":false}
{"id":5,"sha256_fingerprint":"z+yGEV1EA4G890lZHTVEek7cDyXZM03KXcXaMMoGf2s=","start_time":1724935339263523,"end_time":1724935339263525,"error_msg":"ok","success":false}
{"id":6,"sha256_fingerprint":"CwVbCRL6RpBn89VcH/8vxGLnLfcvtLR2RoiSs86UAS8=","start_time":1724935339263527,"end_time":1724935339263529,"error_msg":"ok","success":false}
{"id":7,"sha256_fingerprint":"iwDWtGcdMav5UXeSSDTbU8ERV80EgR90j3UgA3B9Sss=","start_time":1724935339263534,"end_time":1724935339263547,"error_msg":"ok","success":false}
{"id":8,"sha256_fingerprint":"Oq0AWKUinOwDDt34YIhKD6kApov6QIjA8ZwVFMHAhd8=","start_time":1724935339263551,"end_time":1724935339263553,"error_msg":"ok","success":false}
{"id":9,"sha256_fingerprint":"R8leXa7i+6OsHYrgKXJkF1KpkDo3+9n7EIukrg5YKTg=","start_time":1724935339263557,"end_time":1724935339263558,"error_msg":"ok","success":false}
{"id":10,"sha256_fingerprint":"0WcBWKTBqIfNfB+3m/PoyvY05oaJi+i0iQ4JwJ3ehvI=","start_time":1724935339263561,"end_time":1724935339263563,"error_msg":"ok","success":false}
{"id":11,"sha256_fingerprint":"Pyyp9FXo730ZmBzO2WK3cVxoQBtSaBj2FuRBQrEywt8=","start_time":1724935339263566,"end_time":1724935339263567,"error_msg":"ok","success":false}
{"id":12,"sha256_fingerprint":"aCnhYAwnzcBXBlbvY4pLm7Eciyd6L6lDARtyVgiP8YU=","start_time":1724935339263570,"end_time":1724935339263571,"error_msg":"ok","success":false}
{"id":13,"sha256_fingerprint":"Hc3z577W37+RBMq5llvOFkQTKMOcElhrrYwInrSTPtI=","start_time":1724935339263575,"end_time":1724935339263585,"error_msg":"ok","success":false}
{"id":14,"sha256_fingerprint":"VlVFbWTCIXa937QO2JM0pCG77UvLpphMe7zJfwKtiMg=","start_time":1724935339263589,"end_time":1724935339263591,"error_msg":"ok","success":false}
{"id":15,"sha256_fingerprint":"M+pnf8sj804dtIvcHfOAlgIL2bXxsR9r+SKaqLpAsnI=","start_time":1724935339263594,"end_time":1724935339263596,"error_msg":"ok","success":false}
{"id":16,"sha256_fingerprint":"++5izKbzPOVEi8wETazYUgb9L1WgoXDIB5nQ13lMM6U=","start_time":1724935339263598,"end_time":1724935339263600,"error_msg":"ok","success":false}
{"id":17,"sha256_fingerprint":"sCn3yOlU2+H5rs9jzet1Mv/qwGj7fBfN+pj0IPjcE9Y=","start_time":1724935339263604,"end_time":1724935339263614,"error_msg":"ok","success":false}
@embhorn
Copy link
Member

embhorn commented Aug 29, 2024

Hi @rumpelsepp

Thank you for this excellent report! We enabled a new ASN parser component in between these versions, WOLFSSL_ASN_TEMPLATE. I suspect this is related to the discrepancy in parsing the certs.

I am requesting a review of this behavior by the team.

Thanks,
@embhorn - wolfSSL Support

@SparkiDev
Copy link
Contributor

Hi @rumpelsepp

Taking a look at certificate number 2, I don't see anything in particular that is wrong with it except may be the signature.
OpenSSL parses the certificate fine.

If you don't compile with --enable-ecccustcurves, the explicit curves will cause an error.
Alternatively, compiling with --enable-fastmath and --enable-all turns on --enable-ecccustcurves.
You don't have that in your configuration line above though.

The old code was totally confused by the explicit parameters.
The new code either fails when it sees explicit parameters, as --enable-ecccustcurves is not enabled, or passes as they are parseable.

Is the configuration line above the one you used with 5.5 and 5.7?

Sean

@rumpelsepp
Copy link
Author

Is the configuration line above the one you used with 5.5 and 5.7?

yes, it is.

@SparkiDev
Copy link
Contributor

Hi @rumpelsepp,

With the configure line you specified and a test program that uses the function you gave:
Certificate 1: Failed to load. Has explicit ECC parameters - contains seed but version is 1.
Certificate 2: Loaded. Has explicit ECC parameters that are well-formed.
Certificate 3: Loaded. Has explicit ECC parameters that are well-formed.
Certificate 4: Loaded. Has explicit ECC parameters that are well-formed.
Certificate 5: Loaded. Has explicit ECC parameters that are well-formed.
Certificate 6: Loaded. Has explicit ECC parameters that are well-formed.
Certificate 7: Failed to load. Has explicit ECC parameters - contains seed but version is 1.
Certificate 8: Loaded. Has explicit ECC parameters that are well-formed.
Certificate 9: Loaded. Has explicit ECC parameters that are well-formed.
Certificate 10: Loaded. Has explicit ECC parameters that are well-formed.
Certificate 11: Loaded. Has explicit ECC parameters that are well-formed.
Certificate 12: Loaded. Has explicit ECC parameters that are well-formed.
Certificate 13: Failed to load. Has explicit ECC parameters - contains seed but version is 1.
Certificate 14: Loaded. Has explicit ECC parameters that are well-formed.
Certificate 15: Loaded. Has explicit ECC parameters that are well-formed.
Certificate 16: Loaded. Has explicit ECC parameters that are well-formed.
Certificate 17: Loaded. Has explicit ECC parameters that are well-formed.

The explicit ECC parameters in certificates 1-16 look invalid but the ASN.1 is well formed.
The explicit ECC parameters in certificate 17 looks valid.

Go may not support explicit ECC parameters if it fails on certificate 17.

Do you have further questions on this?

Sean

@SparkiDev
Copy link
Contributor

Hi @rumpelsepp,

Did this information help you?

Thanks,
Sean

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants