From d189c74efbd7cba5e71082a63b1d1133c454e6d0 Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Sun, 3 May 2020 18:27:49 -0700 Subject: [PATCH 01/21] Update CI --- .gitlab-ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 3ad5cd800..393628988 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -137,7 +137,7 @@ CentOS6/OpenSSL: Fedora/Coverity: only: refs: - - master + - multiple_auth_feature - coverity script: - dnf update -y @@ -151,7 +151,7 @@ Fedora/Coverity: 'pkgconfig(tss2-esys)' 'pkgconfig(krb5-gssapi)' 'pkgconfig(libp11)' ant - curl -o /tmp/cov-analysis-linux64.tgz https://scan.coverity.com/download/linux64 - --form project=$COVERITY_SCAN_PROJECT_NAME --form token=$COVERITY_SCAN_TOKEN + --form project="$COVERITY_SCAN_PROJECT_NAME" --form token="$COVERITY_SCAN_TOKEN" - tar xfz /tmp/cov-analysis-linux64.tgz - ./autogen.sh - cd java @@ -163,8 +163,8 @@ Fedora/Coverity: - ./configure --with-java --disable-dsa-tests --without-gnutls-version-check - cov-analysis-linux64-*/bin/cov-build --dir cov-int make -j4 - tar cfz cov-int.tar.gz cov-int - - curl https://scan.coverity.com/builds?project=$COVERITY_SCAN_PROJECT_NAME - --form token=$COVERITY_SCAN_TOKEN --form email=$GITLAB_USER_EMAIL + - curl https://scan.coverity.com/builds?project="$COVERITY_SCAN_PROJECT_NAME" + --form token="$COVERITY_SCAN_TOKEN" --form email=$GITLAB_USER_EMAIL --form file=@cov-int.tar.gz --form version="`git describe --tags`" --form description="`git describe --tags` / $CI_COMMIT_TITLE / $CI_COMMIT_REF_NAME:$CI_PIPELINE_ID " tags: -- GitLab From 72dce68d51bb1d21bfba669fa1ed1fbfc78948d1 Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Thu, 23 Apr 2020 23:15:59 -0700 Subject: [PATCH 02/21] Rework load_certificate to prefer abstract privkey Sets up a subsequent commit. Previously, there were two types of keys: x509_privkey and abstract privkey. Deduplicate functionality by preferring abstract privkey. Signed-off-by: Tom Carroll --- gnutls.c | 183 ++++++++++++++++++++++++++++++++----------------------- 1 file changed, 107 insertions(+), 76 deletions(-) diff --git a/gnutls.c b/gnutls.c index 36bc82e06..fc3484389 100644 --- a/gnutls.c +++ b/gnutls.c @@ -442,7 +442,7 @@ static int load_datum(struct openconnect_info *vpninfo, interpreting the file as other types */ #define NOT_PKCS12 1 -static int load_pkcs12_certificate(struct openconnect_info *vpninfo, +static int load_pkcs12_certificate_(struct openconnect_info *vpninfo, gnutls_datum_t *datum, gnutls_x509_privkey_t *key, gnutls_x509_crt_t **chain, @@ -527,6 +527,65 @@ static int load_pkcs12_certificate(struct openconnect_info *vpninfo, return 0; } +static int load_pkcs12_certificate(struct openconnect_info *vpninfo, + gnutls_datum_t *datum, + gnutls_privkey_t *key, + gnutls_x509_crt_t **chain, + unsigned int *chain_len, + gnutls_x509_crt_t **extra_certs, + unsigned int *extra_certs_len, + gnutls_x509_crl_t *crl) +{ + gnutls_x509_privkey_t x509key; + gnutls_privkey_t privkey = NULL; + gnutls_x509_crt_t *crts = NULL, *xcrts = NULL; + unsigned int ncrts = 0, nxcrts = 0; + gnutls_x509_crl_t crevoke = NULL; + unsigned int i; + int gerr, ret; + + ret = load_pkcs12_certificate_(vpninfo, datum, &x509key, + &crts, &ncrts, &xcrts, &nxcrts, &crevoke); + if (ret != 0) + return ret; + + /** + * bind to abstract privkey + */ + gerr = gnutls_privkey_init(&privkey); + if (gerr >= 0) + gerr = gnutls_privkey_import_x509(privkey, x509key, + GNUTLS_PRIVKEY_IMPORT_AUTO_RELEASE); + if (gerr < 0) { + vpn_progress(vpninfo, PRG_ERR, + _("Error initialising private key structure: (%d) %s\n"), + gerr, gnutls_strerror(gerr)); + ret = -EIO; + goto cleanup; + } + x509key = NULL; + + *key = privkey, privkey = NULL; + *chain = crts, crts = NULL; + *chain_len = ncrts, ncrts = 0; + *extra_certs = xcrts, xcrts = NULL; + *extra_certs_len = nxcrts, nxcrts = 0; + *crl = crevoke, crevoke = NULL; + ret = 0; + + cleanup: + gnutls_privkey_deinit(privkey); + gnutls_x509_privkey_deinit(x509key); + for (i = 0; i < ncrts; i++) + gnutls_x509_crt_deinit(crts[i]); + gnutls_free(crts); + for (i = 0; i < nxcrts; i++) + gnutls_x509_crt_deinit(xcrts[i]); + gnutls_free(xcrts); + gnutls_x509_crl_deinit(crevoke); + return ret; +} + static int count_x509_certificates(gnutls_datum_t *datum) { int count = 0; @@ -556,7 +615,6 @@ static int get_cert_name(gnutls_x509_crt_t cert, char *name, size_t namelen) return 0; } -#if defined(HAVE_P11KIT) || defined(HAVE_TROUSERS) || defined(HAVE_TSS2) || defined (HAVE_GNUTLS_SYSTEM_KEYS) /* We have to convert the array of X509 certificates to gnutls_pcert_st for ourselves. There's no function that takes a gnutls_privkey_t as the key and gnutls_x509_crt_t certificates. */ @@ -606,7 +664,6 @@ static int verify_signed_data(gnutls_pubkey_t pubkey, gnutls_privkey_t privkey, return gnutls_pubkey_verify_data2(pubkey, algo, 0, data, sig); } -#endif /* (P11KIT || TROUSERS || TSS2 || SYSTEM_KEYS) */ static int openssl_hash_password(struct openconnect_info *vpninfo, char *pass, gnutls_datum_t *key, gnutls_datum_t *salt) @@ -904,13 +961,11 @@ static void fill_token_info(char *buf, size_t s, unsigned char *dst, size_t dstl static int load_certificate(struct openconnect_info *vpninfo) { - gnutls_datum_t fdata; + gnutls_datum_t fdata = {NULL, 0}; gnutls_x509_privkey_t key = NULL; -#if defined(HAVE_P11KIT) || defined(HAVE_TROUSERS) || defined(HAVE_TSS2) || defined(HAVE_GNUTLS_SYSTEM_KEYS) gnutls_privkey_t pkey = NULL; gnutls_datum_t pkey_sig = {NULL, 0}; void *dummy_hash_data = &load_certificate; -#endif #if defined(HAVE_P11KIT) || defined(HAVE_GNUTLS_SYSTEM_KEYS) char *cert_url = (char *)vpninfo->cert; #endif @@ -929,12 +984,8 @@ static int load_certificate(struct openconnect_info *vpninfo) int i; int cert_is_p11 = 0, key_is_p11 = 0; int cert_is_sys = 0, key_is_sys = 0; - unsigned char key_id[20]; - size_t key_id_size = sizeof(key_id); char name[80]; - fdata.data = NULL; - key_is_p11 = !strncmp(vpninfo->sslkey, "pkcs11:", 7); cert_is_p11 = !strncmp(vpninfo->cert, "pkcs11:", 7); @@ -1037,7 +1088,7 @@ static int load_certificate(struct openconnect_info *vpninfo) /* Is it PKCS#12? */ if (!key_is_p11) { /* PKCS#12 should actually contain certificates *and* private key */ - ret = load_pkcs12_certificate(vpninfo, &fdata, &key, + ret = load_pkcs12_certificate(vpninfo, &fdata, &pkey, &supporting_certs, &nr_supporting_certs, &extra_certs, &nr_extra_certs, &crl); @@ -1055,7 +1106,7 @@ static int load_certificate(struct openconnect_info *vpninfo) goto got_key; } vpn_progress(vpninfo, PRG_ERR, - _("PKCS#11 file contained no certificate\n")); + _("PKCS#12 file contained no certificate\n")); ret = -EINVAL; goto out; } @@ -1431,39 +1482,30 @@ static int load_certificate(struct openconnect_info *vpninfo) vpninfo->cert_password = NULL; } - /* Now attempt to make sure we use the *correct* certificate, to match - the key. Since we have a software key, we can easily query it and - compare its key_id with each certificate till we find a match. */ - err = gnutls_x509_privkey_get_key_id(key, 0, key_id, &key_id_size); - if (err) { + /** + * We have a x509_privkey. Now convert it to an + * abstract privkey. + */ + err = gnutls_privkey_init(&pkey); + if (err >= 0) + err = gnutls_privkey_import_x509(pkey, key, + GNUTLS_PRIVKEY_IMPORT_AUTO_RELEASE); + if (err < 0) { vpn_progress(vpninfo, PRG_ERR, - _("Failed to get key ID: %s\n"), - gnutls_strerror(err)); - ret = -EINVAL; + _("Error initialising private key: (%d) %s\n"), + err, gnutls_strerror(err)); + /** + * out will free pkey + */ + ret = -ENOMEM; goto out; } - /* If extra_certs[] is NULL, we have one candidate in 'cert' to check. */ - for (i = 0; i < (extra_certs ? nr_extra_certs : 1); i++) { - unsigned char cert_id[20]; - size_t cert_id_size = sizeof(cert_id); - - err = gnutls_x509_crt_get_key_id(extra_certs ? extra_certs[i] : cert, 0, cert_id, &cert_id_size); - if (err) - continue; - - if (cert_id_size == key_id_size && !memcmp(cert_id, key_id, key_id_size)) { - if (extra_certs) { - cert = extra_certs[i]; - extra_certs[i] = NULL; - } - goto got_key; - } - } - /* There's no pkey (there's an x509 key), so even if p11-kit or trousers is - enabled we'll fall straight through the bit at match_cert: below, and go - directly to the bit where it prints the 'no match found' error and exits. */ + key = NULL; + /** + * Silence label not used warning + */ + goto match_cert; -#if defined(HAVE_P11KIT) || defined(HAVE_TROUSERS) || defined(HAVE_TSS2) || defined(HAVE_GNUTLS_SYSTEM_KEYS) match_cert: /* If we have a privkey from PKCS#11 or TPM, we can't do the simple comparison of key ID that we do for software keys to find which certificate is a @@ -1518,7 +1560,6 @@ static int load_certificate(struct openconnect_info *vpninfo) gnutls_free(pkey_sig.data); pkey_sig.data = NULL; } -#endif /* P11KIT || TROUSERS || TSS2 || SYSTEM_KEYS */ /* We shouldn't reach this. It means that we didn't find *any* matching cert */ vpn_progress(vpninfo, PRG_ERR, @@ -1684,39 +1725,32 @@ static int load_certificate(struct openconnect_info *vpninfo) key and certs. GnuTLS makes us do this differently for X509 privkeys vs. TPM/PKCS#11 "generic" privkeys, and the latter is particularly 'fun' for GnuTLS 2.12... */ -#if defined(HAVE_P11KIT) || defined(HAVE_TROUSERS) || defined(HAVE_TSS2) || defined(HAVE_GNUTLS_SYSTEM_KEYS) - if (pkey) { #if GNUTLS_VERSION_NUMBER >= 0x030600 - if (gnutls_privkey_get_pk_algorithm(pkey, NULL) == GNUTLS_PK_RSA) { - /* - * For hardware RSA keys, we need to check if they can cope with PSS. - * If not, disable TLSv1.3 which would make PSS mandatory. - * https://bugzilla.redhat.com/show_bug.cgi?id=1663058 - */ - err = gnutls_privkey_sign_data2(pkey, GNUTLS_SIGN_RSA_PSS_RSAE_SHA256, 0, &fdata, &pkey_sig); - if (err) { - vpn_progress(vpninfo, PRG_INFO, - _("Private key appears not to support RSA-PSS. Disabling TLSv1.3\n")); - vpninfo->no_tls13 = 1; - } else { - free(pkey_sig.data); - pkey_sig.data = NULL; - } + if (gnutls_privkey_get_pk_algorithm(pkey, NULL) == GNUTLS_PK_RSA) { + /* + * For hardware RSA keys, we need to check if they can cope with PSS. + * If not, disable TLSv1.3 which would make PSS mandatory. + * https://bugzilla.redhat.com/show_bug.cgi?id=1663058 + */ + err = gnutls_privkey_sign_data2(pkey, GNUTLS_SIGN_RSA_PSS_RSAE_SHA256, 0, &fdata, &pkey_sig); + if (err) { + vpn_progress(vpninfo, PRG_INFO, + _("Private key appears not to support RSA-PSS. Disabling TLSv1.3\n")); + vpninfo->no_tls13 = 1; + } else { + free(pkey_sig.data); + pkey_sig.data = NULL; } + } #endif - err = assign_privkey(vpninfo, pkey, - supporting_certs, - nr_supporting_certs, - free_supporting_certs); - if (!err) { - pkey = NULL; /* we gave it away, and potentially also some - of extra_certs[] may have been zeroed. */ - } - } else -#endif /* P11KIT || TROUSERS || TSS2 || SYSTEM_KEYS */ - err = gnutls_certificate_set_x509_key(vpninfo->https_cred, - supporting_certs, - nr_supporting_certs, key); + err = assign_privkey(vpninfo, pkey, + supporting_certs, + nr_supporting_certs, + free_supporting_certs); + if (!err) { + pkey = NULL; /* we gave it away, and potentially also some + of extra_certs[] may have been zeroed. */ + } if (err) { vpn_progress(vpninfo, PRG_ERR, @@ -1750,13 +1784,10 @@ static int load_certificate(struct openconnect_info *vpninfo) } gnutls_free(extra_certs); -#if defined(HAVE_P11KIT) || defined(HAVE_TROUSERS) || defined(HAVE_TSS2) || defined(HAVE_GNUTLS_SYSTEM_KEYS) - if (pkey) - gnutls_privkey_deinit(pkey); + gnutls_privkey_deinit(pkey); /* If we support arbitrary privkeys, we might have abused fdata.data just to point to something to hash. Don't free it in that case! */ if (fdata.data != dummy_hash_data) -#endif gnutls_free(fdata.data); #ifdef HAVE_P11KIT -- GitLab From 6d32b620d586c99fb5c1912073754179fecaa8cb Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Thu, 23 Apr 2020 23:15:59 -0700 Subject: [PATCH 03/21] Factored out key+cert match mechanism Previously there were two alternates to check that the key matched the certificate. Alternative one was employed if the key was a x509_privkey; alternative two was used if the key was an abstract privkey. By preferring the abstract privkey, the code can be deduplicated and simplified. The adoption was modeled after GnuTLS 3.6.13 _gnutls_check_key_cert_match, which GnuTLS uses to verify that key and cert are a pair. Match was factored out of load_certificate and put into a separate function. Signed-off-by: Tom Carroll --- gnutls.c | 273 +++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 185 insertions(+), 88 deletions(-) diff --git a/gnutls.c b/gnutls.c index fc3484389..c8b5c267c 100644 --- a/gnutls.c +++ b/gnutls.c @@ -654,17 +654,6 @@ static int assign_privkey(struct openconnect_info *vpninfo, return err; } -static int verify_signed_data(gnutls_pubkey_t pubkey, gnutls_privkey_t privkey, - const gnutls_datum_t *data, const gnutls_datum_t *sig) -{ - gnutls_sign_algorithm_t algo; - - algo = gnutls_pk_to_sign(gnutls_privkey_get_pk_algorithm(privkey, NULL), - GNUTLS_DIG_SHA1); - - return gnutls_pubkey_verify_data2(pubkey, algo, 0, data, sig); -} - static int openssl_hash_password(struct openconnect_info *vpninfo, char *pass, gnutls_datum_t *key, gnutls_datum_t *salt) { @@ -959,18 +948,162 @@ static void fill_token_info(char *buf, size_t s, unsigned char *dst, size_t dstl memset(dst + s, ' ', dstlen - s); } +static void free_datum(gnutls_datum_t *dat) +{ + if (dat != NULL) { + gnutls_free(dat->data); + *dat = (gnutls_datum_t) {0}; + } +} + +#define TEST_TEXT "TEST TEXT" + +/** + * returns 1 if the key and cert constitue a keycert pair; otherwise, + * return 0. + * + * signature is an existing SHA-1 signature, computed load_tpm1_key + */ +static unsigned int check_key_cert_match(struct openconnect_info *vpninfo, + gnutls_privkey_t key, gnutls_x509_crt_t cert, + const gnutls_datum_t *signature) +{ + const gnutls_datum_t test = {(void *)TEST_TEXT, sizeof(TEST_TEXT) - 1}; + gnutls_datum_t stack_sig = {0}; + const gnutls_datum_t *sig = signature ? signature : &stack_sig; + gnutls_digest_algorithm_t dig; + gnutls_pubkey_t pub; + unsigned int sign; + int pk, key_pk, err; + + err = gnutls_pubkey_init(&pub); + if (err < 0) + return 0; + + err = gnutls_pubkey_import_x509(pub, cert, 0); + if (err < 0) + goto cleanup; + + pk = gnutls_pubkey_get_pk_algorithm(pub, NULL); + key_pk = gnutls_privkey_get_pk_algorithm(key, NULL); + +#if GNUTLS_VERSION_NUMBER >= 0x030600 + if (pk == GNUTLS_PK_RSA_PSS || key_pk == GNUTLS_PK_RSA_PSS) { + /** Cannot mix an RSA-PSS key with an RSA certificate **/ + if (pk == GNUTLS_PK_RSA) { + vpn_progress(vpninfo, PRG_INFO, + _("Cannot mix an RSA-PSS key with an RSA certificate")); + err = GNUTLS_E_CONSTRAINT_ERROR; + goto cleanup; + } + key_pk = GNUTLS_PK_RSA_PSS; + } +#endif /* GNUTLS_VERSION_NUMBER >= 0x030600 */ + + if (pk != key_pk) { + err = GNUTLS_E_CONSTRAINT_ERROR; + goto cleanup; + } + + /** + * Caller-provider signature uses SHA1 digest. + */ + if (sig->size > 0) { + dig = GNUTLS_DIG_SHA1; + } else { + /** + * Otherwise try SHA256. If that doesn't fly---for example, + * pk == GNUTLS_PK_GOST_01---use the preferred digest. The reason + * we don't default to the preferred digest is because of preceived + * performance costs: preferred digest is a function of key size. + * Larger digests are not necessary to demonstrate the relationship + * between key and cert. + */ + err = 0; + dig = GNUTLS_DIG_SHA256; + if (gnutls_pk_to_sign(pk, dig) == GNUTLS_SIGN_UNKNOWN) + err = gnutls_pubkey_get_preferred_hash_algorithm(pub, &dig, NULL); + if (err < 0) + goto cleanup; + } + + sign = gnutls_pk_to_sign(pk, dig); + + /* now check if keys really match. We use the sign/verify approach + * because we cannot always obtain the parameters from the abstract + * keys (e.g. PKCS #11). */ + + /** + * Compute signature if not provided one + */ + if (sig->size == 0) { +#if GNUTLS_VERSION_NUMBER >= 0x030600 + /** + * A PSS signature is generated when key is GNUTLS_PK_RSA + * and certificate is GNUTLS_PK_RSA_PSS. + */ + err = gnutls_privkey_sign_data2(key, sign, 0, &test, &stack_sig); +#else /* GNUTLS_VERSION_NUMBER < 0x030600 */ + err = gnutls_privkey_sign_data(key, dig, 0, &test, &stack_sig); +#endif + if (err < 0) { + vpn_progress(vpninfo, PRG_ERR, + _("Error signing test data with private key: (%d) %s\n"), + err, gnutls_strerror(err)); + vpn_progress(vpninfo, PRG_DEBUG, + _("error signing test data: " + "key is %s, certificate is %s, digest is %s, " + "and signing algorithm is %s.\n"), + gnutls_pk_get_name(key_pk), gnutls_pk_get_name(pk), + gnutls_digest_get_name(dig), + gnutls_sign_algorithm_get_name(sign)); + goto cleanup; + } + /** + * sig may reference either signature or &stack_sig. + */ + sig = &stack_sig; + } + + /** + * Verify the signature. If err == 0, then key and cert + * constitute a keycert pair; otherwise, they do not. + */ +#ifndef GNUTLS_VERIFY_ALLOW_BROKEN +# define GNUTLS_VERIFY_ALLOW_BROKEN 0 +#endif /* !GNUTLS_VERIFY_ALLOW_BROKEN */ + err = gnutls_pubkey_verify_data2(pub, sign, + GNUTLS_VERIFY_ALLOW_BROKEN, &test, sig); + + free_datum(&stack_sig); + + if (err < 0 && err != GNUTLS_E_PK_SIG_VERIFY_FAILED) { + vpn_progress(vpninfo, PRG_ERR, + _("Error validating signature: (%d) %s\n"), + err, gnutls_strerror(err)); + vpn_progress(vpninfo, PRG_DEBUG, + _("error verifying test signature: " + "key is %s, certificate is %s, digest is %s, " + "and signing algorithm is %s.\n"), + gnutls_pk_get_name(key_pk), gnutls_pk_get_name(pk), + gnutls_digest_get_name(dig), + gnutls_sign_algorithm_get_name(sign)); + } + + cleanup: + gnutls_pubkey_deinit(pub); + return err >= 0; +} + static int load_certificate(struct openconnect_info *vpninfo) { gnutls_datum_t fdata = {NULL, 0}; gnutls_x509_privkey_t key = NULL; gnutls_privkey_t pkey = NULL; gnutls_datum_t pkey_sig = {NULL, 0}; - void *dummy_hash_data = &load_certificate; -#if defined(HAVE_P11KIT) || defined(HAVE_GNUTLS_SYSTEM_KEYS) char *cert_url = (char *)vpninfo->cert; -#endif -#ifdef HAVE_P11KIT char *key_url = (char *)vpninfo->sslkey; +#ifdef HAVE_P11KIT gnutls_pkcs11_privkey_t p11key = NULL; #endif char *pem_header; @@ -1506,59 +1639,21 @@ static int load_certificate(struct openconnect_info *vpninfo) */ goto match_cert; + /* If extra_certs[] is NULL, we have one candidate in 'cert' to check. */ match_cert: - /* If we have a privkey from PKCS#11 or TPM, we can't do the simple comparison - of key ID that we do for software keys to find which certificate is a - match. So sign some dummy data and then check the signature against each - of the available certificates until we find the right one. */ - if (pkey) { - /* The TPM code may have already signed it, to test authorisation. We - only sign here for PKCS#11 keys, in which case fdata might be - empty too so point it at dummy data. */ - if (!pkey_sig.data) { - if (!fdata.data) { - fdata.data = dummy_hash_data; - fdata.size = 20; - } - - err = gnutls_privkey_sign_data(pkey, GNUTLS_DIG_SHA1, 0, &fdata, &pkey_sig); - if (err) { - vpn_progress(vpninfo, PRG_ERR, - _("Error signing test data with private key: %s\n"), - gnutls_strerror(err)); - ret = -EINVAL; - goto out; + for (i = 0; i < (extra_certs ? nr_extra_certs : 1); i++) { + int match; + + match = check_key_cert_match(vpninfo, pkey, + extra_certs ? extra_certs[i] : cert, &pkey_sig); + if (match > 0) { + if (extra_certs) { + cert = extra_certs[i]; + extra_certs[i] = NULL; } + free_datum(&pkey_sig); + goto got_key; } - - /* If extra_certs[] is NULL, we have one candidate in 'cert' to check. */ - for (i = 0; i < (extra_certs ? nr_extra_certs : 1); i++) { - gnutls_pubkey_t pubkey; - - gnutls_pubkey_init(&pubkey); - err = gnutls_pubkey_import_x509(pubkey, extra_certs ? extra_certs[i] : cert, 0); - if (err) { - vpn_progress(vpninfo, PRG_ERR, - _("Error validating signature against certificate: %s\n"), - gnutls_strerror(err)); - /* We'll probably fail shortly if we don't find it. */ - gnutls_pubkey_deinit(pubkey); - continue; - } - err = verify_signed_data(pubkey, pkey, &fdata, &pkey_sig); - gnutls_pubkey_deinit(pubkey); - - if (err >= 0) { - if (extra_certs) { - cert = extra_certs[i]; - extra_certs[i] = NULL; - } - gnutls_free(pkey_sig.data); - goto got_key; - } - } - gnutls_free(pkey_sig.data); - pkey_sig.data = NULL; } /* We shouldn't reach this. It means that we didn't find *any* matching cert */ @@ -1732,7 +1827,12 @@ static int load_certificate(struct openconnect_info *vpninfo) * If not, disable TLSv1.3 which would make PSS mandatory. * https://bugzilla.redhat.com/show_bug.cgi?id=1663058 */ - err = gnutls_privkey_sign_data2(pkey, GNUTLS_SIGN_RSA_PSS_RSAE_SHA256, 0, &fdata, &pkey_sig); + + /** + * Change made to simplify factoring out tls13 test routine. + */ + const gnutls_datum_t text = {(void *)TEST_TEXT, sizeof(TEST_TEXT)-1}; + err = gnutls_privkey_sign_data2(pkey, GNUTLS_SIGN_RSA_PSS_RSAE_SHA256, 0, &text, &pkey_sig); if (err) { vpn_progress(vpninfo, PRG_INFO, _("Private key appears not to support RSA-PSS. Disabling TLSv1.3\n")); @@ -1740,30 +1840,32 @@ static int load_certificate(struct openconnect_info *vpninfo) } else { free(pkey_sig.data); pkey_sig.data = NULL; - } } #endif - err = assign_privkey(vpninfo, pkey, - supporting_certs, - nr_supporting_certs, - free_supporting_certs); - if (!err) { - pkey = NULL; /* we gave it away, and potentially also some - of extra_certs[] may have been zeroed. */ - } + /* OK, now we've checked the cert expiry and warned the user if it's + going to expire soon, and we've built up as much of a trust chain + in supporting_certs[] as we can find, to help the server work around + OpenSSL RT#1942. Set up the GnuTLS credentials with the appropriate + key and certs. + */ + err = assign_privkey(vpninfo, pkey, supporting_certs, + nr_supporting_certs, free_supporting_certs); if (err) { vpn_progress(vpninfo, PRG_ERR, _("Setting certificate failed: %s\n"), gnutls_strerror(err)); ret = -EIO; - } else - ret = 0; + goto out; + } + + pkey = NULL; /* we gave it away, and potentially also some + of extra_certs[] may have been zeroed. */ + ret = 0; + out: - if (crl) - gnutls_x509_crl_deinit(crl); - if (key) - gnutls_x509_privkey_deinit(key); + gnutls_x509_crl_deinit(crl); + gnutls_x509_privkey_deinit(key); if (supporting_certs) { for (i = 0; i < nr_supporting_certs; i++) { /* We get here in an error case with !free_supporting_certs @@ -1787,17 +1889,12 @@ static int load_certificate(struct openconnect_info *vpninfo) gnutls_privkey_deinit(pkey); /* If we support arbitrary privkeys, we might have abused fdata.data just to point to something to hash. Don't free it in that case! */ - if (fdata.data != dummy_hash_data) - gnutls_free(fdata.data); - -#ifdef HAVE_P11KIT - /* This exists in the HAVE_GNUTLS_SYSTEM_KEYS case but will never - change so it's OK not to add to the #ifdef mess here. */ + free_datum(&fdata); + free_datum(&pkey_sig); if (cert_url != vpninfo->cert) free(cert_url); if (key_url != vpninfo->sslkey) free(key_url); -#endif return ret; } -- GitLab From 8eb2c734a739401dfb4d4763cdd0980a280a28fa Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Thu, 23 Apr 2020 23:15:59 -0700 Subject: [PATCH 04/21] Copy the certificate Making a copy of the certificate simplifies the logic. It sets us up so that a certificate chain can be returned. Signed-off-by: Tom Carroll --- gnutls.c | 69 +++++++++++--------------------------------------------- 1 file changed, 13 insertions(+), 56 deletions(-) diff --git a/gnutls.c b/gnutls.c index c8b5c267c..df30b7307 100644 --- a/gnutls.c +++ b/gnutls.c @@ -621,8 +621,7 @@ static int get_cert_name(gnutls_x509_crt_t cert, char *name, size_t namelen) static int assign_privkey(struct openconnect_info *vpninfo, gnutls_privkey_t pkey, gnutls_x509_crt_t *certs, - unsigned int nr_certs, - uint8_t *free_certs) + unsigned int nr_certs) { gnutls_pcert_st *pcerts = calloc(nr_certs, sizeof(*pcerts)); int i, err; @@ -1111,10 +1110,9 @@ static int load_certificate(struct openconnect_info *vpninfo) gnutls_x509_crt_t last_cert, cert = NULL; gnutls_x509_crt_t *extra_certs = NULL, *supporting_certs = NULL; unsigned int nr_supporting_certs = 0, nr_extra_certs = 0; - uint8_t *free_supporting_certs = NULL; int err; /* GnuTLS error */ int ret; - int i; + unsigned int i; int cert_is_p11 = 0, key_is_p11 = 0; int cert_is_sys = 0, key_is_sys = 0; char name[80]; @@ -1230,12 +1228,6 @@ static int load_certificate(struct openconnect_info *vpninfo) else if (!ret) { if (nr_supporting_certs) { cert = supporting_certs[0]; - free_supporting_certs = gnutls_malloc(nr_supporting_certs); - if (!free_supporting_certs) { - ret = -ENOMEM; - goto out; - } - memset(free_supporting_certs, 1, nr_supporting_certs); goto got_key; } vpn_progress(vpninfo, PRG_ERR, @@ -1705,21 +1697,12 @@ static int load_certificate(struct openconnect_info *vpninfo) supporting_certs[0] = cert; nr_supporting_certs = 1; - free_supporting_certs = gnutls_malloc(1); - if (!free_supporting_certs) { - vpn_progress(vpninfo, PRG_ERR, - _("Failed to allocate memory for certificate\n")); - ret = -ENOMEM; - goto out; - } - free_supporting_certs[0] = 1; } last_cert = supporting_certs[nr_supporting_certs-1]; while (1) { - uint8_t free_issuer; - gnutls_x509_crt_t issuer; - void *tmp; + gnutls_x509_crt_t issuer = NULL; + gnutls_x509_crt_t *clist = NULL; for (i = 0; i < nr_extra_certs; i++) { if (extra_certs[i] && @@ -1731,15 +1714,13 @@ static int load_certificate(struct openconnect_info *vpninfo) /* We found the next cert in the chain in extra_certs[] */ issuer = extra_certs[i]; extra_certs[i] = NULL; - free_issuer = 1; } else { /* Look for it in the system trust cafile too. */ err = gnutls_certificate_get_issuer(vpninfo->https_cred, - last_cert, &issuer, 0); - free_issuer = 0; - + last_cert, &issuer, GNUTLS_TL_GET_COPY); #ifdef HAVE_P11KIT - if (err && cert_is_p11) { + if (err == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE + && cert_is_p11) { gnutls_datum_t t; err = gnutls_pkcs11_get_raw_issuer(cert_url, last_cert, &t, GNUTLS_X509_FMT_DER, 0); @@ -1749,8 +1730,6 @@ static int load_certificate(struct openconnect_info *vpninfo) err = gnutls_x509_crt_import(issuer, &t, GNUTLS_X509_FMT_DER); if (err) gnutls_x509_crt_deinit(issuer); - else - free_issuer = 1; } gnutls_free(t.data); } @@ -1774,35 +1753,24 @@ static int load_certificate(struct openconnect_info *vpninfo) /* Don't actually include the root CA. If they don't already trust it, then handing it to them isn't going to help. But don't omit the original certificate if it's self-signed. */ - if (free_issuer) - gnutls_x509_crt_deinit(issuer); + gnutls_x509_crt_deinit(issuer); break; } /* OK, we found a new cert to add to our chain. */ - tmp = supporting_certs; + clist = supporting_certs; supporting_certs = gnutls_realloc(supporting_certs, sizeof(cert) * (nr_supporting_certs+1)); if (!supporting_certs) { - supporting_certs = tmp; - realloc_failed: + supporting_certs = clist; vpn_progress(vpninfo, PRG_ERR, _("Failed to allocate memory for supporting certificates\n")); - if (free_issuer) - gnutls_x509_crt_deinit(issuer); + gnutls_x509_crt_deinit(issuer); break; } - tmp = free_supporting_certs; - free_supporting_certs = gnutls_realloc(free_supporting_certs, nr_supporting_certs+1); - if (!free_supporting_certs) { - free_supporting_certs = tmp; - goto realloc_failed; - } - /* Append the new one */ supporting_certs[nr_supporting_certs] = issuer; - free_supporting_certs[nr_supporting_certs] = free_issuer; nr_supporting_certs++; last_cert = issuer; } @@ -1813,13 +1781,6 @@ static int load_certificate(struct openconnect_info *vpninfo) _("Adding supporting CA '%s'\n"), name); } - /* OK, now we've checked the cert expiry and warned the user if it's - going to expire soon, and we've built up as much of a trust chain - in supporting_certs[] as we can find, to help the server work around - OpenSSL RT#1942. Set up the GnuTLS credentials with the appropriate - key and certs. GnuTLS makes us do this differently for X509 privkeys - vs. TPM/PKCS#11 "generic" privkeys, and the latter is particularly - 'fun' for GnuTLS 2.12... */ #if GNUTLS_VERSION_NUMBER >= 0x030600 if (gnutls_privkey_get_pk_algorithm(pkey, NULL) == GNUTLS_PK_RSA) { /* @@ -1850,7 +1811,7 @@ static int load_certificate(struct openconnect_info *vpninfo) key and certs. */ err = assign_privkey(vpninfo, pkey, supporting_certs, - nr_supporting_certs, free_supporting_certs); + nr_supporting_certs); if (err) { vpn_progress(vpninfo, PRG_ERR, _("Setting certificate failed: %s\n"), @@ -1868,13 +1829,9 @@ static int load_certificate(struct openconnect_info *vpninfo) gnutls_x509_privkey_deinit(key); if (supporting_certs) { for (i = 0; i < nr_supporting_certs; i++) { - /* We get here in an error case with !free_supporting_certs - and should free them all in that case */ - if (!free_supporting_certs || free_supporting_certs[i]) - gnutls_x509_crt_deinit(supporting_certs[i]); + gnutls_x509_crt_deinit(supporting_certs[i]); } gnutls_free(supporting_certs); - gnutls_free(free_supporting_certs); } else if (cert) { /* Not if supporting_certs. It's supporting_certs[0] then and was already freed. */ -- GitLab From 1e1ba280c4d3495d5f09dd9dec3ae3a8fe89aee7 Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Thu, 23 Apr 2020 23:15:59 -0700 Subject: [PATCH 05/21] Distinguish out of memory from insufficient creds Signed-off-by: Tom Carroll --- gnutls.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/gnutls.c b/gnutls.c index df30b7307..111f3746e 100644 --- a/gnutls.c +++ b/gnutls.c @@ -624,9 +624,17 @@ static int assign_privkey(struct openconnect_info *vpninfo, unsigned int nr_certs) { gnutls_pcert_st *pcerts = calloc(nr_certs, sizeof(*pcerts)); - int i, err; + unsigned int i; + int err; - if (!pcerts) + /** + * Added check for nr_certs > 0 to allow the caller to + * distinguish between out of memory (signaled by + * GNUTLS_E_MEMORY_ERROR) and when either pkey == NULL or + * nr_certs == 0. In these cases, GNUTLS_E_INSUFFICIENT_CREDENTIALS + * is signaled. + */ + if (nr_certs > 0 && pcerts == NULL) return GNUTLS_E_MEMORY_ERROR; for (i = 0 ; i < nr_certs; i++) { -- GitLab From f2df65bc9384684e368d4ac4187d15d6c661dbb0 Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Thu, 23 Apr 2020 23:15:59 -0700 Subject: [PATCH 06/21] Factor out trust and cafile loading Simplifies subsequent load_keycert rework. Previously, a separate path for android existed. Noted that it duplicated much of the existing functionality of load_datum and cafile. Functionality was factored into a separate function, removing duplicative code path for Android. Signed-off-by: Tom Carroll --- gnutls.c | 162 +++++++++++++++++++++++++++++++------------------------ 1 file changed, 93 insertions(+), 69 deletions(-) diff --git a/gnutls.c b/gnutls.c index 111f3746e..b8638757b 100644 --- a/gnutls.c +++ b/gnutls.c @@ -1102,6 +1102,89 @@ static unsigned int check_key_cert_match(struct openconnect_info *vpninfo, return err >= 0; } +static int load_trust(struct openconnect_info *vpninfo) +{ + gnutls_x509_trust_list_t tlist; + int err, ret; + + err = gnutls_x509_trust_list_init(&tlist, 0); + if (err < 0) { + vpn_progress(vpninfo, PRG_ERR, + _("Failed to initialize trusts: (%d) %s\n"), + err, gnutls_strerror(err)); + return -ENOMEM; + } + + if (!vpninfo->no_system_trust) { + err = gnutls_x509_trust_list_add_system_trust(tlist, + GNUTLS_TL_NO_DUPLICATES|GNUTLS_TL_USE_IN_TLS, 0); + if (err < 0) { + vpn_progress(vpninfo, PRG_ERR, + _("Error loading system trust: (%d) %s\n"), + err, gnutls_strerror(err)); + ret = -EIO; + goto cleanup; + } else { + vpn_progress(vpninfo, PRG_INFO, + _("Loaded system trust (%d CAs available)\n"), + err); + } + } + + if (vpninfo->cafile) { + if (gnutls_url_is_supported(vpninfo->cafile)) { + /** + * Handle pkcs11: and other GnuTLS URLS + */ + err = gnutls_x509_trust_list_add_trust_file(tlist, + vpninfo->cafile, NULL, GNUTLS_X509_FMT_PEM, + GNUTLS_TL_NO_DUPLICATES|GNUTLS_TL_USE_IN_TLS, + 0); + } else { + /** + * Handle files and ANDROID_KEYSTORE + */ + gnutls_datum_t tmp = {NULL, 0}; + + ret = load_datum(vpninfo, &tmp, vpninfo->cafile); + if (ret < 0) + goto cleanup; + + err = gnutls_x509_trust_list_add_trust_mem(tlist, &tmp, NULL, + GNUTLS_X509_FMT_PEM, + GNUTLS_TL_NO_DUPLICATES|GNUTLS_TL_USE_IN_TLS, + 0); + + gnutls_free(tmp.data); + } + + if (err == GNUTLS_E_NO_CERTIFICATE_FOUND) + err = 0; + + if (err < 0) { + vpn_progress(vpninfo, PRG_ERR, + _("Error loading cafile: (%d) %s\n"), + err, gnutls_strerror(err)); + ret = -EINVAL; + goto cleanup; + } else { + vpn_progress(vpninfo, PRG_INFO, + _("Loaded cafile (%d CAs available)\n"), + err); + } + } + + /** + * set_trust_list takes ownership of tlist + */ + gnutls_certificate_set_trust_list(vpninfo->https_cred, tlist, 0); + return 0; + + cleanup: + gnutls_x509_trust_list_deinit(tlist, 1); + return ret; +} + static int load_certificate(struct openconnect_info *vpninfo) { gnutls_datum_t fdata = {NULL, 0}; @@ -2164,79 +2247,20 @@ int openconnect_open_https(struct openconnect_info *vpninfo) if (!vpninfo->https_cred) { gnutls_certificate_allocate_credentials(&vpninfo->https_cred); - if (!vpninfo->no_system_trust) - gnutls_certificate_set_x509_system_trust(vpninfo->https_cred); + + err = load_trust(vpninfo); + if (err < 0) { + vpn_progress(vpninfo, PRG_ERR, + _("Failed loading trusts. Aborting.\n")); + gnutls_certificate_free_credentials(vpninfo->https_cred); + vpninfo->https_cred = NULL; + closesocket(ssl_sock); + return err; + } gnutls_certificate_set_verify_function(vpninfo->https_cred, verify_peer); -#ifdef ANDROID_KEYSTORE - if (vpninfo->cafile && !strncmp(vpninfo->cafile, "keystore:", 9)) { - gnutls_datum_t datum; - unsigned int nr_certs; - - err = load_datum(vpninfo, &datum, vpninfo->cafile); - if (err < 0) { - gnutls_certificate_free_credentials(vpninfo->https_cred); - vpninfo->https_cred = NULL; - return err; - } - - /* For GnuTLS 3.x We should use gnutls_x509_crt_list_import2() */ - nr_certs = count_x509_certificates(&datum); - if (nr_certs) { - gnutls_x509_crt_t *certs; - int i; - - certs = calloc(nr_certs, sizeof(*certs)); - if (!certs) { - vpn_progress(vpninfo, PRG_ERR, - _("Failed to allocate memory for cafile certs\n")); - gnutls_free(datum.data); - gnutls_certificate_free_credentials(vpninfo->https_cred); - vpninfo->https_cred = NULL; - closesocket(ssl_sock); - return -ENOMEM; - } - err = gnutls_x509_crt_list_import(certs, &nr_certs, &datum, - GNUTLS_X509_FMT_PEM, 0); - gnutls_free(datum.data); - if (err >= 0) { - nr_certs = err; - err = gnutls_certificate_set_x509_trust(vpninfo->https_cred, - certs, nr_certs); - } - for (i = 0; i < nr_certs; i++) - gnutls_x509_crt_deinit(certs[i]); - free(certs); - if (err < 0) { - /* From crt_list_import or set_x509_trust */ - vpn_progress(vpninfo, PRG_ERR, - _("Failed to read certs from cafile: %s\n"), - gnutls_strerror(err)); - gnutls_certificate_free_credentials(vpninfo->https_cred); - vpninfo->https_cred = NULL; - closesocket(ssl_sock); - return -EINVAL; - } - } - } else -#endif - if (vpninfo->cafile) { - err = gnutls_certificate_set_x509_trust_file(vpninfo->https_cred, - vpninfo->cafile, - GNUTLS_X509_FMT_PEM); - if (err < 0) { - vpn_progress(vpninfo, PRG_ERR, - _("Failed to open CA file '%s': %s\n"), - vpninfo->cafile, gnutls_strerror(err)); - gnutls_certificate_free_credentials(vpninfo->https_cred); - vpninfo->https_cred = NULL; - closesocket(ssl_sock); - return -EINVAL; - } - } - if (vpninfo->cert) { err = load_certificate(vpninfo); if (err) { -- GitLab From 8205a00132c6e977edaefb0a54a42ac8ca59403e Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Thu, 23 Apr 2020 23:15:59 -0700 Subject: [PATCH 07/21] Factor out tls13 checks Simplifies subsequent load_keycert work. Signed-off-by: Tom Carroll --- gnutls.c | 56 ++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/gnutls.c b/gnutls.c index b8638757b..3b8559193 100644 --- a/gnutls.c +++ b/gnutls.c @@ -1185,6 +1185,39 @@ static int load_trust(struct openconnect_info *vpninfo) return ret; } +static void check_tls13(struct openconnect_info *vpninfo, + gnutls_privkey_t key) +{ + int pk = gnutls_privkey_get_pk_algorithm(key, NULL); + +#if GNUTLS_VERSION_NUMBER >= 0x030600 + /** + * For hardware RSA keys, we need to check if they can cope with PSS. + * If not, disable TLSv1.3 which would make PSS mandatory. + * https://bugzilla.redhat.com/show_bug.cgi?id=1663058 + */ + if (pk == GNUTLS_PK_RSA) { + gnutls_datum_t data = {(void *)TEST_TEXT, sizeof TEST_TEXT-1}; + gnutls_datum_t signature = {NULL, 0}; + int gerr; + + gerr = gnutls_privkey_sign_data2(key, + GNUTLS_SIGN_RSA_PSS_RSAE_SHA256, 0, &data, &signature); + free_datum(&signature); + if (gerr < 0) { + vpninfo->no_tls13 = 1; + vpn_progress(vpninfo, PRG_INFO, + _("Private key appears not to support RSA-PSS." + " Disabling TLSv1.3\n")); + } + } +#endif + /* hush not used warnings */ + (void) pk; + (void) vpninfo; + (void) key; +} + static int load_certificate(struct openconnect_info *vpninfo) { gnutls_datum_t fdata = {NULL, 0}; @@ -1872,28 +1905,7 @@ static int load_certificate(struct openconnect_info *vpninfo) _("Adding supporting CA '%s'\n"), name); } -#if GNUTLS_VERSION_NUMBER >= 0x030600 - if (gnutls_privkey_get_pk_algorithm(pkey, NULL) == GNUTLS_PK_RSA) { - /* - * For hardware RSA keys, we need to check if they can cope with PSS. - * If not, disable TLSv1.3 which would make PSS mandatory. - * https://bugzilla.redhat.com/show_bug.cgi?id=1663058 - */ - - /** - * Change made to simplify factoring out tls13 test routine. - */ - const gnutls_datum_t text = {(void *)TEST_TEXT, sizeof(TEST_TEXT)-1}; - err = gnutls_privkey_sign_data2(pkey, GNUTLS_SIGN_RSA_PSS_RSAE_SHA256, 0, &text, &pkey_sig); - if (err) { - vpn_progress(vpninfo, PRG_INFO, - _("Private key appears not to support RSA-PSS. Disabling TLSv1.3\n")); - vpninfo->no_tls13 = 1; - } else { - free(pkey_sig.data); - pkey_sig.data = NULL; - } -#endif + check_tls13(vpninfo, pkey); /* OK, now we've checked the cert expiry and warned the user if it's going to expire soon, and we've built up as much of a trust chain -- GitLab From 96130b9f75d8d694970df5b1b11f41adc82967ce Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Thu, 23 Apr 2020 23:15:59 -0700 Subject: [PATCH 08/21] Check gnutls_init return status Check if gnutls_init has signaled error. If error, then unwind, freeing credentials and other resources. Return error to caller. Signed-off-by: Tom Carroll --- gnutls.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/gnutls.c b/gnutls.c index 3b8559193..385dade5f 100644 --- a/gnutls.c +++ b/gnutls.c @@ -2285,8 +2285,22 @@ int openconnect_open_https(struct openconnect_info *vpninfo) } } } - gnutls_init(&vpninfo->https_sess, GNUTLS_CLIENT|GNUTLS_FORCE_CLIENT_CERT); - gnutls_session_set_ptr(vpninfo->https_sess, (void *) vpninfo); + +/** + * Should we disable the TLS ticket extension since we are not resuming + * sessions? + */ + err = gnutls_init(&vpninfo->https_sess, GNUTLS_CLIENT|GNUTLS_FORCE_CLIENT_CERT); + if (err < 0) { + vpn_progress(vpninfo, PRG_ERR, + _("Failed initializing session: (%d) %s\n"), + err, gnutls_strerror(err)); + gnutls_certificate_free_credentials(vpninfo->https_cred); + vpninfo->https_cred = NULL; + closesocket(ssl_sock); + return -ENOMEM; + } + gnutls_session_set_ptr(vpninfo->https_sess, vpninfo); /* * For versions of GnuTLS older than 3.2.9, we try to avoid long * packets by silently disabling extensions such as SNI. -- GitLab From 6ff1b5148d87ffee89030591ee9497e851164270 Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Tue, 5 May 2020 22:57:48 -0700 Subject: [PATCH 09/21] A simple referenced-counted string An implementation of a simple reference-counted string. The purpose of this is to simplify password password and handling, and reduce the number of password copies. Signed-off-by: Tom Carroll --- gnutls.c | 39 ++++++++++++++++++++++++++++++++ gnutls.h | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/gnutls.c b/gnutls.c index 385dade5f..e0cedbc6f 100644 --- a/gnutls.c +++ b/gnutls.c @@ -2773,3 +2773,42 @@ void destroy_eap_ttls(struct openconnect_info *vpninfo, void *sess) { gnutls_deinit(sess); } + +const rcstring *rcstring_new_len(const char *s, size_t len) +{ + struct rcstring_st *rcstr; + size_t rcstr_size; + + rcstr_size = offsetof(struct rcstring_st, str.data) + len + 1; + if (rcstr_size <= len) return NULL; + rcstr = malloc(rcstr_size); + if (rcstr == NULL) return NULL; + rcstr->refcount = 1; + rcstr->str.len = len; + memcpy(rcstr->str.data, s, len); + rcstr->str.data[len] = 0; + return rcstr->str.data; +} + +const rcstring *rcstring_new(const char *s) +{ + return rcstring_new_len(s, strlen(s)); +} + +void rcstring_release_zero(const rcstring *str, void (*zero_func)(char *, size_t)) +{ + struct rcstring_st *rcstr; + + if (str == NULL) return; + rcstr = RCSTRING(str); + if (--rcstr->refcount <= 0) { + if (zero_func != NULL) + (*zero_func)(rcstr->str.data, rcstr->str.len); + free(rcstr); + } +} + +void zero_password(char *s, size_t n) +{ + clear_mem(s, n); +} diff --git a/gnutls.h b/gnutls.h index 0e7a8176c..edaf58122 100644 --- a/gnutls.h +++ b/gnutls.h @@ -72,4 +72,73 @@ char *get_gnutls_cipher(gnutls_session_t session); #define gnutls_check_version_numeric gtls_ver #endif +/** + * rcstring is a simple referenced counted string. This was added to + * simplify password passing and to reduce the number of times the + * password needs to be copied. + */ + +typedef char rcstring; + +struct rcstring_st { + int refcount; + struct { + size_t len; + char data[1]; + } str; +}; + +#define RCSTRING(p) \ + ((struct rcstring_st *)((char *)(p) - offsetof(struct rcstring_st, str. data))) + +const rcstring *rcstring_new_len(const char *s, size_t n); + +const rcstring *rcstring_new(const char *s); + +/** + * Acquire a reference to string + */ +static inline const char * +rcstring_acquire(const rcstring *str) +{ + if (str == NULL) return NULL; + ++RCSTRING(str)->refcount; + return str; +} +/** + * Release a reference to the string + */ +void rcstring_release_zero(const rcstring *str, + void (*zero_func)(char *,size_t)); + +static inline void +rcstring_release(const rcstring *str) +{ + return rcstring_release_zero(str, NULL); +} +/** + * Length of string + */ +static inline size_t +rcstring_length(const rcstring *str) +{ + if (str == NULL) return 0; + return RCSTRING(str)->str.len; +} +/** + * passwords are zeroed when the refcount <= 0 + */ +#define password_new(s) rcstring_new(s) + +#define password_acquire(s) rcstring_acquire(s) + +void zero_password(char *s, size_t n); + +static inline void +password_free(const rcstring **password) +{ + rcstring_release_zero(*password, zero_password); + *password = NULL; +} + #endif /* __OPENCONNECT_GNUTLS_H__ */ -- GitLab From fd1450c084a8478a74ca5890a9c9853897f8f702 Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Thu, 23 Apr 2020 23:15:59 -0700 Subject: [PATCH 10/21] Factor out keycert loading The objective is to have load_keycert fetch credentials for both establishing the TLS session and responding to the multicert auth challenge. The main changes are: load_keycert that is parameterized by key_path, cert_path, and password; have secondary functions parameterized by password; load_certificate stub to call load_keycert and match caller impedance, including maintaining gnutls_certificate_credentials and gnutls_session dependencies. Signed-off-by: Tom Carroll --- gnutls.c | 376 ++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 247 insertions(+), 129 deletions(-) diff --git a/gnutls.c b/gnutls.c index e0cedbc6f..076654e4a 100644 --- a/gnutls.c +++ b/gnutls.c @@ -40,7 +40,7 @@ #endif #if defined(HAVE_P11KIT) || defined(HAVE_GNUTLS_SYSTEM_KEYS) -static int gnutls_pin_callback(void *priv, int attempt, const char *uri, +static int pin_callback(void *user, int attempt, const char *token_uri, const char *token_label, unsigned int flags, char *pin, size_t pin_max); #endif /* HAVE_P11KIT || HAVE_GNUTLS_SYSTEM_KEYS */ @@ -444,6 +444,7 @@ static int load_datum(struct openconnect_info *vpninfo, static int load_pkcs12_certificate_(struct openconnect_info *vpninfo, gnutls_datum_t *datum, + const char *password, gnutls_x509_privkey_t *key, gnutls_x509_crt_t **chain, unsigned int *chain_len, @@ -469,7 +470,16 @@ static int load_pkcs12_certificate_(struct openconnect_info *vpninfo, return NOT_PKCS12; } - pass = vpninfo->cert_password; + pass = NULL; + if (password && (pass = strdup(password)) == NULL) { + err = GNUTLS_E_MEMORY_ERROR; + goto error; + } + /** + * A check of pass == password determines if this is the first + * attempt. + */ + password = pass; while ((err = gnutls_pkcs12_verify_mac(p12, pass)) == GNUTLS_E_MAC_VERIFY_FAILED) { if (!pass) { /* OpenSSL's PKCS12_parse() code will try both NULL and "" automatically, @@ -483,7 +493,7 @@ static int load_pkcs12_certificate_(struct openconnect_info *vpninfo, vpn_progress(vpninfo, PRG_ERR, _("Failed to decrypt PKCS#12 certificate file\n")); free_pass(&pass); - vpninfo->cert_password = NULL; + password = NULL; err = request_passphrase(vpninfo, "openconnect_pkcs12", &pass, _("Enter PKCS#12 pass phrase:")); if (err) { @@ -493,6 +503,7 @@ static int load_pkcs12_certificate_(struct openconnect_info *vpninfo, } /* If it wasn't GNUTLS_E_MAC_VERIFY_FAILED, then the problem wasn't just a bad password. Give up. */ + error: if (err) { int level = PRG_ERR; int ret = -EINVAL; @@ -501,12 +512,14 @@ static int load_pkcs12_certificate_(struct openconnect_info *vpninfo, /* If the first attempt, and we didn't know for sure it was PKCS#12 anyway, bail out and try loading it as something different. */ - if (pass == vpninfo->cert_password) { + if (pass == password) { /* Make it non-fatal... */ level = PRG_DEBUG; ret = NOT_PKCS12; } + free_pass(&pass); + vpn_progress(vpninfo, level, _("Failed to process PKCS#12 file: %s\n"), gnutls_strerror(err)); @@ -515,7 +528,7 @@ static int load_pkcs12_certificate_(struct openconnect_info *vpninfo, err = gnutls_pkcs12_simple_parse(p12, pass, key, chain, chain_len, extra_certs, extra_certs_len, crl, 0); free_pass(&pass); - vpninfo->cert_password = NULL; + password = NULL; gnutls_pkcs12_deinit(p12); if (err) { @@ -529,6 +542,7 @@ static int load_pkcs12_certificate_(struct openconnect_info *vpninfo, static int load_pkcs12_certificate(struct openconnect_info *vpninfo, gnutls_datum_t *datum, + const char *password, gnutls_privkey_t *key, gnutls_x509_crt_t **chain, unsigned int *chain_len, @@ -544,7 +558,7 @@ static int load_pkcs12_certificate(struct openconnect_info *vpninfo, unsigned int i; int gerr, ret; - ret = load_pkcs12_certificate_(vpninfo, datum, &x509key, + ret = load_pkcs12_certificate_(vpninfo, datum, password, &x509key, &crts, &ncrts, &xcrts, &nxcrts, &crevoke); if (ret != 0) return ret; @@ -714,7 +728,8 @@ static int openssl_hash_password(struct openconnect_info *vpninfo, char *pass, static int import_openssl_pem(struct openconnect_info *vpninfo, gnutls_x509_privkey_t key, - char type, char *pem_header, size_t pem_size) + char type, char *pem_header, size_t pem_size, + const char *password) { gnutls_cipher_hd_t handle; gnutls_cipher_algorithm_t cipher; @@ -846,8 +861,11 @@ static int import_openssl_pem(struct openconnect_info *vpninfo, if (!key_data) goto out_enc_key; - pass = vpninfo->cert_password; - vpninfo->cert_password = NULL; + if ((pass = strdup(password)) == NULL) { + vpn_progress(vpninfo, PRG_ERR, + _("Out of memory.\n")); + goto out; + } while (1) { memcpy(key_data, b64_data.data, b64_data.size); @@ -1218,14 +1236,18 @@ static void check_tls13(struct openconnect_info *vpninfo, (void) key; } -static int load_certificate(struct openconnect_info *vpninfo) +static int load_keycert(struct openconnect_info *vpninfo, + const char *key_path, const char *cert_path, const char *password, + gnutls_privkey_t *key_, + gnutls_x509_crt_t **chain_, unsigned int *chain_len_, + gnutls_x509_crl_t *crl_) { gnutls_datum_t fdata = {NULL, 0}; gnutls_x509_privkey_t key = NULL; gnutls_privkey_t pkey = NULL; gnutls_datum_t pkey_sig = {NULL, 0}; - char *cert_url = (char *)vpninfo->cert; - char *key_url = (char *)vpninfo->sslkey; + char *key_url = NULL; + char *cert_url = NULL; #ifdef HAVE_P11KIT gnutls_pkcs11_privkey_t p11key = NULL; #endif @@ -1241,12 +1263,24 @@ static int load_certificate(struct openconnect_info *vpninfo) int cert_is_sys = 0, key_is_sys = 0; char name[80]; - key_is_p11 = !strncmp(vpninfo->sslkey, "pkcs11:", 7); - cert_is_p11 = !strncmp(vpninfo->cert, "pkcs11:", 7); + if (cert_path == NULL || cert_path[0] == 0) + return -EINVAL; + + if (key_path == NULL || key_path[0] == 0) + key_path = cert_path; + + if (key_ == NULL || chain_ == NULL || chain_len_ == NULL) + return -EINVAL; + + cert_url = (char *)cert_path; + key_url = (char *)key_path; + + key_is_p11 = !strncmp(key_path, "pkcs11:", 7); + cert_is_p11 = !strncmp(cert_path, "pkcs11:", 7); /* GnuTLS returns true for pkcs11:, tpmkey:, system:, and custom URLs. */ - key_is_sys = !key_is_p11 && gnutls_url_is_supported(vpninfo->sslkey); - cert_is_sys = !cert_is_p11 && gnutls_url_is_supported(vpninfo->cert); + key_is_sys = !key_is_p11 && gnutls_url_is_supported(key_path); + cert_is_sys = !cert_is_p11 && gnutls_url_is_supported(cert_path); #ifndef HAVE_GNUTLS_SYSTEM_KEYS if (key_is_sys || cert_is_sys) { @@ -1286,7 +1320,7 @@ static int load_certificate(struct openconnect_info *vpninfo) if (key_is_p11 && !p11_kit_uri_parse(key_url, P11_KIT_URI_FOR_ANY, uri)) { - if (vpninfo->sslkey == vpninfo->cert || + if (key_path == cert_path || !p11_kit_uri_get_attribute(uri, CKA_CLASS)) { class = CKO_PRIVATE_KEY; p11_kit_uri_set_attribute(uri, &attr); @@ -1311,7 +1345,7 @@ static int load_certificate(struct openconnect_info *vpninfo) goto out; } - gnutls_x509_crt_set_pin_function(cert, gnutls_pin_callback, vpninfo); + gnutls_x509_crt_set_pin_function(cert, pin_callback, vpninfo); /* Yes, even for *system* URLs the only API GnuTLS offers us is ...import_pkcs11_url(). */ @@ -1333,17 +1367,17 @@ static int load_certificate(struct openconnect_info *vpninfo) /* OK, not a PKCS#11 certificate so it must be coming from a file... */ vpn_progress(vpninfo, PRG_DEBUG, - _("Using certificate file %s\n"), vpninfo->cert); + _("Using certificate file %s\n"), cert_path); /* Load file contents */ - ret = load_datum(vpninfo, &fdata, vpninfo->cert); + ret = load_datum(vpninfo, &fdata, cert_path); if (ret) return ret; /* Is it PKCS#12? */ if (!key_is_p11) { /* PKCS#12 should actually contain certificates *and* private key */ - ret = load_pkcs12_certificate(vpninfo, &fdata, &pkey, + ret = load_pkcs12_certificate(vpninfo, &fdata, password, &pkey, &supporting_certs, &nr_supporting_certs, &extra_certs, &nr_extra_certs, &crl); @@ -1402,7 +1436,7 @@ static int load_certificate(struct openconnect_info *vpninfo) #ifdef HAVE_GNUTLS_SYSTEM_KEYS if (key_is_sys) { vpn_progress(vpninfo, PRG_DEBUG, - _("Using system key %s\n"), vpninfo->sslkey); + _("Using system key %s\n"), key_path); err = gnutls_privkey_init(&pkey); if (err) { @@ -1413,13 +1447,13 @@ static int load_certificate(struct openconnect_info *vpninfo) goto out; } - gnutls_privkey_set_pin_function(pkey, gnutls_pin_callback, vpninfo); + gnutls_privkey_set_pin_function(pkey, pin_callback, vpninfo); - err = gnutls_privkey_import_url(pkey, vpninfo->sslkey, 0); + err = gnutls_privkey_import_url(pkey, key_path, 0); if (err) { vpn_progress(vpninfo, PRG_ERR, - _("Error importing system key %s: %s\n"), - vpninfo->sslkey, gnutls_strerror(err)); + _("Error importing system key %s: %s\n"), + key_path, gnutls_strerror(err)); ret = -EIO; goto out; } @@ -1440,7 +1474,7 @@ static int load_certificate(struct openconnect_info *vpninfo) goto out; } - gnutls_pkcs11_privkey_set_pin_function(p11key, gnutls_pin_callback, vpninfo); + gnutls_pkcs11_privkey_set_pin_function(p11key, pin_callback, vpninfo); err = gnutls_pkcs11_privkey_import_url(p11key, key_url, 0); @@ -1451,7 +1485,7 @@ static int load_certificate(struct openconnect_info *vpninfo) can work out what token the *cert* was found in and try that before we give up... */ if (err == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE && - vpninfo->cert == vpninfo->sslkey) { + cert_path == key_path) { gnutls_pkcs11_obj_t crt; P11KitUri *uri; CK_TOKEN_INFO *token; @@ -1577,14 +1611,14 @@ static int load_certificate(struct openconnect_info *vpninfo) /* OK, not a PKCS#11 key so it must be coming from a file... load the file into memory, unless it's the same as the cert file and we already loaded that. */ - if (!fdata.data || vpninfo->sslkey != vpninfo->cert) { + if (!fdata.data || key_path != cert_path) { gnutls_free(fdata.data); fdata.data = NULL; vpn_progress(vpninfo, PRG_DEBUG, - _("Using private key file %s\n"), vpninfo->sslkey); + _("Using private key file %s\n"), key_path); - ret = load_datum(vpninfo, &fdata, vpninfo->sslkey); + ret = load_datum(vpninfo, &fdata, key_path); if (ret) goto out; } @@ -1642,7 +1676,7 @@ static int load_certificate(struct openconnect_info *vpninfo) while (*p == '\n' || *p == '\r') p++; ret = import_openssl_pem(vpninfo, key, type, p, - fdata.size - (p - (char *)fdata.data)); + fdata.size - (p - (char *)fdata.data), password); if (ret) goto out; } else { @@ -1669,7 +1703,7 @@ static int load_certificate(struct openconnect_info *vpninfo) } } else if (strstr((char *)fdata.data, "-----BEGIN ENCRYPTED PRIVATE KEY-----")) { /* Encrypted PKCS#8 */ - char *pass = vpninfo->cert_password; + char *pass = (char *)password; while ((err = gnutls_x509_privkey_import_pkcs8(key, &fdata, GNUTLS_X509_FMT_PEM, @@ -1681,7 +1715,7 @@ static int load_certificate(struct openconnect_info *vpninfo) ret = -EINVAL; goto out; } - vpninfo->cert_password = NULL; + password = NULL; if (pass) { vpn_progress(vpninfo, PRG_ERR, _("Failed to decrypt PKCS#8 certificate file\n")); @@ -1695,14 +1729,14 @@ static int load_certificate(struct openconnect_info *vpninfo) } } free_pass(&pass); - vpninfo->cert_password = NULL; + password = NULL; } else if (!gnutls_x509_privkey_import(key, &fdata, GNUTLS_X509_FMT_DER) || !gnutls_x509_privkey_import_pkcs8(key, &fdata, GNUTLS_X509_FMT_DER, NULL, GNUTLS_PKCS_PLAIN)) { /* Unencrypted DER (PKCS#1 or PKCS#8) */ } else { /* Last chance: try encrypted PKCS#8 DER. And give up if it's not that */ - char *pass = vpninfo->cert_password; + char *pass = (char *)password; while ((err = gnutls_x509_privkey_import_pkcs8(key, &fdata, GNUTLS_X509_FMT_DER, @@ -1714,7 +1748,7 @@ static int load_certificate(struct openconnect_info *vpninfo) ret = -EINVAL; goto out; } - vpninfo->cert_password = NULL; + password = NULL; if (pass) { vpn_progress(vpninfo, PRG_ERR, _("Failed to decrypt PKCS#8 certificate file\n")); @@ -1728,7 +1762,7 @@ static int load_certificate(struct openconnect_info *vpninfo) } } free_pass(&pass); - vpninfo->cert_password = NULL; + password = NULL; } /** @@ -1785,21 +1819,9 @@ static int load_certificate(struct openconnect_info *vpninfo) a PKCS#12 file we may have a trust chain in 'supporting_certs[]' too. */ check_certificate_expiry(vpninfo, cert); get_cert_name(cert, name, sizeof(name)); - get_cert_md5_fingerprint(vpninfo, cert, vpninfo->local_cert_md5); vpn_progress(vpninfo, PRG_INFO, _("Using client certificate '%s'\n"), name); - if (crl) { - err = gnutls_certificate_set_x509_crl(vpninfo->https_cred, &crl, 1); - if (err) { - vpn_progress(vpninfo, PRG_ERR, - _("Setting certificate revocation list failed: %s\n"), - gnutls_strerror(err)); - ret = -EINVAL; - goto out; - } - } - /* OpenSSL has problems with certificate chains — if there are multiple certs with the same name, it doesn't necessarily choose the _right_ one. (RT#1942) @@ -1810,8 +1832,8 @@ static int load_certificate(struct openconnect_info *vpninfo) since we'll expand the supporting_certs array with more from the cafile and extra_certs[] array if we can, and those extra certs must not be freed (twice). */ - if (!nr_supporting_certs) { - supporting_certs = gnutls_malloc(sizeof(*supporting_certs)); + if (nr_supporting_certs == 0) { + supporting_certs = gnutls_malloc(sizeof supporting_certs[0]); if (!supporting_certs) { vpn_progress(vpninfo, PRG_ERR, _("Failed to allocate memory for certificate\n")); @@ -1820,8 +1842,8 @@ static int load_certificate(struct openconnect_info *vpninfo) } supporting_certs[0] = cert; nr_supporting_certs = 1; - } + cert = NULL; last_cert = supporting_certs[nr_supporting_certs-1]; while (1) { @@ -1834,6 +1856,7 @@ static int load_certificate(struct openconnect_info *vpninfo) break; } + err = 0; if (i < nr_extra_certs) { /* We found the next cert in the chain in extra_certs[] */ issuer = extra_certs[i]; @@ -1868,11 +1891,11 @@ static int load_certificate(struct openconnect_info *vpninfo) } } #endif - if (err) - break; - } + if (err) + break; + if (gnutls_x509_crt_check_issuer(issuer, issuer)) { /* Don't actually include the root CA. If they don't already trust it, then handing it to them isn't going to help. But don't omit the @@ -1884,7 +1907,7 @@ static int load_certificate(struct openconnect_info *vpninfo) /* OK, we found a new cert to add to our chain. */ clist = supporting_certs; supporting_certs = gnutls_realloc(supporting_certs, - sizeof(cert) * (nr_supporting_certs+1)); + sizeof supporting_certs[0] * (nr_supporting_certs+1)); if (!supporting_certs) { supporting_certs = clist; vpn_progress(vpninfo, PRG_ERR, @@ -1898,6 +1921,7 @@ static int load_certificate(struct openconnect_info *vpninfo) nr_supporting_certs++; last_cert = issuer; } + for (i = 1; i < nr_supporting_certs; i++) { get_cert_name(supporting_certs[i], name, sizeof(name)); @@ -1905,44 +1929,31 @@ static int load_certificate(struct openconnect_info *vpninfo) _("Adding supporting CA '%s'\n"), name); } - check_tls13(vpninfo, pkey); - - /* OK, now we've checked the cert expiry and warned the user if it's - going to expire soon, and we've built up as much of a trust chain - in supporting_certs[] as we can find, to help the server work around - OpenSSL RT#1942. Set up the GnuTLS credentials with the appropriate - key and certs. - */ - err = assign_privkey(vpninfo, pkey, supporting_certs, - nr_supporting_certs); - if (err) { - vpn_progress(vpninfo, PRG_ERR, - _("Setting certificate failed: %s\n"), - gnutls_strerror(err)); - ret = -EIO; - goto out; - } - - pkey = NULL; /* we gave it away, and potentially also some - of extra_certs[] may have been zeroed. */ + /* we gave it away, and potentially also some + of extra_certs[] may have been zeroed. */ + /* pkey = NULL */ + *key_ = pkey, pkey = NULL; + *chain_ = supporting_certs, supporting_certs = NULL; + *chain_len_ = nr_supporting_certs, nr_supporting_certs = 0; + if (crl_) + *crl_ = crl, crl = NULL; ret = 0; out: gnutls_x509_crl_deinit(crl); gnutls_x509_privkey_deinit(key); - if (supporting_certs) { + if (nr_supporting_certs > 0) { for (i = 0; i < nr_supporting_certs; i++) { gnutls_x509_crt_deinit(supporting_certs[i]); } gnutls_free(supporting_certs); - } else if (cert) { + } else { /* Not if supporting_certs. It's supporting_certs[0] then and was already freed. */ gnutls_x509_crt_deinit(cert); } for (i = 0; i < nr_extra_certs; i++) { - if (extra_certs[i]) - gnutls_x509_crt_deinit(extra_certs[i]); + gnutls_x509_crt_deinit(extra_certs[i]); } gnutls_free(extra_certs); @@ -1951,13 +1962,66 @@ static int load_certificate(struct openconnect_info *vpninfo) just to point to something to hash. Don't free it in that case! */ free_datum(&fdata); free_datum(&pkey_sig); - if (cert_url != vpninfo->cert) + + if (cert_url != cert_path) free(cert_url); - if (key_url != vpninfo->sslkey) + if (key_url != key_path) free(key_url); return ret; } +static int load_certificate(struct openconnect_info *vpninfo) +{ + gnutls_privkey_t key = NULL; + gnutls_x509_crt_t *chain = NULL; + unsigned int chain_len = 0; + gnutls_x509_crl_t crl = NULL; + unsigned int i; + int ret; + + ret = load_keycert(vpninfo, vpninfo->sslkey, vpninfo->cert, + vpninfo->cert_password, &key, &chain, &chain_len, &crl); + if (ret < 0) + goto cleanup; + + ret = assign_privkey(vpninfo, key, chain, chain_len); + if (ret < 0) { + vpn_progress(vpninfo, PRG_ERR, + _("Setting certificate failed: (%d) %s\n"), + ret, gnutls_strerror(ret)); + ret = -EIO; + goto cleanup; + } + + check_tls13(vpninfo, key); + + key = NULL; /* we gave it away */ + + get_cert_md5_fingerprint(vpninfo, chain[0], vpninfo->local_cert_md5); + + if (crl != NULL) { + ret = gnutls_certificate_set_x509_crl(vpninfo->https_cred, + &crl, 1); + if (ret < 0) { + vpn_progress(vpninfo, PRG_ERR, + _("Setting certificate revocation list failed: (%d) %s\n"), + ret, gnutls_strerror(ret)); + ret = -EINVAL; + goto cleanup; + } + } + + ret = 0; + + cleanup: + gnutls_privkey_deinit(key); + for (i = 0; i < chain_len; i++) + gnutls_x509_crt_deinit(chain[i]); + gnutls_free(chain); + gnutls_x509_crl_deinit(crl); + return ret; +} + static int get_cert_fingerprint(struct openconnect_info *vpninfo, gnutls_x509_crt_t cert, gnutls_digest_algorithm_t algo, @@ -2547,56 +2611,21 @@ int openconnect_local_cert_md5(struct openconnect_info *vpninfo, } #if defined(HAVE_P11KIT) || defined(HAVE_GNUTLS_SYSTEM_KEYS) -static int gnutls_pin_callback(void *priv, int attempt, const char *uri, - const char *token_label, unsigned int flags, - char *pin, size_t pin_max) +static int request_pin(struct openconnect_info *vpninfo, char **ppin, + const char *token_uri, const char *token_label, unsigned int flags) { - struct openconnect_info *vpninfo = priv; - struct pin_cache **cache = &vpninfo->pin_cache; - struct oc_auth_form f; - struct oc_form_opt o; char message[1024]; + struct oc_auth_form f = {0}; + struct oc_form_opt o = {0}; int ret; - if (!vpninfo || !vpninfo->process_auth_form) - return -1; - - while (*cache) { - if (!strcmp(uri, (*cache)->token)) { - if ((*cache)->pin) { - if (attempt == 0) { - snprintf(pin, pin_max, "%s", (*cache)->pin); - return 0; - } - memset((*cache)->pin, 0x5a, strlen((*cache)->pin)); - free((*cache)->pin); - (*cache)->pin = NULL; - } - break; - } - cache = &(*cache)->next; - } - if (!*cache) { - *cache = calloc(1, sizeof(struct pin_cache)); - if (!*cache) - return -1; - - (*cache)->token = strdup(uri); - } - - if (!attempt && vpninfo->cert_password) { - snprintf(pin, pin_max, "%s", vpninfo->cert_password); - (*cache)->pin = vpninfo->cert_password; - vpninfo->cert_password = NULL; - return 0; - } + if (vpninfo->process_auth_form == NULL) + return GNUTLS_E_INVALID_REQUEST; - memset(&f, 0, sizeof(f)); f.auth_id = (char *)"pkcs11_pin"; f.opts = &o; - message[sizeof(message)-1] = 0; - snprintf(message, sizeof(message) - 1, _("PIN required for %s"), token_label); + snprintf(message, sizeof message, _("PIN required for %s"), token_label); f.message = message; if (flags & GNUTLS_PIN_WRONG) @@ -2614,14 +2643,103 @@ static int gnutls_pin_callback(void *priv, int attempt, const char *uri, o._value = NULL; ret = process_auth_form(vpninfo, &f); - if (ret || !o._value) - return -1; + if (ret != OC_FORM_RESULT_OK || o._value == NULL) + return GNUTLS_E_PKCS11_PIN_ERROR; - snprintf(pin, pin_max, "%s", o._value); - (*cache)->pin = o._value; + *ppin = o._value; return 0; } + +int pin_callback(void *user, int attempts, const char *token_uri, + const char *token_label, unsigned int flags, + char *pin, size_t pin_max) +{ + struct openconnect_info *vpninfo = user; + struct pin_cache *cache; + size_t pinlen; + int ret; + + if (!vpninfo) + return GNUTLS_E_INVALID_REQUEST; + + if (token_uri == NULL || token_uri[0] == 0) + token_uri = "(no URI)"; + + if (token_label == NULL || token_label[0] == 0) + token_label = "(no name)"; + + for (cache = vpninfo->pin_cache; cache != NULL; cache = cache->next) { + if (strcmp(cache->token, token_uri) == 0) + break; + } + + /** + * Add entry for token_uri if one does not exist + */ + if (cache == NULL) { + /** + * set myself up for a subsequent commit + */ + const char *first_pass = vpninfo->cert_password; + + cache = calloc(1, sizeof *cache); + if (cache == NULL) + return GNUTLS_E_MEMORY_ERROR; + *cache = + (struct pin_cache) { .pin = first_pass ? strdup(first_pass) : NULL, + .token = strdup(token_uri), + .next = vpninfo->pin_cache }; + if ((first_pass && cache->pin == NULL) + || cache->token == NULL) { + free_pass(&cache->pin); + free(cache->token); + free(cache); + return GNUTLS_E_MEMORY_ERROR; + } + vpninfo->pin_cache = cache; + } + + /** + * Wrong PIN. + */ + if (attempts > 0) + free_pass(&cache->pin); + + /** + * PIN should have length + */ + if (cache->pin && cache->pin[0] == 0) + free_pass(&cache->pin); + + /** + * No PIN in cache, prompt for PIN + */ + if (cache->pin == NULL) { + ret = request_pin(vpninfo, &cache->pin, token_uri,token_label, flags); + if (ret < 0) + return ret; + } else { + vpn_progress(vpninfo, PRG_DEBUG, + "Re-using cached PIN for token '%s'\n", token_label); + } + + /** + * Submit PIN + */ + pinlen = cache->pin != NULL ? strlen(cache->pin) : 0; + if (pinlen > 0 && pinlen < pin_max) { + strncpy(pin, cache->pin, pin_max); + return 0; + } else { + free_pass(&cache->pin); + if (pinlen == 0) + vpn_progress(vpninfo, PRG_ERR, _("PIN too long\n")); + else + vpn_progress(vpninfo, PRG_ERR, _("No PIN provided\n")); + return GNUTLS_E_PKCS11_PIN_ERROR; + } +} #endif /* HAVE_P11KIT || HAVE_GNUTLS_SYSTEM_KEYS */ #ifdef HAVE_LIBPCSCLITE -- GitLab From 294a015e4ac7669416c23b177414b9f453a91dda Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Wed, 6 May 2020 00:23:36 -0700 Subject: [PATCH 11/21] Use rcstring for password. rcstring simplifies password handling, especially in latter phases. Moreover, it reduces the number of copies. Signed-off-by: Tom Carroll --- gnutls.c | 115 ++++++++++++++++++++++++----------------- gnutls.h | 7 +++ openconnect-internal.h | 2 + ssl.c | 22 +++++--- 4 files changed, 94 insertions(+), 52 deletions(-) diff --git a/gnutls.c b/gnutls.c index 076654e4a..681ad12a6 100644 --- a/gnutls.c +++ b/gnutls.c @@ -444,7 +444,7 @@ static int load_datum(struct openconnect_info *vpninfo, static int load_pkcs12_certificate_(struct openconnect_info *vpninfo, gnutls_datum_t *datum, - const char *password, + const rcstring *password, gnutls_x509_privkey_t *key, gnutls_x509_crt_t **chain, unsigned int *chain_len, @@ -453,7 +453,7 @@ static int load_pkcs12_certificate_(struct openconnect_info *vpninfo, gnutls_x509_crl_t *crl) { gnutls_pkcs12_t p12; - char *pass; + const rcstring *pass; int err; err = gnutls_pkcs12_init(&p12); @@ -470,31 +470,22 @@ static int load_pkcs12_certificate_(struct openconnect_info *vpninfo, return NOT_PKCS12; } - pass = NULL; - if (password && (pass = strdup(password)) == NULL) { - err = GNUTLS_E_MEMORY_ERROR; - goto error; - } - /** - * A check of pass == password determines if this is the first - * attempt. - */ - password = pass; + pass = password_acquire(password); while ((err = gnutls_pkcs12_verify_mac(p12, pass)) == GNUTLS_E_MAC_VERIFY_FAILED) { if (!pass) { /* OpenSSL's PKCS12_parse() code will try both NULL and "" automatically, * but GnuTLS requires two separate attempts. */ err = gnutls_pkcs12_verify_mac(p12, ""); if (!err) { - pass = strdup(""); + pass = password_new(""); break; } } else vpn_progress(vpninfo, PRG_ERR, _("Failed to decrypt PKCS#12 certificate file\n")); - free_pass(&pass); + password_free(&pass); password = NULL; - err = request_passphrase(vpninfo, "openconnect_pkcs12", &pass, + err = ask_password(vpninfo, "openconnect_pkcs12", &pass, _("Enter PKCS#12 pass phrase:")); if (err) { gnutls_pkcs12_deinit(p12); @@ -503,7 +494,6 @@ static int load_pkcs12_certificate_(struct openconnect_info *vpninfo, } /* If it wasn't GNUTLS_E_MAC_VERIFY_FAILED, then the problem wasn't just a bad password. Give up. */ - error: if (err) { int level = PRG_ERR; int ret = -EINVAL; @@ -518,7 +508,7 @@ static int load_pkcs12_certificate_(struct openconnect_info *vpninfo, ret = NOT_PKCS12; } - free_pass(&pass); + password_free(&pass); vpn_progress(vpninfo, level, _("Failed to process PKCS#12 file: %s\n"), @@ -527,7 +517,7 @@ static int load_pkcs12_certificate_(struct openconnect_info *vpninfo, } err = gnutls_pkcs12_simple_parse(p12, pass, key, chain, chain_len, extra_certs, extra_certs_len, crl, 0); - free_pass(&pass); + password_free(&pass); password = NULL; gnutls_pkcs12_deinit(p12); @@ -542,7 +532,7 @@ static int load_pkcs12_certificate_(struct openconnect_info *vpninfo, static int load_pkcs12_certificate(struct openconnect_info *vpninfo, gnutls_datum_t *datum, - const char *password, + const rcstring *password, gnutls_privkey_t *key, gnutls_x509_crt_t **chain, unsigned int *chain_len, @@ -675,7 +665,7 @@ static int assign_privkey(struct openconnect_info *vpninfo, return err; } -static int openssl_hash_password(struct openconnect_info *vpninfo, char *pass, +static int openssl_hash_password(struct openconnect_info *vpninfo, const char *pass, gnutls_datum_t *key, gnutls_datum_t *salt) { unsigned char md5[16]; @@ -729,7 +719,7 @@ static int openssl_hash_password(struct openconnect_info *vpninfo, char *pass, static int import_openssl_pem(struct openconnect_info *vpninfo, gnutls_x509_privkey_t key, char type, char *pem_header, size_t pem_size, - const char *password) + const rcstring *password) { gnutls_cipher_hd_t handle; gnutls_cipher_algorithm_t cipher; @@ -738,7 +728,8 @@ static int import_openssl_pem(struct openconnect_info *vpninfo, gnutls_datum_t salt, enc_key; unsigned char *key_data; const char *begin; - char *pass, *p; + const char *pass; + char *p; char *pem_start = pem_header; int ret, err, i; @@ -861,11 +852,8 @@ static int import_openssl_pem(struct openconnect_info *vpninfo, if (!key_data) goto out_enc_key; - if ((pass = strdup(password)) == NULL) { - vpn_progress(vpninfo, PRG_ERR, - _("Out of memory.\n")); - goto out; - } + pass = password_acquire(password); + password = NULL; while (1) { memcpy(key_data, b64_data.data, b64_data.size); @@ -941,9 +929,9 @@ static int import_openssl_pem(struct openconnect_info *vpninfo, fail: if (pass) { vpn_progress(vpninfo, PRG_ERR, _("Decrypting PEM key failed\n")); - free_pass(&pass); + password_free(&pass); } - err = request_passphrase(vpninfo, "openconnect_pem", + err = ask_password(vpninfo, "openconnect_pem", &pass, _("Enter PEM pass phrase:")); if (err) { ret = -EINVAL; @@ -952,7 +940,7 @@ static int import_openssl_pem(struct openconnect_info *vpninfo, } out: free(key_data); - free_pass(&pass); + password_free(&pass); out_enc_key: free(enc_key.data); out_b64: @@ -1237,7 +1225,8 @@ static void check_tls13(struct openconnect_info *vpninfo, } static int load_keycert(struct openconnect_info *vpninfo, - const char *key_path, const char *cert_path, const char *password, + const char *key_path, const char *cert_path, + const rcstring *password, gnutls_privkey_t *key_, gnutls_x509_crt_t **chain_, unsigned int *chain_len_, gnutls_x509_crl_t *crl_) @@ -1703,11 +1692,15 @@ static int load_keycert(struct openconnect_info *vpninfo, } } else if (strstr((char *)fdata.data, "-----BEGIN ENCRYPTED PRIVATE KEY-----")) { /* Encrypted PKCS#8 */ - char *pass = (char *)password; + const char *pass = password_acquire(password); while ((err = gnutls_x509_privkey_import_pkcs8(key, &fdata, GNUTLS_X509_FMT_PEM, pass?:"", 0))) { + int pass_is_nonnull = pass != NULL; + + password_free(&pass); + password = NULL; if (err != GNUTLS_E_DECRYPTION_FAILED) { vpn_progress(vpninfo, PRG_ERR, _("Failed to load private key as PKCS#8: %s\n"), @@ -1715,20 +1708,18 @@ static int load_keycert(struct openconnect_info *vpninfo, ret = -EINVAL; goto out; } - password = NULL; - if (pass) { + if (pass_is_nonnull) { vpn_progress(vpninfo, PRG_ERR, _("Failed to decrypt PKCS#8 certificate file\n")); - free_pass(&pass); } - err = request_passphrase(vpninfo, "openconnect_pem", + err = ask_password(vpninfo, "openconnect_pem", &pass, _("Enter PEM pass phrase:")); if (err) { ret = -EINVAL; goto out; } } - free_pass(&pass); + password_free(&pass); password = NULL; } else if (!gnutls_x509_privkey_import(key, &fdata, GNUTLS_X509_FMT_DER) || !gnutls_x509_privkey_import_pkcs8(key, &fdata, GNUTLS_X509_FMT_DER, @@ -1736,11 +1727,15 @@ static int load_keycert(struct openconnect_info *vpninfo, /* Unencrypted DER (PKCS#1 or PKCS#8) */ } else { /* Last chance: try encrypted PKCS#8 DER. And give up if it's not that */ - char *pass = (char *)password; + const char *pass = password_acquire(password); while ((err = gnutls_x509_privkey_import_pkcs8(key, &fdata, GNUTLS_X509_FMT_DER, pass?:"", 0))) { + int pass_is_nonnull = pass != NULL; + + password_free(&pass); + password = NULL; if (err != GNUTLS_E_DECRYPTION_FAILED) { vpn_progress(vpninfo, PRG_ERR, _("Failed to determine type of private key %s\n"), @@ -1748,20 +1743,18 @@ static int load_keycert(struct openconnect_info *vpninfo, ret = -EINVAL; goto out; } - password = NULL; - if (pass) { + if (pass_is_nonnull) { vpn_progress(vpninfo, PRG_ERR, _("Failed to decrypt PKCS#8 certificate file\n")); - free_pass(&pass); } - err = request_passphrase(vpninfo, "openconnect_pem", + err = ask_password(vpninfo, "openconnect_pem", &pass, _("Enter PKCS#8 pass phrase:")); if (err) { ret = -EINVAL; goto out; } } - free_pass(&pass); + password_free(&pass); password = NULL; } @@ -1974,13 +1967,22 @@ static int load_certificate(struct openconnect_info *vpninfo) { gnutls_privkey_t key = NULL; gnutls_x509_crt_t *chain = NULL; + const rcstring *password; unsigned int chain_len = 0; gnutls_x509_crl_t crl = NULL; unsigned int i; int ret; + password = password_new(vpninfo->cert_password); + if (vpninfo->cert_password && password == NULL) + return -ENOMEM; + ret = load_keycert(vpninfo, vpninfo->sslkey, vpninfo->cert, - vpninfo->cert_password, &key, &chain, &chain_len, &crl); + password, &key, &chain, &chain_len, &crl); + + password_free(&password); + free_pass(&vpninfo->cert_password); + if (ret < 0) goto cleanup; @@ -2905,6 +2907,7 @@ const rcstring *rcstring_new_len(const char *s, size_t len) rcstr->str.len = len; memcpy(rcstr->str.data, s, len); rcstr->str.data[len] = 0; + /* coverity[leaked_storage] - rcstr is not leaked */ return rcstr->str.data; } @@ -2913,6 +2916,11 @@ const rcstring *rcstring_new(const char *s) return rcstring_new_len(s, strlen(s)); } +void zero_password(char *s, size_t n) +{ + clear_mem(s, n); +} + void rcstring_release_zero(const rcstring *str, void (*zero_func)(char *, size_t)) { struct rcstring_st *rcstr; @@ -2926,7 +2934,22 @@ void rcstring_release_zero(const rcstring *str, void (*zero_func)(char *, size_t } } -void zero_password(char *s, size_t n) +int __attribute__((format(printf, 4, 5))) +ask_password(struct openconnect_info *vpninfo, + const char *label, const rcstring **password, const char *fmt, ...) { - clear_mem(s, n); + const rcstring *str = NULL; + char *response = NULL; + va_list args; + int ret; + + va_start(args, fmt); + ret = request_passphrase_v(vpninfo, label, &response, fmt, args); + va_end(args); + if (ret == 0) + str = rcstring_new(response); + free_pass(&response); + if (str == NULL) + ret = -ENOMEM; + return ret; } diff --git a/gnutls.h b/gnutls.h index edaf58122..dd382fa4d 100644 --- a/gnutls.h +++ b/gnutls.h @@ -140,5 +140,12 @@ password_free(const rcstring **password) rcstring_release_zero(*password, zero_password); *password = NULL; } +/** + * ask_password is similar request_passphrase Instead of a + * char **, it expects const rcstring ** + */ +int __attribute__((format(printf, 4, 5))) +ask_password(struct openconnect_info *, const char *label, + const rcstring **password, const char *fmt, ...); #endif /* __OPENCONNECT_GNUTLS_H__ */ diff --git a/openconnect-internal.h b/openconnect-internal.h index 92edf763d..0b01dbb6e 100644 --- a/openconnect-internal.h +++ b/openconnect-internal.h @@ -948,6 +948,8 @@ int connect_https_socket(struct openconnect_info *vpninfo); int __attribute__ ((format(printf, 4, 5))) request_passphrase(struct openconnect_info *vpninfo, const char *label, char **response, const char *fmt, ...); +int request_passphrase_v(struct openconnect_info *vpninfo, const char *label, + char **response, const char *fmt, va_list args); int __attribute__ ((format (printf, 2, 3))) openconnect_SSL_printf(struct openconnect_info *vpninfo, const char *fmt, ...); int openconnect_print_err_cb(const char *str, size_t len, void *ptr); diff --git a/ssl.c b/ssl.c index 7d277c3e0..c08031984 100644 --- a/ssl.c +++ b/ssl.c @@ -523,21 +523,18 @@ int __attribute__ ((format (printf, 2, 3))) } -int __attribute__ ((format(printf, 4, 5))) - request_passphrase(struct openconnect_info *vpninfo, const char *label, - char **response, const char *fmt, ...) +int request_passphrase_v(struct openconnect_info *vpninfo, + const char *label, char **response, + const char *fmt, va_list args) { struct oc_auth_form f; struct oc_form_opt o; char buf[1024]; - va_list args; int ret; buf[1023] = 0; memset(&f, 0, sizeof(f)); - va_start(args, fmt); vsnprintf(buf, 1023, fmt, args); - va_end(args); f.auth_id = (char *)label; f.opts = &o; @@ -557,6 +554,19 @@ int __attribute__ ((format(printf, 4, 5))) return -EIO; } +int __attribute__ ((format(printf, 4, 5))) + request_passphrase(struct openconnect_info *vpninfo, const char *label, + char **response, const char *fmt, ...) +{ + va_list args; + int ret; + + va_start(args, fmt); + ret = request_passphrase_v(vpninfo, label, response, fmt, args); + va_end(args); + return ret; +} + #if defined(__sun__) || defined(__NetBSD__) || defined(__DragonFly__) int openconnect_passphrase_from_fsid(struct openconnect_info *vpninfo) { -- GitLab From 005ffed2ccbb1eb96e5f1d54779d4223ce2f1033 Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Thu, 23 Apr 2020 23:15:59 -0700 Subject: [PATCH 12/21] GnuTLS challenge signing This is the GnuTLS implementation for signing the challenge. Variables added to store the paths for the second set of credentials. Challenge response comprises two elements: an identity and signature. The identity is the user certificate, represented as a PEM-encoded PKCS7 certificate chain. The challenge is signed with the user certificate and submitted back to the server as a PEM-encoded signature. Signed-off-by: Tom Carroll --- gnutls.c | 217 +++++++++++++++++++++++++++++++++++++++++ library.c | 3 + openconnect-internal.h | 30 ++++++ 3 files changed, 250 insertions(+) diff --git a/gnutls.c b/gnutls.c index 681ad12a6..b40fa64a7 100644 --- a/gnutls.c +++ b/gnutls.c @@ -2024,6 +2024,223 @@ static int load_certificate(struct openconnect_info *vpninfo) return ret; } +static int pem_encode(const gnutls_datum_t *data, + gnutls_datum_t *result) +{ + char *r, *s, *p = NULL; + size_t r_n, s_n, p_n = 0; + int ret; + + if (result == NULL) + return GNUTLS_E_INVALID_REQUEST; + + r = (char *)data->data; + r_n = data->size; + + /* handle when data->size == 0 */ + if (r_n == 0) { + ret = 0; + goto done; + } + + /* size of encoding */ + p_n = ((r_n + 2) / 3) * 4; + /* plus newlines (subtract off nul) */ + p_n += ((p_n + 63) / 64) - 1; + /* plus nul */ + p = s = malloc(p_n + 1); s_n = p_n; + if (p == NULL) + return GNUTLS_E_MEMORY_ERROR; + + /** + * r := ptr to data to encode + * r_n := the length of data to encode + * s := output pointer + * s_n := length of output, not including terminating nul + */ + while (r_n > 0) { + /* encode one line at at time */ + const gnutls_datum_t datum = {(void *)r, r_n > 48 ? 48 : r_n}; + char tmp[66]; + size_t n = sizeof tmp; + + r += datum.size; r_n -= datum.size; + ret = gnutls_pem_base64_encode(NULL, &datum, tmp, &n); + /* check assumptions */ + if ((ret < 0) || (n > 64) || (n > s_n)) { + ret = GNUTLS_E_BASE64_ENCODING_ERROR; + goto cleanup; + } + /* append nul */ + tmp[n++] = '\n'; + /* copy */ + memcpy(s, tmp, n); + /* s_n will wrap on the last block */ + s += n; s_n -= n; + } + /* nul terminate */ + p[p_n] = 0; + + done: + *result = (gnutls_datum_t) {(void *)p, p_n}; + p = NULL; + + cleanup: + free(p); + return ret; +} + +/** + * Sign the challenge provide by the server. + * + * challenge is the XML response provided by the server. An example is + * + * + * + * ANYCONNECT-MCA + * 136775778 + * multiple-cert + * single-sign-on + * 1506879881148 + * + * + * sha256 + * sha384 + * sha512 + * + * FA4003BD87436B227####snip####C138A08FF724F0100015B863F75091483 9EE79C86DFE8F0B9A0199E2 + * + * + * + * identity is a PEM-encoded (base64, line length of 64) PKCS7 + * certificate chain. Note that I have only gotten anyconnect to + * use a single certificate for signing a challenge. + * + * response is the PEM-encoded signature of the callenge. + */ +int cert_auth_challenge_response(struct openconnect_info *vpninfo, + int cert_rq, const char *challenge, char **identity, + struct challenge_response *response) +{ + const struct table_entry { + int id; + gnutls_digest_algorithm_t digest; + } *entry, table[] = { + { CERT_AUTH_DIGEST_SHA512, GNUTLS_DIG_SHA512 }, + { CERT_AUTH_DIGEST_SHA384, GNUTLS_DIG_SHA384 }, + { CERT_AUTH_DIGEST_SHA256, GNUTLS_DIG_SHA256 }, + { CERT_AUTH_DIGEST_UNKNOWN, GNUTLS_DIG_UNKNOWN }, + }; + gnutls_privkey_t key = NULL; + gnutls_pubkey_t pubkey = NULL; + gnutls_x509_crt_t *chain = NULL; + unsigned int chain_len = 0; + gnutls_pkcs7_t p7 = NULL; + gnutls_datum_t p7bin, p7pem = {NULL, 0}; + const gnutls_datum_t dat = {(void *)challenge, strlen(challenge)}; + gnutls_datum_t sig, sigpem = {NULL, 0}; + unsigned int i; + int pk; + int ret = -ENOMEM, err = 0; + + if (response == NULL) + return -EINVAL; + + ret = load_keycert(vpninfo, vpninfo->key2, vpninfo->cert2, + vpninfo->key2_password, &key, &chain, &chain_len, NULL); + if (ret < 0) + goto cleanup; + /** + * Get PK algorithm of the certificate + */ + err = gnutls_pubkey_init(&pubkey); + if (err < 0) + goto cleanup; + + err = gnutls_pubkey_import_x509(pubkey, chain[0], 0); + if (err < 0) + goto cleanup; + + pk = gnutls_pubkey_get_pk_algorithm(pubkey, NULL); + + /* Export identity as PKCS7 */ + err = gnutls_pkcs7_init(&p7); + if (err < 0) + goto cleanup; + + for (i = 0; i < chain_len; i++) { + err = gnutls_pkcs7_set_crt(p7, chain[i]); + if (err < 0) + goto cleanup; + } + + err = gnutls_pkcs7_export2(p7, GNUTLS_X509_FMT_DER, &p7bin); + if (err < 0) + goto cleanup; + + /* export as PEM */ + err = pem_encode(&p7bin, &p7pem); + free_datum(&p7bin); + if (err < 0) + goto cleanup; + + /** + * Sign the challenge + * Anyconnect prefers SHA512 + */ + for (entry = table; entry->digest != GNUTLS_DIG_UNKNOWN; entry++) { + gnutls_digest_algorithm_t algo = entry->digest; + + if ((cert_rq & entry->id) != entry->id) + continue; +#if GNUTLS_VERSION_NUMBER >= 0x030600 + err = gnutls_privkey_sign_data2(key, gnutls_pk_to_sign(pk, algo), 0, &dat, &sig); +#else /* GNUTLS_VERSION_NUMBER < 0x030600 */ + err = gnutls_privkey_sign_data(key, algo, 0, &dat, &sig); +#endif + if (err != GNUTLS_E_INVALID_REQUEST + && err != GNUTLS_E_CONSTRAINT_ERROR) + break; + } + if (entry->digest == GNUTLS_DIG_UNKNOWN) { + err = GNUTLS_E_UNSUPPORTED_SIGNATURE_ALGORITHM; + ret = -EINVAL; + } + if (err < 0) + goto cleanup; + + err = pem_encode(&sig, &sigpem); + free_datum(&sig); + if (err < 0) + goto cleanup; + + /* Success! */ + *identity = (char *)p7pem.data; + p7pem = (gnutls_datum_t) {0}; + *response = + (struct challenge_response) {entry->id, (char *)sigpem.data}; + sigpem = (gnutls_datum_t) {0}; + ret = 0; + + cleanup: + if (err < 0) { + vpn_progress(vpninfo, PRG_ERR, + _("Failed responding to the challenge: (%d) %s\n"), + err, gnutls_strerror(err)); + vpn_progress(vpninfo, PRG_DEBUG, + _("cert_rq = 0x%.2x"), cert_rq); + } + free_datum(&sigpem); + free_datum(&p7pem); + gnutls_pkcs7_deinit(p7); + gnutls_privkey_deinit(key); + gnutls_pubkey_deinit(pubkey); + for (i = 0; i < chain_len; i++) + gnutls_x509_crt_deinit(chain[i]); + gnutls_free(chain); + return ret; +} + static int get_cert_fingerprint(struct openconnect_info *vpninfo, gnutls_x509_crt_t cert, gnutls_digest_algorithm_t algo, diff --git a/library.c b/library.c index 586c5798a..5d89f4d1d 100644 --- a/library.c +++ b/library.c @@ -395,6 +395,9 @@ void openconnect_vpninfo_free(struct openconnect_info *vpninfo) free(vpninfo->ifname); free(vpninfo->dtls_cipher); free(vpninfo->peer_cert_hash); + free(vpninfo->cert2); + free(vpninfo->key2); + free_pass(&vpninfo->key2_password); #if defined(OPENCONNECT_OPENSSL) free(vpninfo->cstp_cipher); #if defined(HAVE_BIO_METH_FREE) diff --git a/openconnect-internal.h b/openconnect-internal.h index 0b01dbb6e..b86b6cb24 100644 --- a/openconnect-internal.h +++ b/openconnect-internal.h @@ -451,6 +451,10 @@ struct openconnect_info { int no_http_keepalive; int dump_http_traffic; + char *cert2; + char *key2; + char *key2_password; + int token_mode; int token_bypassed; int token_tries; @@ -744,6 +748,32 @@ struct openconnect_info { } while(0) #define vpn_perror(vpninfo, msg) vpn_progress((vpninfo), PRG_ERR, "%s: %s\n", (msg), strerror(errno)) +/* certificate authentication */ +typedef enum { + CERT_AUTH_REQ_CERT1 = (1U<<0), +#define CERT_AUTH_REQ_CERT1 CERT_AUTH_REQ_CERT1 + CERT_AUTH_REQ_CERT2 = (1U<<1), +#define CERT_AUTH_CERT2 CERT_AUTH_REQ_CERT2, + CERT_AUTH_DIGEST_SHA256 = (1U<<8), +#define CERT_AUTH_DIGEST_SHA256 CERT_AUTH_DIGEST_SHA256 + CERT_AUTH_DIGEST_SHA384 = (1U<<9), +#define CERT_AUTH_DIGEST_SHA384 CERT_AUTH_DIGEST_SHA384 + CERT_AUTH_DIGEST_SHA512 = (1U<<10), +#define CERT_AUTH_DIGEST_SHA512 CERT_AUTH_DIGEST_SHA512 + CERT_AUTH_DIGEST_UNKNOWN = (1U<<30), +#define CERT_AUTH_DIGEST_UNKNOWN CERT_AUTH_DIGEST_UNKNOWN +} cert_auth_t; +#define CERT_AUTH_DIGEST_MASK (CERT_AUTH_DIGEST_SHA256|CERT_AUTH_DIGEST_SHA384|CERT_AUTH_DIGEST_SHA512) + +struct challenge_response { + int digest; + char *data; +}; + +int cert_auth_challenge_response(struct openconnect_info *vpninfo, + int cert_rq, const char *challenge, char **identity, + struct challenge_response *response); + /****************************************************************************/ /* Oh Solaris how we hate thee! */ #ifdef HAVE_SUNOS_BROKEN_TIME -- GitLab From 5c0bafe76391c219285787ae6e1fd3dd727b560f Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Fri, 24 Apr 2020 23:47:33 -0700 Subject: [PATCH 13/21] Add password to pin callback There are (at least) two passwords, one for TLS session and another for the multiple certificate-based authentication challenge. Add password parameter to the pin callback. Signed-off-by: Tom Carroll --- gnutls.c | 153 +++++++++++++++++++++++++++++++++-------- openconnect-internal.h | 3 + 2 files changed, 127 insertions(+), 29 deletions(-) diff --git a/gnutls.c b/gnutls.c index b40fa64a7..0127dc05c 100644 --- a/gnutls.c +++ b/gnutls.c @@ -40,9 +40,27 @@ #endif #if defined(HAVE_P11KIT) || defined(HAVE_GNUTLS_SYSTEM_KEYS) -static int pin_callback(void *user, int attempt, const char *token_uri, - const char *token_label, unsigned int flags, - char *pin, size_t pin_max); +/** + * pin callback context + * + * Password (first_pass) is notably part of the context + */ +struct oc_pin_ctx { + gnutls_pin_callback_t cb; + void *cb_arg; + + struct openconnect_info *vpninfo; + char *first_pass; + struct oc_pin_ctx *next; +}; + +static struct oc_pin_ctx *pinctx_new( + struct openconnect_info *vpninfo, + const char *password); + +static void pinctx_free(struct oc_pin_ctx *ctx); + +static void release_pin_ctx(struct openconnect_info *vpninfo); #endif /* HAVE_P11KIT || HAVE_GNUTLS_SYSTEM_KEYS */ #include "gnutls.h" @@ -1244,6 +1262,7 @@ static int load_keycert(struct openconnect_info *vpninfo, gnutls_x509_crl_t crl = NULL; gnutls_x509_crt_t last_cert, cert = NULL; gnutls_x509_crt_t *extra_certs = NULL, *supporting_certs = NULL; + struct oc_pin_ctx *pinctx = NULL; unsigned int nr_supporting_certs = 0, nr_extra_certs = 0; int err; /* GnuTLS error */ int ret; @@ -1334,11 +1353,17 @@ static int load_keycert(struct openconnect_info *vpninfo, goto out; } - gnutls_x509_crt_set_pin_function(cert, pin_callback, vpninfo); + err = 0; + pinctx = pinctx_new(vpninfo, password); + if (pinctx == NULL) + err = GNUTLS_E_MEMORY_ERROR; + if (err >= 0) { + gnutls_x509_crt_set_pin_function(cert, pinctx->cb, pinctx->cb_arg); - /* Yes, even for *system* URLs the only API GnuTLS offers us is + /* Yes, even for *system* URLs the only API GnuTLS offers us is ...import_pkcs11_url(). */ - err = gnutls_x509_crt_import_pkcs11_url(cert, cert_url, 0); + err = gnutls_x509_crt_import_pkcs11_url(cert, cert_url, 0); + } if (err == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) err = gnutls_x509_crt_import_pkcs11_url(cert, cert_url, GNUTLS_PKCS11_OBJ_FLAG_LOGIN); @@ -1350,6 +1375,9 @@ static int load_keycert(struct openconnect_info *vpninfo, ret = -EIO; goto out; } + pinctx->next = vpninfo->pin_ctx; + vpninfo->pin_ctx = pinctx; + pinctx = NULL; goto got_certs; } #endif /* HAVE_P11KIT || HAVE_GNUTLS_SYSTEM_KEYS */ @@ -1436,9 +1464,14 @@ static int load_keycert(struct openconnect_info *vpninfo, goto out; } - gnutls_privkey_set_pin_function(pkey, pin_callback, vpninfo); - - err = gnutls_privkey_import_url(pkey, key_path, 0); + err = 0; + pinctx = pinctx_new(vpninfo, password); + if (pinctx == NULL) + err = GNUTLS_E_MEMORY_ERROR; + if (err >= 0) { + gnutls_privkey_set_pin_function(pkey, pinctx->cb, pinctx->cb_arg); + err = gnutls_privkey_import_url(pkey, key_path, 0); + } if (err) { vpn_progress(vpninfo, PRG_ERR, _("Error importing system key %s: %s\n"), @@ -1446,6 +1479,9 @@ static int load_keycert(struct openconnect_info *vpninfo, ret = -EIO; goto out; } + pinctx->next = vpninfo->pin_ctx; + vpninfo->pin_ctx = pinctx; + pinctx = NULL; goto match_cert; } #endif /* HAVE_GNUTLS_SYSTEM_KEYS */ @@ -1463,9 +1499,14 @@ static int load_keycert(struct openconnect_info *vpninfo, goto out; } - gnutls_pkcs11_privkey_set_pin_function(p11key, pin_callback, vpninfo); - - err = gnutls_pkcs11_privkey_import_url(p11key, key_url, 0); + err = 0; + pinctx = pinctx_new(vpninfo, password); + if (pinctx == NULL) + err = GNUTLS_E_MEMORY_ERROR; + if (err >= 0) { + gnutls_pkcs11_privkey_set_pin_function(p11key, pinctx->cb, pinctx->cb_arg); + err = gnutls_pkcs11_privkey_import_url(p11key, key_url, 0); + } /* Annoyingly, some tokens don't even admit the *existence* of the key until they're logged in. And thus a search doesn't @@ -1593,6 +1634,9 @@ static int load_keycert(struct openconnect_info *vpninfo, ret = -EIO; goto out; } + pinctx->next = vpninfo->pin_ctx; + vpninfo->pin_ctx = pinctx; + pinctx = NULL; goto match_cert; } #endif /* HAVE_P11KIT */ @@ -1956,6 +2000,8 @@ static int load_keycert(struct openconnect_info *vpninfo, free_datum(&fdata); free_datum(&pkey_sig); + pinctx_free(pinctx); + if (cert_url != cert_path) free(cert_url); if (key_url != key_path) @@ -2754,6 +2800,9 @@ void openconnect_close_https(struct openconnect_info *vpninfo, int final) #ifdef HAVE_TSS2 release_tpm2_ctx(vpninfo); #endif +#if defined(HAVE_P11KIT) || defined(HAVE_GNUTLS_SYSTEM_KEYS) + release_pin_ctx(vpninfo); +#endif /* defined(HAVE_P11KIT) || defined(HAVE_GNUTLS_SYSTEM_KEYS) */ } } @@ -2870,18 +2919,15 @@ static int request_pin(struct openconnect_info *vpninfo, char **ppin, return 0; } -int pin_callback(void *user, int attempts, const char *token_uri, - const char *token_label, unsigned int flags, - char *pin, size_t pin_max) +static int pin_callback_(struct openconnect_info *vpninfo, + char *first_pass, int attempts, + const char *token_uri, const char *token_label, + unsigned int flags, char *pin, size_t pin_max) { - struct openconnect_info *vpninfo = user; struct pin_cache *cache; size_t pinlen; int ret; - if (!vpninfo) - return GNUTLS_E_INVALID_REQUEST; - if (token_uri == NULL || token_uri[0] == 0) token_uri = "(no URI)"; @@ -2897,28 +2943,25 @@ int pin_callback(void *user, int attempts, const char *token_uri, * Add entry for token_uri if one does not exist */ if (cache == NULL) { - /** - * set myself up for a subsequent commit - */ - const char *first_pass = vpninfo->cert_password; - cache = calloc(1, sizeof *cache); if (cache == NULL) return GNUTLS_E_MEMORY_ERROR; *cache = - (struct pin_cache) { .pin = first_pass ? strdup(first_pass) : NULL, - .token = strdup(token_uri), - .next = vpninfo->pin_cache }; - if ((first_pass && cache->pin == NULL) - || cache->token == NULL) { + (struct pin_cache) { .pin = first_pass, + .token = strdup(token_uri), + .next = vpninfo->pin_cache }; + if (cache->token == NULL) { free_pass(&cache->pin); free(cache->token); free(cache); return GNUTLS_E_MEMORY_ERROR; } vpninfo->pin_cache = cache; + first_pass = NULL; } + free_pass(&first_pass); + /** * Wrong PIN. */ @@ -2959,6 +3002,58 @@ int pin_callback(void *user, int attempts, const char *token_uri, return GNUTLS_E_PKCS11_PIN_ERROR; } } + +static int pin_callback(void *user, int attempts, + const char *token_uri, const char *token_label, + unsigned int flags, char *pin, size_t pin_max) +{ + struct oc_pin_ctx *cb = user; + int ret; + + if (cb == NULL || cb->vpninfo == NULL) + return GNUTLS_E_INVALID_REQUEST; + ret = pin_callback_(cb->vpninfo, cb->first_pass, attempts, + token_uri, token_label, flags, pin, pin_max); + cb->first_pass = NULL; + return ret; +} + +struct oc_pin_ctx * +pinctx_new(struct openconnect_info *vpninfo, const char *password) +{ + struct oc_pin_ctx *pinctx; + char *first_pass = NULL; + + if ((pinctx = malloc(sizeof *pinctx)) == NULL) + return NULL; + if (password && (first_pass = strdup(password)) == NULL) { + free(pinctx); + return NULL; + } + *pinctx = (struct oc_pin_ctx) { .cb = pin_callback, + .cb_arg = pinctx, + .vpninfo = vpninfo, + .first_pass = first_pass }; + return pinctx; +} + +void pinctx_free(struct oc_pin_ctx *pinctx) +{ + if (pinctx) { + free_pass(&pinctx->first_pass); + free(pinctx); + } +} + +void release_pin_ctx(struct openconnect_info *vpninfo) +{ + struct oc_pin_ctx *pinctx; + + while ((pinctx = vpninfo->pin_ctx)) { + vpninfo->pin_ctx = pinctx->next; + pinctx_free(pinctx); + } +} #endif /* HAVE_P11KIT || HAVE_GNUTLS_SYSTEM_KEYS */ #ifdef HAVE_LIBPCSCLITE diff --git a/openconnect-internal.h b/openconnect-internal.h index b86b6cb24..b3806136e 100644 --- a/openconnect-internal.h +++ b/openconnect-internal.h @@ -364,6 +364,7 @@ struct esp { struct oc_pcsc_ctx; struct oc_tpm1_ctx; struct oc_tpm2_ctx; +struct oc_pin_ctx; struct openconnect_info { const struct vpn_proto *proto; @@ -455,6 +456,8 @@ struct openconnect_info { char *key2; char *key2_password; + struct oc_pin_ctx *pin_ctx; + int token_mode; int token_bypassed; int token_tries; -- GitLab From 1f878bd5a273b7b010b7fb634f631276907ce683 Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Fri, 24 Apr 2020 23:04:43 -0700 Subject: [PATCH 14/21] Add password parameter to load_tpm1 Signed-off-by: Tom Carroll --- gnutls.c | 2 +- gnutls.h | 1 + gnutls_tpm.c | 26 ++++++++++++++++++++------ 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/gnutls.c b/gnutls.c index 0127dc05c..2a8060ac3 100644 --- a/gnutls.c +++ b/gnutls.c @@ -1663,7 +1663,7 @@ static int load_keycert(struct openconnect_info *vpninfo, _("This version of OpenConnect was built without TPM support\n")); return -EINVAL; #else - ret = load_tpm1_key(vpninfo, &fdata, &pkey, &pkey_sig); + ret = load_tpm1_key(vpninfo, &fdata, password, &pkey, &pkey_sig); if (ret) goto out; diff --git a/gnutls.h b/gnutls.h index dd382fa4d..6d39f9be9 100644 --- a/gnutls.h +++ b/gnutls.h @@ -25,6 +25,7 @@ #include "openconnect-internal.h" int load_tpm1_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, + const char *password, gnutls_privkey_t *pkey, gnutls_datum_t *pkey_sig); void release_tpm1_ctx(struct openconnect_info *info); diff --git a/gnutls_tpm.c b/gnutls_tpm.c index 2bed077f8..01abc3f04 100644 --- a/gnutls_tpm.c +++ b/gnutls_tpm.c @@ -86,14 +86,22 @@ static int tpm_sign_fn(gnutls_privkey_t key, void *_vpninfo, } int load_tpm1_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, + const char *password, gnutls_privkey_t *pkey, gnutls_datum_t *pkey_sig) { static const TSS_UUID SRK_UUID = TSS_UUID_SRK; gnutls_datum_t asn1; unsigned int tss_len; char *pass; + unsigned int tries; int ofs, err; + if (vpninfo->tpm1) { + vpn_progress(vpninfo, PRG_ERR, + _("TPM1 is in use.\n")); + return -EBUSY; + } + err = gnutls_pem_base64_decode_alloc("TSS KEY BLOB", fdata, &asn1); if (err) { vpn_progress(vpninfo, PRG_ERR, @@ -163,9 +171,14 @@ int load_tpm1_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, goto out_srk; } - pass = vpninfo->cert_password; - vpninfo->cert_password = NULL; - while (1) { + pass = NULL; + if (password && (pass = strdup(password)) == NULL) { + vpn_progress(vpninfo, PRG_ERR, + _("Out of memory.\n")); + goto out_srk; + } + + for (tries = 0; ; tries++) { static const char nullpass[20]; /* We don't seem to get the error here... */ @@ -177,6 +190,9 @@ int load_tpm1_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, err = Tspi_Policy_SetSecret(vpninfo->tpm1->srk_policy, TSS_SECRET_MODE_SHA1, sizeof(nullpass), (BYTE *)nullpass); + + free_pass(&pass); + if (err) { vpn_progress(vpninfo, PRG_ERR, _("Failed to set TPM PIN: %s\n"), @@ -184,8 +200,6 @@ int load_tpm1_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, goto out_srkpol; } - free_pass(&pass); - /* ... we get it here instead. */ err = Tspi_Context_LoadKeyByBlob(vpninfo->tpm1->tpm_context, vpninfo->tpm1->srk, tss_len, asn1.data + ofs, @@ -193,7 +207,7 @@ int load_tpm1_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, if (!err) break; - if (pass) + if (tries > 0 || password) vpn_progress(vpninfo, PRG_ERR, _("Failed to load TPM key blob: %s\n"), Trspi_Error_String(err)); -- GitLab From 901d58bf30b33e89055248037527e9a807c5b389 Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Thu, 23 Apr 2020 23:15:59 -0700 Subject: [PATCH 15/21] Converse the multicert auth protocol Implement the multiple certificate-based authentication protocol. The XML-message protocol may require three rounds of conversations (instead of the existing two): HTTP redirect, announce capabilities and receive multicert challenge, and respond to the multicert challenge and obtain cookie. Signed-off-by: Tom Carroll --- auth.c | 301 ++++++++++++++++++++++++++++++++++++++++- main.c | 30 ++++ openconnect-internal.h | 4 + 3 files changed, 332 insertions(+), 3 deletions(-) diff --git a/auth.c b/auth.c index c07ed15b3..6684b8b24 100644 --- a/auth.c +++ b/auth.c @@ -44,6 +44,14 @@ static int cstp_can_gen_tokencode(struct openconnect_info *vpninfo, struct oc_auth_form *form, struct oc_form_opt *opt); +/* multiple certificate-based authentication */ +static int announce_auth_methods(struct openconnect_info *vpninfo, + xmlNodePtr root); +static void parse_multicert_request(struct openconnect_info *vpninfo, + xmlNodePtr node, int *cert_rq); +static int multicert_response(struct openconnect_info *vpninfo, + int cert_rq, const char *challenge, struct oc_text_buf *body); + int openconnect_set_option_value(struct oc_form_opt *opt, const char *value) { if (opt->type == OC_FORM_OPT_SELECT) { @@ -575,7 +583,7 @@ static int parse_xml_response(struct openconnect_info *vpninfo, char *response, continue; } else if (xmlnode_is_named(xml_node, "client-cert-request")) { if (cert_rq) - *cert_rq = 1; + *cert_rq |= CERT_AUTH_REQ_CERT1; else { vpn_progress(vpninfo, PRG_ERR, _("Received when not expected.\n")); @@ -594,6 +602,22 @@ static int parse_xml_response(struct openconnect_info *vpninfo, char *response, ret = parse_host_scan_node(vpninfo, xml_node); } else if (xmlnode_is_named(xml_node, "config")) { parse_config_node(vpninfo, xml_node); + } else if (xmlnode_is_named(xml_node, "multiple-client-cert-request")) { + if (cert_rq) { + *cert_rq |= CERT_AUTH_REQ_CERT1|CERT_AUTH_REQ_CERT2; + parse_multicert_request(vpninfo, xml_node, cert_rq); + } else { + vpn_progress(vpninfo, PRG_ERR, + _("Received when not expected.\n")); + ret = -EINVAL; + } + } else if (xmlnode_is_named(xml_node, "cert-authenticated")) { + /** + * cert-authenticated indicates that the certificate for the + * TLS session is valid. Thus, remove flag for CERT1 request. + */ + if (cert_rq) + *cert_rq &= ~CERT_AUTH_REQ_CERT1; } else { xmlnode_get_text(xml_node, "session-token", &vpninfo->cookie); xmlnode_get_text(xml_node, "error", &form->error); @@ -805,9 +829,14 @@ static int xmlpost_initial_req(struct openconnect_info *vpninfo, if (buf_error(url_buf)) goto bad; - node = xmlNewTextChild(root, NULL, XCAST("group-access"), XCAST(url_buf->data)); + node = xmlNewTextChild(root, NULL, XCAST("group-access"), + XCAST(url_buf->data)); if (!node) goto bad; + + if (announce_auth_methods(vpninfo, root) < 0) + goto bad; + if (cert_fail) { node = xmlNewTextChild(root, NULL, XCAST("client-cert-fail"), NULL); if (!node) @@ -1362,7 +1391,7 @@ newgroup: if (result < 0) goto fail; - if (cert_rq) { + if (cert_rq & CERT_AUTH_REQ_CERT1) { int cert_failed = 0; free_auth_form(form); @@ -1386,7 +1415,26 @@ newgroup: if (result < 0) goto fail; continue; + } else if (cert_rq & CERT_AUTH_REQ_CERT2) { + // load cert + free_auth_form(form); form = NULL; + buf_truncate(request_body); + result = multicert_response(vpninfo, cert_rq, form_buf, + request_body); + if (result < 0) + goto fail; + /** + * Hack!!! + * If we processed a redirect, ntries == 3 once execution + * returns to the top of the for loop. + * This causes the client to attempt GET instead of POST. This + * is not what we want. Thus, we decrement tries by one as + * a work around. + */ + tries -= tries > 0; + continue; } + if (form && form->action) { vpninfo->redirect_url = strdup(form->action); handle_redirect(vpninfo); @@ -1552,3 +1600,250 @@ out: return result; } + +/** + * Multiple certificate-based authentication + * + * Two certificates are employed: a "machine" certificate and a + * "user" certificate. The machine certificate is used to establish + * the TLS session. The user certificate is used to sign a challenge. + * + * An example XML exchange follows: + * CLIENT + * + * + * 4.4.01054 + * + * + * + * ANYCONNECT-MCA + * 136775778 + * multiple-cert + * single-sign-on + * 1506879881148 + * + * + * sha256 + * sha384 + * sha512 + * + * FA4003BD87436B227####snip####C138A08FF724F0100015B863F750914839EE79C86DFE8F0B9A0199E2 + * + * + * + * CLIENT + * + * + * 4.4.01054 + * + * + * ANYCONNECT-MCA + * 608423386 + * multiple-cert + * single-sign-on + * 1506879881148 + * + * + * + * + * MIIG+AYJKoZIhvcNAQcCoIIG6TCCBuU + * yTCCAzwwggIkAgkApaQuJKNF4RowDQYJKoZIhvcNAQELBQAwWTELMAkGA1UEBhMC + * #Snip# + * gSCx8Luo9V76nPjDI8PORurSFVWL9jiGJH0rLakYoGv + * + * FIYur1Dzb4VPThVZtYwxSsCVRBUin/8MwWK+G5u2Phr4fJ + * #snip# + * EYt4G2hQ4hySySYqD4L4iV91uCT5b5Bmr5HZmSqKehg0zrDBjqxx7CLMSf2pSmQnjMwi6D0ygT= + * + * + * + */ + + /** + * Announce our authentication capabilities + */ +int announce_auth_methods(struct openconnect_info *vpninfo, + xmlNodePtr root) +{ + xmlNodePtr node; + + if (vpninfo->cert2 != NULL) { + node = xmlNewChild(root, NULL, XCAST("capabilities"), NULL); + if (node) + node = xmlNewTextChild(node, NULL, XCAST("auth-method"), + XCAST("multiple-cert")); + if (!node) + goto bad; + } + + return 0; + + bad: + + return -ENOMEM; +} + +struct multicert_digest_entry { + const xmlChar *name; + int id; +}; + +static const struct multicert_digest_entry multicert_digests[] = { + { XCAST("sha512"), CERT_AUTH_DIGEST_SHA512 }, + { XCAST("sha384"), CERT_AUTH_DIGEST_SHA384 }, + { XCAST("sha256"), CERT_AUTH_DIGEST_SHA256 }, + { NULL, CERT_AUTH_DIGEST_UNKNOWN }, +}; + +static int multicert_digest_by_name(const xmlChar *name) +{ + const struct multicert_digest_entry *entry; + int id = CERT_AUTH_DIGEST_UNKNOWN; + + for (entry = multicert_digests; entry->name != NULL; entry++) { + if (xmlStrcasecmp(entry->name, name) == 0) { + id = entry->id; + break; + } + } + return id; +} + +static const xmlChar *multicert_digest_by_id(int id) +{ + const struct multicert_digest_entry *entry; + const xmlChar *ret = NULL; + + for (entry = multicert_digests; entry->name != NULL; entry++) { + if (entry->id == id) { + ret = entry->name; + break; + } + } + + return ret; +} + +void parse_multicert_request(struct openconnect_info *vpninfo, + xmlNodePtr node, int *cert_rq) +{ + xmlNodePtr child; + xmlChar *content; + int digest; + + /* node is a multiple-client-cert-request element */ + for (child = node->children; child; child = child->next) { + if (child->type != XML_ELEMENT_NODE) + continue; + + if (xmlStrcmp(child->name, XCAST("hash-algorithm")) != 0) + continue; + + content = xmlNodeGetContent(child); + + digest = multicert_digest_by_name(content); + /* not found */ + if (digest == CERT_AUTH_DIGEST_UNKNOWN) + vpn_progress(vpninfo, PRG_INFO, + _("Algorithm '%s' is unknown.\n"), + (char *)content); + else + *cert_rq |= digest; + xmlFree(content); + } +} + +int multicert_response(struct openconnect_info *vpninfo, + int cert_rq, const char *challenge, struct oc_text_buf *body) +{ + xmlDocPtr doc = NULL; + xmlNodePtr root, auth, node, chain; + const xmlChar *digest_name; + char *identity = NULL; + struct challenge_response resp = {0}; + int ret; + + if ((cert_rq & CERT_AUTH_DIGEST_MASK) == 0) { + vpn_progress(vpninfo, PRG_ERR, + _("Couldn't agree on signature algorithm")); + return xmlpost_initial_req(vpninfo, body, 1); + } + + ret = cert_auth_challenge_response(vpninfo, cert_rq, challenge, + &identity, &resp); + if (ret < 0) + return xmlpost_initial_req(vpninfo, body, 1); + + digest_name = multicert_digest_by_id(resp.digest); + if (digest_name == NULL) + goto bad; + + doc = xmlpost_new_query(vpninfo, "auth-reply", &root); + if (!doc) + goto bad; + + node = xmlNewChild(root, NULL, XCAST("session-token"), NULL); + if (!node) + goto bad; + + node = xmlNewChild(root, NULL, XCAST("session-id"), NULL); + if (!node) + goto bad; + + if (vpninfo->opaque_srvdata != NULL) { + node = xmlCopyNode(vpninfo->opaque_srvdata, 1); + if (!node || !xmlAddChild(root, node)) + goto bad; + } + // key1 ownership is proved by TLS session + auth = xmlNewChild(root, NULL, XCAST("auth"), NULL); + if (!auth) + goto bad; + + chain = xmlNewChild(auth, NULL, XCAST("client-cert-chain"), NULL); + if (!chain || !xmlNewProp(chain, XCAST("cert-store"), XCAST("1M"))) + goto bad; + + if (!xmlNewChild(chain, NULL, XCAST("client-cert-sent-via-protocol"), + NULL)) + goto bad; + // key2 ownership is proved by signing the challenge + chain = xmlNewChild(auth, NULL, XCAST("client-cert-chain"), NULL); + if (!chain || !xmlNewProp(chain, XCAST("cert-store"), XCAST("1U"))) + goto bad; + + node = xmlNewTextChild(chain, NULL, XCAST("client-cert"), + XCAST(identity)); + if (!node || !xmlNewProp(node, XCAST("cert-format"), XCAST("pkcs7"))) + goto bad; + + node = xmlNewTextChild(chain, NULL, + XCAST("client-cert-auth-signature"), XCAST(resp.data)); + if (!node || !xmlNewProp(node, XCAST("hash-algorithm-chosen"), + digest_name)) + goto bad; + + free(identity); free(resp.data); + return xmlpost_complete(doc, body); + + bad: + free(identity); free(resp.data); + xmlpost_complete(doc, NULL); + return -ENOMEM; +} diff --git a/main.c b/main.c index cc3dd91ee..a4a21f977 100644 --- a/main.c +++ b/main.c @@ -194,6 +194,9 @@ enum { OPT_PROTOCOL, OPT_PASSTOS, OPT_VERSION, + OPT_MULTICERT_CERT2, + OPT_MULTICERT_KEY2, + OPT_MULTICERT_KEY2_PASSWORD, }; #ifdef __sun__ @@ -285,6 +288,11 @@ static const struct option long_options[] = { #elif defined(OPENCONNECT_OPENSSL) OPTION("openssl-ciphers", 1, OPT_CIPHERSUITES), #endif +#if ENABLE_MULTICERT > 0 + OPTION("cert2", 1, OPT_MULTICERT_CERT2), + OPTION("key2", 1, OPT_MULTICERT_KEY2), + OPTION("key2-password", 1, OPT_MULTICERT_KEY2_PASSWORD), +#endif /* ENABLE_MULTICERT */ OPTION(NULL, 0, 0) }; @@ -832,6 +840,16 @@ static void usage(void) printf(" %s\n", _("(NOTE: Yubikey OATH disabled in this build)")); #endif +#if ENABLE_MULTICERT > 0 + printf("\n%s:\n", _("Multiple certificate-based authentication")); + printf(" --cert2=CERT2 %s\n", + _("User certificate CERT2")); + printf(" --key2=KEY2 %s\n", + _("User key KEY2")); + printf(" --key2-password=PASS2 %s\n", + _("Passpharse PASS2 for CERT2/KEY2")); +#endif /* ENABLE_MULTICERT > 0 */ + printf("\n%s:\n", _("Server validation")); printf(" --servercert=FINGERPRINT %s\n", _("Server's certificate SHA1 fingerprint")); printf(" --no-cert-check %s\n", _("Do not require server SSL cert to be valid")); @@ -1801,6 +1819,18 @@ int main(int argc, char **argv) strncpy(vpninfo->ciphersuite_config, config_arg, sizeof(vpninfo->ciphersuite_config) - 1); break; + case OPT_MULTICERT_CERT2: + free(vpninfo->cert2); + vpninfo->cert2 = dup_config_arg(); + break; + case OPT_MULTICERT_KEY2: + free(vpninfo->key2); + vpninfo->key2 = dup_config_arg(); + break; + case OPT_MULTICERT_KEY2_PASSWORD: + free(vpninfo->key2_password); + vpninfo->key2_password = dup_config_arg(); + break; default: usage(); } diff --git a/openconnect-internal.h b/openconnect-internal.h index b3806136e..c97f4aa31 100644 --- a/openconnect-internal.h +++ b/openconnect-internal.h @@ -752,6 +752,10 @@ struct openconnect_info { #define vpn_perror(vpninfo, msg) vpn_progress((vpninfo), PRG_ERR, "%s: %s\n", (msg), strerror(errno)) /* certificate authentication */ +#if !defined(ENABLE_MULTICERT) && defined(OPENCONNECT_GNUTLS) +# define ENABLE_MULTICERT 1 +#endif + typedef enum { CERT_AUTH_REQ_CERT1 = (1U<<0), #define CERT_AUTH_REQ_CERT1 CERT_AUTH_REQ_CERT1 -- GitLab From 6b6fca249e9fe1121b1bdbe51ea4f4626ca55062 Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Thu, 23 Apr 2020 23:15:59 -0700 Subject: [PATCH 16/21] Announce multiple cert auth in get_protocols Signed-off-by: Tom Carroll --- library.c | 2 +- openconnect.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/library.c b/library.c index 5d89f4d1d..75d6f3f56 100644 --- a/library.c +++ b/library.c @@ -115,7 +115,7 @@ static const struct vpn_proto openconnect_protos[] = { .name = "anyconnect", .pretty_name = N_("Cisco AnyConnect or openconnect"), .description = N_("Compatible with Cisco AnyConnect SSL VPN, as well as ocserv"), - .flags = OC_PROTO_PROXY | OC_PROTO_CSD | OC_PROTO_AUTH_CERT | OC_PROTO_AUTH_OTP | OC_PROTO_AUTH_STOKEN, + .flags = OC_PROTO_PROXY | OC_PROTO_CSD | OC_PROTO_AUTH_CERT | OC_PROTO_AUTH_OTP | OC_PROTO_AUTH_STOKEN | OC_PROTO_AUTH_MULTI_CERT, .vpn_close_session = cstp_bye, .tcp_connect = cstp_connect, .tcp_mainloop = cstp_mainloop, diff --git a/openconnect.h b/openconnect.h index d1c3d74a1..d3ece5e18 100644 --- a/openconnect.h +++ b/openconnect.h @@ -186,6 +186,7 @@ extern "C" { #define OC_PROTO_AUTH_OTP (1<<3) #define OC_PROTO_AUTH_STOKEN (1<<4) #define OC_PROTO_PERIODIC_TROJAN (1<<4) +#define OC_PROTO_AUTH_MULTI_CERT (1<<5) struct oc_vpn_proto { const char *name; -- GitLab From e13a2edd2ab1258ac76747a2903a6aa407b276d8 Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Thu, 23 Apr 2020 23:15:59 -0700 Subject: [PATCH 17/21] Check key purpose satisfies authentication Signed-off-by: Tom Carroll --- gnutls.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/gnutls.c b/gnutls.c index 2a8060ac3..c747962bd 100644 --- a/gnutls.c +++ b/gnutls.c @@ -2070,6 +2070,67 @@ static int load_certificate(struct openconnect_info *vpninfo) return ret; } +/** + * check if the key purpose is compatible with authentication. + * If not warn. + */ +static unsigned int check_key_purpose(struct openconnect_info *vpninfo, + gnutls_x509_crt_t crt) +{ +#define MAX_OID 128 + char oid[MAX_OID]; + unsigned int i, usage, critical; + int gerr; + + /** + * extendedKeyUsage of either clientAuth or msSmartcardLogin + * would satify the purpose of authentication. + */ + for (i = 0; ; i++) { + size_t oid_size = sizeof oid; + + gerr = gnutls_x509_crt_get_key_purpose_oid(crt, i, oid, + &oid_size, &critical); + if (gerr < 0) { + if (gerr != GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) + vpn_progress(vpninfo, PRG_DEBUG, + _("gnutls_x509_crt_get_key_purpose_oid: (%d) %s\n"), + gerr, gnutls_strerror(gerr)); + break; + } +#ifndef GNUTLS_KP_TLS_WWW_CLIENT +# define GNUTLS_KP_TLS_WWW_CLIENT "1.3.6.1.5.5.7.3.2" +#endif +#ifndef GNUTLS_KP_MS_SMART_CARD_LOGON +# define GNUTLS_KP_MS_SMART_CARD_LOGON "1.3.6.1.4.1.311.20.2.2" +#endif + if (strcmp(oid, GNUTLS_KP_TLS_WWW_CLIENT) == 0 + || strcmp(oid, GNUTLS_KP_MS_SMART_CARD_LOGON) == 0) + return 1; + } + + /** + * keyUsage of digitalSignature or nonRepudiation would also + * work, too + */ + gerr = gnutls_x509_crt_get_key_usage(crt, &usage, &critical); + if (gerr < 0) { + if (gerr != GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) + vpn_progress(vpninfo, PRG_DEBUG, + _("gnutls_X509_crt_get_key_usge: (%d) %s\n"), + gerr, gnutls_strerror(gerr)); + usage = 0; + } + + if (usage & (GNUTLS_KEY_DIGITAL_SIGNATURE|GNUTLS_KEY_NON_REPUDIATION)) + return 1; + + vpn_progress(vpninfo, PRG_INFO, + _("Key doesn't specify a usage or purpose" + " compatible with authentication\n")); + return 0; +} + static int pem_encode(const gnutls_datum_t *data, gnutls_datum_t *result) { @@ -2209,6 +2270,8 @@ int cert_auth_challenge_response(struct openconnect_info *vpninfo, pk = gnutls_pubkey_get_pk_algorithm(pubkey, NULL); + check_key_purpose(vpninfo, chain[0]); + /* Export identity as PKCS7 */ err = gnutls_pkcs7_init(&p7); if (err < 0) -- GitLab From 801b9b2a25dbe7ed278ebbc795f3f16ee0f339eb Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Thu, 23 Apr 2020 23:15:59 -0700 Subject: [PATCH 18/21] Stub openssl challenge_response functions. Signed-off-by: Tom Carroll --- openssl.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/openssl.c b/openssl.c index b15f2f6f2..f028f89e1 100644 --- a/openssl.c +++ b/openssl.c @@ -2139,3 +2139,20 @@ void destroy_eap_ttls(struct openconnect_info *vpninfo, void *ttls) /* Leave the BIO_METH for now. It may get reused and we don't want to * have to call BIO_get_new_index() more times than is necessary */ } + +int cert_auth_challenge_response(struct openconnect_info *vpninfo, + int cert_rq, const char *challenge, char **identity, + struct challenge_response *response) +{ + /* hush not used warnings */ + (void) cert_rq; + (void) challenge; + (void) identity; + (void) response; + + vpn_progress(vpninfo, PRG_ERR, + _("Multiple certificate-based authentication" + " is not implemented for OpenSSL\n")); + + return -ENOSYS; +} -- GitLab From 620584124f6132c16c111b42d98606a085fb2412 Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Sun, 26 Apr 2020 17:01:50 -0700 Subject: [PATCH 19/21] Check calloc and gnutls_privkey_init status Signed-off-by: Tom Carroll --- gnutls_tpm.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/gnutls_tpm.c b/gnutls_tpm.c index 01abc3f04..d0ce1fa6e 100644 --- a/gnutls_tpm.c +++ b/gnutls_tpm.c @@ -87,9 +87,10 @@ static int tpm_sign_fn(gnutls_privkey_t key, void *_vpninfo, int load_tpm1_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, const char *password, - gnutls_privkey_t *pkey, gnutls_datum_t *pkey_sig) + gnutls_privkey_t *pkey_, gnutls_datum_t *pkey_sig) { static const TSS_UUID SRK_UUID = TSS_UUID_SRK; + gnutls_privkey_t pkey = NULL; gnutls_datum_t asn1; unsigned int tss_len; char *pass; @@ -102,14 +103,21 @@ int load_tpm1_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, return -EBUSY; } + vpninfo->tpm1 = calloc(1, sizeof(*vpninfo->tpm1)); + if (vpninfo->tpm1 == NULL) { + vpn_progress(vpninfo, PRG_ERR, + _("Out of memory.\n")); + return -ENOMEM; + } + err = gnutls_pem_base64_decode_alloc("TSS KEY BLOB", fdata, &asn1); if (err) { vpn_progress(vpninfo, PRG_ERR, _("Error decoding TSS key blob: %s\n"), gnutls_strerror(err)); + free(vpninfo->tpm1); return -EINVAL; } - vpninfo->tpm1 = calloc(1, sizeof(*vpninfo->tpm1)); /* Ick. We have to parse the ASN1 OCTET_STRING for ourselves. */ if (asn1.size < 2 || asn1.data[0] != 0x04 /* OCTET_STRING */) { vpn_progress(vpninfo, PRG_ERR, @@ -221,14 +229,21 @@ int load_tpm1_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, goto out_srkpol; } - gnutls_privkey_init(pkey); + err = gnutls_privkey_init(&pkey); /* This would be nicer if there was a destructor callback. I could allocate a data structure with the TPM handles and the vpninfo pointer, and destroy that properly when the key is destroyed. */ - gnutls_privkey_import_ext(*pkey, GNUTLS_PK_RSA, vpninfo, tpm_sign_fn, NULL, 0); + if (err >= 0) + err = gnutls_privkey_import_ext(pkey, GNUTLS_PK_RSA, vpninfo, tpm_sign_fn, NULL, 0); + if (err < 0) { + vpn_progress(vpninfo, PRG_ERR, + _("Error initialising private key structure: (%d) %s\n"), + err, gnutls_strerror(err)); + goto out_key; + } retry_sign: - err = gnutls_privkey_sign_data(*pkey, GNUTLS_DIG_SHA1, 0, fdata, pkey_sig); + err = gnutls_privkey_sign_data(pkey, GNUTLS_DIG_SHA1, 0, fdata, pkey_sig); if (err == GNUTLS_E_INSUFFICIENT_CREDENTIALS) { if (!vpninfo->tpm1->tpm_key_policy) { err = Tspi_Context_CreateObject(vpninfo->tpm1->tpm_context, @@ -270,11 +285,13 @@ int load_tpm1_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, } free(asn1.data); + *pkey_ = pkey; return 0; out_key_policy: Tspi_Context_CloseObject(vpninfo->tpm1->tpm_context, vpninfo->tpm1->tpm_key_policy); vpninfo->tpm1->tpm_key_policy = 0; out_key: + gnutls_privkey_deinit(pkey); Tspi_Context_CloseObject(vpninfo->tpm1->tpm_context, vpninfo->tpm1->tpm_key); vpninfo->tpm1->tpm_key = 0; out_srkpol: -- GitLab From 3a93d9b28a557964bdcedff6e25bb8b5b5c8a830 Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Sat, 2 May 2020 21:38:11 -0700 Subject: [PATCH 20/21] Provide password to load_tpm2_key Permit a single instance of tpm2. Return -EBUSY to inform of caller of existing instance. Duplicate password when necessary. Signed-off-by: Tom Carroll --- gnutls.c | 2 +- gnutls.h | 5 ++++- gnutls_tpm2.c | 10 +++++++++- gnutls_tpm2_esys.c | 19 ++++++++++++++----- gnutls_tpm2_ibm.c | 6 +++++- 5 files changed, 33 insertions(+), 9 deletions(-) diff --git a/gnutls.c b/gnutls.c index c747962bd..8c408c427 100644 --- a/gnutls.c +++ b/gnutls.c @@ -1679,7 +1679,7 @@ static int load_keycert(struct openconnect_info *vpninfo, _("This version of OpenConnect was built without TPM2 support\n")); return -EINVAL; #else - ret = load_tpm2_key(vpninfo, &fdata, &pkey, &pkey_sig); + ret = load_tpm2_key(vpninfo, &fdata, password, &pkey, &pkey_sig); if (ret) goto out; diff --git a/gnutls.h b/gnutls.h index 6d39f9be9..30c99020a 100644 --- a/gnutls.h +++ b/gnutls.h @@ -30,9 +30,12 @@ int load_tpm1_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, void release_tpm1_ctx(struct openconnect_info *info); int load_tpm2_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, + const char *password, gnutls_privkey_t *pkey, gnutls_datum_t *pkey_sig); void release_tpm2_ctx(struct openconnect_info *info); -int install_tpm2_key(struct openconnect_info *vpninfo, gnutls_privkey_t *pkey, gnutls_datum_t *pkey_sig, +int install_tpm2_key(struct openconnect_info *vpninfo, + const char *password, + gnutls_privkey_t *pkey, gnutls_datum_t *pkey_sig, unsigned int parent, int emptyauth, int legacy, gnutls_datum_t *privdata, gnutls_datum_t *pubdata); diff --git a/gnutls_tpm2.c b/gnutls_tpm2.c index 698a9e2ed..29b17e8a6 100644 --- a/gnutls_tpm2.c +++ b/gnutls_tpm2.c @@ -175,6 +175,7 @@ static int decode_data(ASN1_TYPE n, gnutls_datum_t *r) } int load_tpm2_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, + const char *password, gnutls_privkey_t *pkey, gnutls_datum_t *pkey_sig) { gnutls_datum_t asn1, pubdata, privdata; @@ -187,6 +188,12 @@ int load_tpm2_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, int err, ret = -EINVAL; const asn1_static_node *asn1tab; + if (vpninfo->tpm2) { + vpn_progress(vpninfo, PRG_ERR, + _("TPM2 is in use.\n")); + return -EBUSY; + } + err = gnutls_pem_base64_decode_alloc("TSS2 PRIVATE KEY", fdata, &asn1); if (!err) { asn1tab = tpmkey_asn1_tab; @@ -282,7 +289,8 @@ int load_tpm2_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, /* Now we've extracted what we need from the ASN.1, invoke the * actual TPM2 code (whichever implementation we end up with */ - ret = install_tpm2_key(vpninfo, pkey, pkey_sig, parent, emptyauth, + ret = install_tpm2_key(vpninfo, password, + pkey, pkey_sig, parent, emptyauth, asn1tab == tpmkey_asn1_tab_old, &privdata, &pubdata); if (ret < 0) goto out_tpmkey; diff --git a/gnutls_tpm2_esys.c b/gnutls_tpm2_esys.c index c5eabd823..746eea1c7 100644 --- a/gnutls_tpm2_esys.c +++ b/gnutls_tpm2_esys.c @@ -65,6 +65,7 @@ struct oc_tpm2_ctx { TPM2B_PRIVATE priv; TPM2B_DIGEST userauth; TPM2B_DIGEST ownerauth; + char *key_pass; unsigned int need_userauth:1; unsigned int need_ownerauth:1; unsigned int did_ownerauth:1; @@ -356,12 +357,12 @@ static int auth_tpm2_key(struct openconnect_info *vpninfo, ESYS_CONTEXT *ctx, ES { TSS2_RC r; - if (vpninfo->tpm2->need_userauth || vpninfo->cert_password) { + if (vpninfo->tpm2->need_userauth || vpninfo->tpm2->key_pass) { char *pass = NULL; - if (vpninfo->cert_password) { - pass = vpninfo->cert_password; - vpninfo->cert_password = NULL; + if (vpninfo->tpm2->key_pass) { + pass = vpninfo->tpm2->key_pass; + vpninfo->tpm2->key_pass = NULL; } else { int err = request_passphrase(vpninfo, "openconnect_tpm2_key", &pass, _("Enter TPM2 key password:")); @@ -524,7 +525,9 @@ int tpm2_ec_sign_hash_fn(gnutls_privkey_t key, gnutls_sign_algorithm_t algo, return ret; } -int install_tpm2_key(struct openconnect_info *vpninfo, gnutls_privkey_t *pkey, gnutls_datum_t *pkey_sig, +int install_tpm2_key(struct openconnect_info *vpninfo, + const char *password, + gnutls_privkey_t *pkey, gnutls_datum_t *pkey_sig, unsigned int parent, int emptyauth, int legacy, gnutls_datum_t *privdata, gnutls_datum_t *pubdata) { TSS2_RC r; @@ -541,6 +544,11 @@ int install_tpm2_key(struct openconnect_info *vpninfo, gnutls_privkey_t *pkey, g if (!vpninfo->tpm2) return -ENOMEM; + if (password && (vpninfo->tpm2->key_pass = strdup(password)) == NULL) { + free(vpninfo->tpm2); + return -ENOMEM; + } + vpninfo->tpm2->parent = parent; r = Tss2_MU_TPM2B_PRIVATE_Unmarshal(privdata->data, privdata->size, NULL, @@ -583,6 +591,7 @@ void release_tpm2_ctx(struct openconnect_info *vpninfo) if (vpninfo->tpm2) { clear_mem(vpninfo->tpm2->ownerauth.buffer, sizeof(vpninfo->tpm2->ownerauth.buffer)); clear_mem(vpninfo->tpm2->userauth.buffer, sizeof(vpninfo->tpm2->userauth.buffer)); + free_pass(&vpninfo->tpm2->key_pass); free(vpninfo->tpm2); } vpninfo->tpm2 = NULL; diff --git a/gnutls_tpm2_ibm.c b/gnutls_tpm2_ibm.c index 3fe33b6c7..c86d13a34 100644 --- a/gnutls_tpm2_ibm.c +++ b/gnutls_tpm2_ibm.c @@ -470,7 +470,9 @@ int tpm2_ec_sign_hash_fn(gnutls_privkey_t key, gnutls_sign_algorithm_t algo, return ret; } -int install_tpm2_key(struct openconnect_info *vpninfo, gnutls_privkey_t *pkey, gnutls_datum_t *pkey_sig, +int install_tpm2_key(struct openconnect_info *vpninfo, + const char *password, + gnutls_privkey_t *pkey, gnutls_datum_t *pkey_sig, unsigned int parent, int emptyauth, int legacy, gnutls_datum_t *privdata, gnutls_datum_t *pubdata) { @@ -478,6 +480,8 @@ int install_tpm2_key(struct openconnect_info *vpninfo, gnutls_privkey_t *pkey, g BYTE *der; INT32 dersize; + (void) password; + if (!parent_is_persistent(parent) && parent != TPM_RH_OWNER && parent != TPM_RH_NULL && parent != TPM_RH_ENDORSEMENT && parent != TPM_RH_PLATFORM) { -- GitLab From 2ee9d0c1972dddd7e494d90d9ee1ec87593e2398 Mon Sep 17 00:00:00 2001 From: Tom Carroll Date: Sat, 2 May 2020 21:59:36 -0700 Subject: [PATCH 21/21] Check status of gnutls_privkey_init and related Signed-off-by: Tom Carroll --- gnutls_tpm2.c | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/gnutls_tpm2.c b/gnutls_tpm2.c index 29b17e8a6..17dba463e 100644 --- a/gnutls_tpm2.c +++ b/gnutls_tpm2.c @@ -176,11 +176,12 @@ static int decode_data(ASN1_TYPE n, gnutls_datum_t *r) int load_tpm2_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, const char *password, - gnutls_privkey_t *pkey, gnutls_datum_t *pkey_sig) + gnutls_privkey_t *pkey_, gnutls_datum_t *pkey_sig) { gnutls_datum_t asn1, pubdata, privdata; ASN1_TYPE tpmkey_def = ASN1_TYPE_EMPTY, tpmkey = ASN1_TYPE_EMPTY; const char *oid = NULL; + gnutls_privkey_t pkey = NULL; char value_buf[16]; int value_buflen; int emptyauth = 0; @@ -290,34 +291,46 @@ int load_tpm2_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata, /* Now we've extracted what we need from the ASN.1, invoke the * actual TPM2 code (whichever implementation we end up with */ ret = install_tpm2_key(vpninfo, password, - pkey, pkey_sig, parent, emptyauth, + &pkey, pkey_sig, parent, emptyauth, asn1tab == tpmkey_asn1_tab_old, &privdata, &pubdata); if (ret < 0) goto out_tpmkey; - gnutls_privkey_init(pkey); - - switch(ret) { - case GNUTLS_PK_RSA: + err = gnutls_privkey_init(&pkey); + if (err >= 0) { + switch(ret) { + case GNUTLS_PK_RSA: #if GNUTLS_VERSION_NUMBER >= 0x030600 - gnutls_privkey_import_ext4(*pkey, vpninfo, NULL, tpm2_rsa_sign_hash_fn, NULL, NULL, rsa_key_info, 0); + err = gnutls_privkey_import_ext4(pkey, vpninfo, NULL, tpm2_rsa_sign_hash_fn, NULL, NULL, rsa_key_info, 0); #else - gnutls_privkey_import_ext(*pkey, GNUTLS_PK_RSA, vpninfo, tpm2_rsa_sign_fn, NULL, 0); + err = gnutls_privkey_import_ext(pkey, GNUTLS_PK_RSA, vpninfo, tpm2_rsa_sign_fn, NULL, 0); #endif - break; + break; - case GNUTLS_PK_ECC: + case GNUTLS_PK_ECC: #if GNUTLS_VERSION_NUMBER >= 0x030600 - gnutls_privkey_import_ext4(*pkey, vpninfo, NULL, tpm2_ec_sign_hash_fn, NULL, NULL, ec_key_info, 0); + err = gnutls_privkey_import_ext4(pkey, vpninfo, NULL, tpm2_ec_sign_hash_fn, NULL, NULL, ec_key_info, 0); #elif GNUTLS_VERSION_NUMBER >= 0x030400 - gnutls_privkey_import_ext3(*pkey, vpninfo, tpm2_ec_sign_fn, NULL, NULL, ec_key_info, 0); + err = gnutls_privkey_import_ext3(pkey, vpninfo, tpm2_ec_sign_fn, NULL, NULL, ec_key_info, 0); #else - gnutls_privkey_import_ext(*pkey, GNUTLS_PK_EC, vpninfo, tpm2_ec_sign_fn, NULL, 0); + err = gnutls_privkey_import_ext(pkey, GNUTLS_PK_EC, vpninfo, tpm2_ec_sign_fn, NULL, 0); #endif - break; + break; + } + } + + if (err < 0) { + vpn_progress(vpninfo, PRG_ERR, + _("Error initialising private key structure: (%d) %s\n"), + err, gnutls_strerror(err)); + goto out_privkey; } + + *pkey_ = pkey, pkey = NULL; ret = 0; + out_privkey: + gnutls_privkey_deinit(pkey); out_tpmkey: asn1_delete_structure(&tpmkey); asn1_delete_structure(&tpmkey_def); -- GitLab