On Thu, Mar 08, 2012 at 06:41:47PM +0100, Diego Biurrun wrote: > review needs-fixing > > On Fri, Mar 02, 2012 at 06:08:20PM +0000, Diego Biurrun wrote: > > Diego Biurrun has proposed merging lp:~hipl-core/hipl/certificate-exchange > > into lp:hipl. > > > > --- test/lib/core/cert.c 1970-01-01 00:00:00 +0000 > > +++ test/lib/core/cert.c 2012-03-02 18:07:40 +0000 > > @@ -0,0 +1,189 @@ > > + > > +START_TEST(test_cert_DER_encoding) > > +{ > > + int len = 0; > > + X509 *cert = NULL; > > + X509 *cert_decoded = NULL; > > + unsigned char *buf = NULL; > > + > > + HIP_DEBUG("Test DER en/decoding of X509 certificates.\n"); > > + > > + fail_unless((cert = cert_load_x509_certificate(TEST_CERT, > > + ENCODING_FORMAT_PEM)) > > != NULL, > > + NULL); > > + fail_unless((len = cert_X509_to_DER(cert, &buf)) > 0, NULL); > > + fail_unless((cert_decoded = cert_DER_to_X509(buf, len)) != NULL, NULL); > > + fail_unless(X509_cmp(cert, cert_decoded) == 0, NULL); > > + > > + fail_unless((len = cert_X509_to_DER(NULL, &buf)) < 0, NULL); > > + fail_unless((len = cert_X509_to_DER(cert, NULL)) < 0, NULL); > > + fail_unless((len = cert_X509_to_DER(NULL, NULL)) < 0, NULL); > > + > > + fail_unless((cert_decoded = cert_DER_to_X509(NULL, len)) == NULL, > > NULL); > > + fail_unless((cert_decoded = cert_DER_to_X509(buf, len - 1)) == NULL, > > NULL); > > + fail_unless((cert_decoded = cert_DER_to_X509(buf, len + 1)) == NULL, > > NULL); > > + fail_unless((cert_decoded = cert_DER_to_X509(buf, 0)) == NULL, NULL); > > + > > + X509_free(cert); > > + X509_free(cert_decoded); > > buf does not need to be freed? > > > +START_TEST(test_cert_match_public_key) > > +{ > > + int err = 0; > > + RSA *rsa = NULL; > > + X509 *cert = NULL; > > + EVP_PKEY *pkey = NULL; > > + > > + HIP_DEBUG("Test matching of public keys.\n"); > > + > > + fail_unless((err = load_rsa_private_key(TEST_KEY, &rsa)) == 0, NULL); > > + fail_unless((cert = cert_load_x509_certificate(TEST_CERT, > > + ENCODING_FORMAT_PEM)) > > != NULL, > > + NULL); > > + pkey = EVP_PKEY_new(); > > + EVP_PKEY_assign_RSA(pkey, rsa); > > + fail_unless((err = cert_match_public_key(cert, pkey)) == 1, NULL); > > + > > + fail_unless((err = cert_match_public_key(NULL, pkey)) == 0, NULL); > > + fail_unless((err = cert_match_public_key(cert, NULL)) == 0, NULL); > > + fail_unless((err = cert_match_public_key(NULL, NULL)) == 0, NULL); > > + > > + EVP_PKEY_free(pkey); > > + X509_free(cert); > > RSA_free(rsa); > > .. at least if the doxy of load_rsa_private_key() is to be believed. > > > +START_TEST(test_cert_verify_chain) > > +{ > > + int err = 0; > > + X509 *cert = NULL; > > + STACK_OF(X509) * chain = NULL; > > + > > + HIP_DEBUG("Test verification of certificate chains.\n"); > > + > > + fail_unless((cert = cert_load_x509_certificate(TEST_CERT, > > + ENCODING_FORMAT_PEM)) > > != NULL, > > + NULL); > > + fail_unless((chain = sk_X509_new_null()) != NULL, NULL); > > + sk_X509_push(chain, cert); > > + fail_unless((err = cert_verify_chain(cert, NULL, chain, NULL)) == 0, > > NULL); > > + > > + fail_unless((err = cert_verify_chain(NULL, NULL, chain, NULL)) != 0, > > NULL); > > + fail_unless((err = cert_verify_chain(cert, NULL, NULL, NULL)) != 0, > > NULL); > > + > > + HIP_DEBUG("Successfully passed test for verification of certificate > > chains.\n"); > > +} > > I would guess that cert needs to be freed and chain popped or so - OpenSSL > documentation is nothing but horrific. > > > +START_TEST(test_cert_get_X509_from_msg) > > +{ > > + int len = 0; > > + X509 *cert = NULL, *cert2 = NULL; > > + struct hip_common *msg = NULL; > > + unsigned char *buf = NULL; > > + > > + HIP_DEBUG("Test certificate extraction functionality.\n"); > > + > > + fail_unless((cert = cert_load_x509_certificate(TEST_CERT, > > + ENCODING_FORMAT_PEM)) > > != NULL, > > + NULL); > > + msg = hip_msg_alloc(); > > Memory allocation can fail. > > > + hip_build_network_hdr(msg, HIP_UPDATE, 0, &in6addr_any, &in6addr_any); > > + fail_unless((len = cert_X509_to_DER(cert, &buf)) > 0, NULL); > > + fail_unless(hip_build_param_cert(msg, 0, 1, 1, HIP_CERT_X509V3, buf, > > len) == 0, NULL); > > + fail_unless((cert2 = cert_get_X509_from_msg(msg)) != NULL, NULL); > > + fail_unless(X509_cmp(cert, cert2) == 0, NULL); > > + > > + X509_free(cert); > > + X509_free(cert2); > > What about msg and buf? I fixed all the memory leaks in the unit tests and a few in the code. At least all the ones that valgrind detected at the lowest warning level. Turning up the warning volume drowns me in messages, as I feared... Diego