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 <gerow@google.com>

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
+ * TLS_SET_PARAMS_ENGINE_PRV_INIT_FAILED (-2) on error causing PKCS#11 engine
- * PKCS#11 engine failure, or
+ * 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
+ * TLS_SET_PARAMS_ENGINE_PRV_INIT_FAILED (-2) on error causing PKCS#11 engine
- * PKCS#11 engine failure, or
+ * 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);

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));
+				   ERR_error_string(err, NULL));
-			ret = TLS_SET_PARAMS_ENGINE_PRV_INIT_FAILED;
+			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, &params, 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) {

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.
+		 * At this point with the pkcs11 engine the PIN is wrong. We
-		 * We reset the PIN in the configuration to be sure to not use
+		 * reset the PIN in the configuration to be sure to not use it
-		 * it again and the calling function must request a new one.
+		 * 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.
 		 */
+		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);
+	} else if (res == TLS_SET_PARAMS_ENGINE_PRV_INIT_FAILED) {
-		data->conn = NULL;
+		wpa_printf(MSG_INFO, "TLS: Failed to initialize engine");
-		return -1;
+	} else if (res == TLS_SET_PARAMS_ENGINE_PRV_VERIFY_FAILED) {
-	} else if (res) {
+		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);