From fd4fb28179a0b750dff4d38a72a7bf89a2c49813 Mon Sep 17 00:00:00 2001 From: Mike Gerow Date: Wed, 15 Apr 2015 17:57:51 -0700 Subject: [PATCH] OpenSSL: Try to ensure we don't throw away the PIN unnecessarily Now on an engine error we decode the error value and determine if the issue is due to a true PIN error or not. If it is due to incorrrect PIN, delete the PIN as usual, but if it isn't let the PIN be. Signed-off-by: Mike Gerow --- src/crypto/tls.h | 17 +++++++++++------ src/crypto/tls_openssl.c | 36 ++++++++++++++++++++++++++++++++--- src/eap_peer/eap_tls_common.c | 29 +++++++++++++--------------- 3 files changed, 57 insertions(+), 25 deletions(-) diff --git a/src/crypto/tls.h b/src/crypto/tls.h index f9e2e10ef..d13657e16 100644 --- a/src/crypto/tls.h +++ b/src/crypto/tls.h @@ -253,6 +253,7 @@ int tls_connection_established(void *tls_ctx, struct tls_connection *conn); int tls_connection_shutdown(void *tls_ctx, struct tls_connection *conn); enum { + TLS_SET_PARAMS_ENGINE_PRV_BAD_PIN = -4, TLS_SET_PARAMS_ENGINE_PRV_VERIFY_FAILED = -3, TLS_SET_PARAMS_ENGINE_PRV_INIT_FAILED = -2 }; @@ -263,10 +264,12 @@ enum { * @conn: Connection context data from tls_connection_init() * @params: Connection parameters * Returns: 0 on success, -1 on failure, - * TLS_SET_PARAMS_ENGINE_PRV_INIT_FAILED (-2) on possible PIN error causing - * PKCS#11 engine failure, or + * TLS_SET_PARAMS_ENGINE_PRV_INIT_FAILED (-2) on error causing PKCS#11 engine + * failure, or * TLS_SET_PARAMS_ENGINE_PRV_VERIFY_FAILED (-3) on failure to verify the - * PKCS#11 engine private key. + * PKCS#11 engine private key, or + * TLS_SET_PARAMS_ENGINE_PRV_BAD_PIN (-4) on PIN error causing PKCS#11 engine + * failure. */ int __must_check tls_connection_set_params(void *tls_ctx, struct tls_connection *conn, @@ -277,10 +280,12 @@ tls_connection_set_params(void *tls_ctx, struct tls_connection *conn, * @tls_ctx: TLS context data from tls_init() * @params: Global TLS parameters * Returns: 0 on success, -1 on failure, - * TLS_SET_PARAMS_ENGINE_PRV_INIT_FAILED (-2) on possible PIN error causing - * PKCS#11 engine failure, or + * TLS_SET_PARAMS_ENGINE_PRV_INIT_FAILED (-2) on error causing PKCS#11 engine + * failure, or * TLS_SET_PARAMS_ENGINE_PRV_VERIFY_FAILED (-3) on failure to verify the - * PKCS#11 engine private key. + * PKCS#11 engine private key, or + * TLS_SET_PARAMS_ENGINE_PRV_BAD_PIN (-4) on PIN error causing PKCS#11 engine + * failure. */ int __must_check tls_global_set_params( void *tls_ctx, const struct tls_connection_params *params); diff --git a/src/crypto/tls_openssl.c b/src/crypto/tls_openssl.c index d3e9eb931..5d4795a12 100644 --- a/src/crypto/tls_openssl.c +++ b/src/crypto/tls_openssl.c @@ -885,6 +885,27 @@ void tls_deinit(void *ssl_ctx) } +#ifndef OPENSSL_NO_ENGINE + +/* Cryptoki return values */ +#define CKR_PIN_INCORRECT 0x000000a0 +#define CKR_PIN_INVALID 0x000000a1 +#define CKR_PIN_LEN_RANGE 0x000000a2 + +/* libp11 */ +#define ERR_LIB_PKCS11 ERR_LIB_USER + +static int tls_is_pin_error(unsigned int err) +{ + return ERR_GET_LIB(err) == ERR_LIB_PKCS11 && + (ERR_GET_REASON(err) == CKR_PIN_INCORRECT || + ERR_GET_REASON(err) == CKR_PIN_INVALID || + ERR_GET_REASON(err) == CKR_PIN_LEN_RANGE); +} + +#endif /* OPENSSL_NO_ENGINE */ + + static int tls_engine_init(struct tls_connection *conn, const char *engine_id, const char *pin, const char *key_id, const char *cert_id, const char *ca_cert_id) @@ -936,11 +957,16 @@ static int tls_engine_init(struct tls_connection *conn, const char *engine_id, key_id, NULL, &key_cb); if (!conn->private_key) { + unsigned long err = ERR_get_error(); + wpa_printf(MSG_ERROR, "ENGINE: cannot load private key with id '%s' [%s]", key_id, - ERR_error_string(ERR_get_error(), NULL)); - ret = TLS_SET_PARAMS_ENGINE_PRV_INIT_FAILED; + ERR_error_string(err, NULL)); + if (tls_is_pin_error(err)) + ret = TLS_SET_PARAMS_ENGINE_PRV_BAD_PIN; + else + ret = TLS_SET_PARAMS_ENGINE_PRV_INIT_FAILED; goto err; } } @@ -2184,9 +2210,13 @@ static int tls_engine_get_cert(struct tls_connection *conn, if (!ENGINE_ctrl_cmd(conn->engine, "LOAD_CERT_CTRL", 0, ¶ms, NULL, 1)) { + unsigned long err = ERR_get_error(); + wpa_printf(MSG_ERROR, "ENGINE: cannot load client cert with id" " '%s' [%s]", cert_id, - ERR_error_string(ERR_get_error(), NULL)); + ERR_error_string(err, NULL)); + if (tls_is_pin_error(err)) + return TLS_SET_PARAMS_ENGINE_PRV_BAD_PIN; return TLS_SET_PARAMS_ENGINE_PRV_INIT_FAILED; } if (!params.cert) { diff --git a/src/eap_peer/eap_tls_common.c b/src/eap_peer/eap_tls_common.c index 15c1bac51..b4a5b1f30 100644 --- a/src/eap_peer/eap_tls_common.c +++ b/src/eap_peer/eap_tls_common.c @@ -196,28 +196,25 @@ static int eap_tls_init_connection(struct eap_sm *sm, } res = tls_connection_set_params(data->ssl_ctx, data->conn, params); - if (res == TLS_SET_PARAMS_ENGINE_PRV_INIT_FAILED) { + if (res == TLS_SET_PARAMS_ENGINE_PRV_BAD_PIN) { /* - * At this point with the pkcs11 engine the PIN might be wrong. - * We reset the PIN in the configuration to be sure to not use - * it again and the calling function must request a new one. - */ - os_free(config->pin); - config->pin = NULL; - } else if (res == TLS_SET_PARAMS_ENGINE_PRV_VERIFY_FAILED) { - wpa_printf(MSG_INFO, "TLS: Failed to load private key"); - /* - * We do not know exactly but maybe the PIN was wrong, - * so ask for a new one. + * At this point with the pkcs11 engine the PIN is wrong. We + * reset the PIN in the configuration to be sure to not use it + * again and the calling function must request a new one. */ + wpa_printf(MSG_INFO, + "TLS: Bad PIN provided, requesting a new one"); os_free(config->pin); config->pin = NULL; eap_sm_request_pin(sm); sm->ignore = TRUE; - tls_connection_deinit(data->ssl_ctx, data->conn); - data->conn = NULL; - return -1; - } else if (res) { + } else if (res == TLS_SET_PARAMS_ENGINE_PRV_INIT_FAILED) { + wpa_printf(MSG_INFO, "TLS: Failed to initialize engine"); + } else if (res == TLS_SET_PARAMS_ENGINE_PRV_VERIFY_FAILED) { + wpa_printf(MSG_INFO, "TLS: Failed to load private key"); + sm->ignore = TRUE; + } + if (res) { wpa_printf(MSG_INFO, "TLS: Failed to set TLS connection " "parameters"); tls_connection_deinit(data->ssl_ctx, data->conn);