p11tool: fix initialization of security officer's PINtmp-initialize-so-pin-fix

Previously we would call gnutls_pkcs11_token_set_pin() without an
old PIN provided, which will result to the use of C_InitPIN() on the
underlying module. The C_InitPIN() in contrast with C_SetPIN() will
only work for the user and not for the administrator. As such, we
always provide the oldpin for when we change the admin's PIN.

Resolves #561

Signed-off-by: Nikos Mavrogiannopoulos <nmav@redhat.com>

9 files changed, 264 insertions, 64 deletions

diff --git a/NEWS b/NEWS
index 3b5e914886..4f2cc29e4e 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,9 @@ See the end for copying conditions.
 ** libgnutls: Provide the option of transparent re-handshake/reauthentication
    when the GNUTLS_AUTO_REAUTH flag is specified in gnutls_init().
 
+** p11tool: fix initialization of security officer's PIN with the --initialize-so-pin
+   option (#561)
+
 ** API and ABI modifications:
 GNUTLS_AUTO_REAUTH: Added
 
diff --git a/lib/pkcs11.c b/lib/pkcs11.c
index 9909127903..1a335ea959 100644
--- a/lib/pkcs11.c
+++ b/lib/pkcs11.c
@@ -35,9 +35,7 @@
 
 #include <pin.h>
 #include <pkcs11_int.h>
-#include <p11-kit/p11-kit.h>
 #include "pkcs11x.h"
-#include <p11-kit/pin.h>
 #include <system-keys.h>
 #include "x509/x509_int.h"
 
@@ -2781,10 +2779,10 @@ retrieve_pin_from_callback(const struct pin_info_st *pin_info,
 	return 0;
 }
 
-static int
-retrieve_pin(struct pin_info_st *pin_info, struct p11_kit_uri *info,
-	     struct ck_token_info *token_info, int attempts,
-	     ck_user_type_t user_type, struct p11_kit_pin **pin)
+int
+pkcs11_retrieve_pin(struct pin_info_st *pin_info, struct p11_kit_uri *info,
+		    struct ck_token_info *token_info, int attempts,
+		    ck_user_type_t user_type, struct p11_kit_pin **pin)
 {
 	const char *pinfile;
 	int ret = GNUTLS_E_PKCS11_PIN_ERROR;
@@ -2930,8 +2928,8 @@ pkcs11_login(struct pkcs11_session_info *sinfo,
 		}
 
 		ret =
-		    retrieve_pin(pin_info, info, &tinfo, attempt++,
-				 user_type, &pin);
+		    pkcs11_retrieve_pin(pin_info, info, &tinfo, attempt++,
+					user_type, &pin);
 		if (ret < 0) {
 			gnutls_assert();
 			goto cleanup;
diff --git a/lib/pkcs11_int.h b/lib/pkcs11_int.h
index f52db0780c..76c09b460a 100644
--- a/lib/pkcs11_int.h
+++ b/lib/pkcs11_int.h
@@ -31,6 +31,8 @@
 #define PKCS11_ID_SIZE 128
 #define PKCS11_LABEL_SIZE 128
 
+#include <p11-kit/p11-kit.h>
+#include <p11-kit/pin.h>
 #include <p11-kit/uri.h>
 typedef unsigned char ck_bool_t;
 
@@ -269,6 +271,11 @@ static inline int pk_to_genmech(gnutls_pk_algorithm_t pk, ck_key_type_t *type)
 	}
 }
 
+int
+pkcs11_retrieve_pin(struct pin_info_st *pin_info, struct p11_kit_uri *info,
+		    struct ck_token_info *token_info, int attempts,
+		    ck_user_type_t user_type, struct p11_kit_pin **pin);
+
 ck_object_class_t pkcs11_type_to_class(gnutls_pkcs11_obj_type_t type);
 
 ck_rv_t
diff --git a/lib/pkcs11_write.c b/lib/pkcs11_write.c
index 35207d5543..cb5b65d508 100644
--- a/lib/pkcs11_write.c
+++ b/lib/pkcs11_write.c
@@ -1193,6 +1193,8 @@ gnutls_pkcs11_token_init(const char *token_url,
 
 }
 
+#define L(x) ((x==NULL)?0:strlen(x))
+
 /**
  * gnutls_pkcs11_token_set_pin:
  * @token_url: A PKCS #11 URL specifying a token
@@ -1200,9 +1202,11 @@ gnutls_pkcs11_token_init(const char *token_url,
  * @newpin: new user's PIN
  * @flags: one of #gnutls_pin_flag_t.
  *
- * This function will modify or set a user's PIN for the given token. 
- * If it is called to set a user pin for first time the oldpin must
- * be NULL.
+ * This function will modify or set a user or administrator's PIN for
+ * the given token.  If it is called to set a PIN for first time
+ * the oldpin must be %NULL. When setting the admin's PIN with the
+ * %GNUTLS_PIN_SO flag, the @oldpin value must be provided (this requirement
+ * is relaxed after GnuTLS 3.6.5 since which the PIN will be requested if missing).
  *
  * Returns: On success, %GNUTLS_E_SUCCESS (0) is returned, otherwise a
  *   negative error value.
@@ -1240,7 +1244,8 @@ gnutls_pkcs11_token_set_pin(const char *token_url,
 		return ret;
 	}
 
-	if (oldpin == NULL) {
+	if (oldpin == NULL && !(flags & GNUTLS_PIN_SO)) {
+		/* This changes only the user PIN */
 		rv = pkcs11_init_pin(sinfo.module, sinfo.pks,
 				     (uint8_t *) newpin, strlen(newpin));
 		if (rv != CKR_OK) {
@@ -1251,9 +1256,32 @@ gnutls_pkcs11_token_set_pin(const char *token_url,
 			goto finish;
 		}
 	} else {
+		struct p11_kit_pin *pin;
+		unsigned oldpin_size;
+
+		oldpin_size = L(oldpin);
+
+		if (!(sinfo.tinfo.flags & CKF_PROTECTED_AUTHENTICATION_PATH)) {
+			if (newpin == NULL)
+				return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST);
+
+			if (oldpin == NULL) {
+				struct pin_info_st pin_info;
+				memset(&pin_info, 0, sizeof(pin_info));
+
+				ret = pkcs11_retrieve_pin(&pin_info, info, &sinfo.tinfo, 0, CKU_SO, &pin);
+				if (ret < 0) {
+					gnutls_assert();
+					goto finish;
+				}
+				oldpin = (const char*)p11_kit_pin_get_value(pin, NULL);
+				oldpin_size = p11_kit_pin_get_length(pin);
+			}
+		}
+
 		rv = pkcs11_set_pin(sinfo.module, sinfo.pks,
-				    oldpin, strlen(oldpin),
-				    newpin, strlen(newpin));
+				    oldpin, oldpin_size,
+				    newpin, L(newpin));
 		if (rv != CKR_OK) {
 			gnutls_assert();
 			_gnutls_debug_log("p11: %s\n",
diff --git a/src/common.c b/src/common.c
index d94253311c..852200bde1 100644
--- a/src/common.c
+++ b/src/common.c
@@ -984,6 +984,54 @@ int check_command(gnutls_session_t session, const char *str, unsigned no_cli_cer
 	return 0;
 }
 
+/* error is indicated by returning an empty string */
+void getpass_copy(char *pass, size_t max_pass_size, const char *prompt)
+{
+	char *tmp;
+	size_t len;
+
+	tmp = getpass(prompt);
+	if (tmp == NULL) {
+		pass[0] = 0;
+		return;
+	}
+
+	len = strlen(tmp);
+	if (len >= max_pass_size) {
+		gnutls_memset(tmp, 0, len);
+		pass[0] = 0;
+		return;
+	}
+
+	strcpy(pass, tmp);
+	gnutls_memset(tmp, 0, len);
+
+	return;
+}
+
+/* error is indicated by returning an empty string */
+void getenv_copy(char *str, size_t max_str_size, const char *envvar)
+{
+	char *tmp;
+	size_t len;
+
+	tmp = getenv(envvar);
+	if (tmp == NULL) {
+		str[0] = 0;
+		return;
+	}
+
+	len = strlen(tmp);
+	if (len >= max_str_size) {
+		str[0] = 0;
+		return;
+	}
+
+	strcpy(str, tmp);
+
+	return;
+}
+
 #define MIN(x,y) ((x)<(y))?(x):(y)
 #define MAX_CACHE_TRIES 5
 int
@@ -991,26 +1039,26 @@ pin_callback(void *user, int attempt, const char *token_url,
 	     const char *token_label, unsigned int flags, char *pin,
 	     size_t pin_max)
 {
-	const char *password = NULL;
+	char password[MAX_PIN_LEN] = "";
 	common_info_st *info = user;
 	const char *desc;
 	int cache = MAX_CACHE_TRIES;
 	unsigned len;
 /* allow caching of PIN */
 	static char *cached_url = NULL;
-	static char cached_pin[32] = "";
+	static char cached_pin[MAX_PIN_LEN] = "";
 	const char *env;
 
 	if (flags & GNUTLS_PIN_SO) {
 		env = "GNUTLS_SO_PIN";
 		desc = "security officer";
-		if (info)
-			password = info->so_pin;
+		if (info && info->so_pin)
+			snprintf(password, sizeof(password), "%s", info->so_pin);
 	} else {
 		env = "GNUTLS_PIN";
 		desc = "user";
-		if (info)
-			password = info->pin;
+		if (info && info->pin)
+			snprintf(password, sizeof(password), "%s", info->pin);
 	}
 
 	if (flags & GNUTLS_PIN_FINAL_TRY) {
@@ -1046,19 +1094,19 @@ pin_callback(void *user, int attempt, const char *token_url,
 		}
 	}
 
-	if (password == NULL) {
-		password = getenv(env);
-		if (password == NULL) /* compatibility */
-			password = getenv("GNUTLS_PIN");
+	if (password[0] == 0) {
+		getenv_copy(password, sizeof(password), env);
+		if (password[0] == 0) /* compatibility */
+			getenv_copy(password, sizeof(password), "GNUTLS_PIN");
 	}
 
-	if (password == NULL && (info == NULL || info->batch == 0 || info->ask_pass != 0)) {
+	if (password[0] == 0 && (info == NULL || info->batch == 0 || info->ask_pass != 0)) {
 		if (token_label && token_label[0] != 0) {
 			fprintf(stderr, "Token '%s' with URL '%s' ", token_label, token_url);
 			fprintf(stderr, "requires %s PIN\n", desc);
-			password = getpass("Enter PIN: ");
+			getpass_copy(password, sizeof(password), "Enter PIN: ");
 		} else {
-			password = getpass("Enter password: ");
+			getpass_copy(password, sizeof(password), "Enter password: ");
 		}
 
 	} else {
@@ -1072,7 +1120,7 @@ pin_callback(void *user, int attempt, const char *token_url,
 		}
 	}
 
-	if (password == NULL || password[0] == 0 || password[0] == '\n') {
+	if (password[0] == 0 || password[0] == '\n') {
 		fprintf(stderr, "No PIN given.\n");
 		if (info != NULL && info->batch != 0) {
 			fprintf(stderr, "note: when operating in batch mode, set the GNUTLS_PIN or GNUTLS_SO_PIN environment variables\n");
diff --git a/src/common.h b/src/common.h
index 61227a5af0..a4a49d0fe9 100644
--- a/src/common.h
+++ b/src/common.h
@@ -25,6 +25,7 @@
 
 #include <config.h>
 #include <gnutls/gnutls.h>
+#include <gnutls/pkcs11.h>
 #include <certtool-common.h>
 #include <c-ctype.h>
 #include <string.h>
@@ -71,6 +72,10 @@ const char *raw_to_hex(const unsigned char *raw, size_t raw_size);
 const char *raw_to_base64(const unsigned char *raw, size_t raw_size);
 int check_command(gnutls_session_t session, const char *str, unsigned no_cli_cert);
 
+#define MAX_PIN_LEN GNUTLS_PKCS11_MAX_PIN_LEN
+void getenv_copy(char *str, size_t max_str_size, const char *envvar);
+void getpass_copy(char *pass, size_t max_pass_size, const char *prompt);
+
 int
 pin_callback(void *user, int attempt, const char *token_url,
 	     const char *token_label, unsigned int flags, char *pin,
diff --git a/src/p11tool-args.def b/src/p11tool-args.def
index 3f320ce416..8477a4ddac 100644
--- a/src/p11tool-args.def
+++ b/src/p11tool-args.def
@@ -57,8 +57,9 @@ flag = {
 
 flag = {
     name      = initialize-so-pin;
-    descrip   = "Initializes/Resets a PKCS #11 token security officer PIN";
-    doc = "";
+    descrip   = "Initializes/Resets a PKCS #11 token security officer PIN.";
+    doc = "This initializes the security officer's PIN. When used non-interactively use the GNUTLS_NEW_SO_PIN
+environment variables to initialize SO's PIN.";
 };
 
 flag = {
diff --git a/src/pkcs11.c b/src/pkcs11.c
index 0dc2c563fe..66ef6b0fe0 100644
--- a/src/pkcs11.c
+++ b/src/pkcs11.c
@@ -21,8 +21,6 @@
  */
 #include <config.h>
 
-#include <getpass.h>
-
 #include <gnutls/gnutls.h>
 #include <gnutls/pkcs11.h>
 #include <gnutls/abstract.h>
@@ -1411,8 +1409,7 @@ pkcs11_init(FILE * outfile, const char *url, const char *label,
 	    common_info_st * info)
 {
 	int ret;
-	const char *pin;
-	char so_pin[32];
+	char so_pin[MAX_PIN_LEN];
 
 	pkcs11_common(info);
 
@@ -1426,21 +1423,19 @@ pkcs11_init(FILE * outfile, const char *url, const char *label,
 		app_exit(1);
 	}
 
-	if (info->so_pin != NULL)
-		pin = info->so_pin;
-	else {
-		pin = getenv("GNUTLS_SO_PIN");
-		if (pin == NULL && info->batch == 0)
-			pin = getpass("Enter Security Officer's PIN: ");
-		if (pin == NULL)
+	if (info->so_pin != NULL) {
+		snprintf(so_pin, sizeof(so_pin), "%s", info->so_pin);
+	} else {
+		getenv_copy(so_pin, sizeof(so_pin), "GNUTLS_SO_PIN");
+		if (so_pin[0] == 0 && info->batch == 0)
+			getpass_copy(so_pin, sizeof(so_pin), "Enter Security Officer's PIN: ");
+		if (so_pin[0] == 0)
 			app_exit(1);
 	}
 
-	if (strlen(pin) >= sizeof(so_pin) || pin[0] == '\n')
+	if (so_pin[0] == '\n' || so_pin[0] == 0)
 		app_exit(1);
 
-	strcpy(so_pin, pin);
-
 	fprintf(stderr, "Initializing token... ");
 	ret = gnutls_pkcs11_token_init(url, so_pin, label);
 	if (ret < 0) {
@@ -1459,7 +1454,7 @@ void
 pkcs11_set_token_pin(FILE * outfile, const char *url, common_info_st * info, unsigned so)
 {
 	int ret;
-	const char *pin;
+	char newpin[MAX_PIN_LEN] = "";
 
 	pkcs11_common(info);
 
@@ -1468,34 +1463,32 @@ pkcs11_set_token_pin(FILE * outfile, const char *url, common_info_st * info, uns
 		app_exit(1);
 	}
 
-	fprintf(stderr, "Setting token's user PIN...\n");
+	if (so)
+		fprintf(stderr, "Setting admin's PIN...\n");
+	else
+		fprintf(stderr, "Setting user's PIN...\n");
 
 	if (so) {
-		if (info->so_pin != NULL) {
-			pin = info->so_pin;
-		} else {
-			pin = getenv("GNUTLS_SO_PIN");
-			if (pin == NULL && info->batch == 0)
-				pin = getpass("Enter Administrators's new PIN: ");
-			if (pin == NULL)
-				app_exit(1);
+		getenv_copy(newpin, sizeof(newpin), "GNUTLS_NEW_SO_PIN");
+		if (newpin[0] == 0 && info->batch == 0) {
+			getpass_copy(newpin, sizeof(newpin), "Enter Administrators's new PIN: ");
 		}
 	} else {
 		if (info->pin != NULL) {
-			pin = info->pin;
+			snprintf(newpin, sizeof(newpin), "%s", info->pin);
 		} else {
-			pin = getenv("GNUTLS_PIN");
-			if (pin == NULL && info->batch == 0)
-				pin = getpass("Enter User's new PIN: ");
-			if (pin == NULL)
-				app_exit(1);
+			getenv_copy(newpin, sizeof(newpin), "GNUTLS_PIN");
+			if (newpin[0] == 0 && info->batch == 0)
+				getpass_copy(newpin, sizeof(newpin), "Enter User's new PIN: ");
 		}
 	}
 
-	if (pin == NULL || pin[0] == '\n')
+	if (newpin[0] == 0 || newpin[0] == '\n') {
+		fprintf(stderr, "No PIN was given to change\n");
 		app_exit(1);
+	}
 
-	ret = gnutls_pkcs11_token_set_pin(url, NULL, pin, (so!=0)?GNUTLS_PIN_SO:GNUTLS_PIN_USER);
+	ret = gnutls_pkcs11_token_set_pin(url, NULL, newpin, (so!=0)?GNUTLS_PIN_SO:GNUTLS_PIN_USER);
 	if (ret < 0) {
 		fprintf(stderr, "Error in %s:%d: %s\n", __func__, __LINE__,
 			gnutls_strerror(ret));
diff --git a/tests/testpkcs11.sh b/tests/testpkcs11.sh
index a3d2c7f2b6..c126d143f2 100755
--- a/tests/testpkcs11.sh
+++ b/tests/testpkcs11.sh
@@ -57,7 +57,7 @@ CERTTOOL_PARAM="--stdout-info"
 
 if test "${WINDIR}" != ""; then
 	exit 77
-fi 
+fi
 
 ASAN_OPTIONS="detect_leaks=0"
 export ASAN_OPTIONS
@@ -873,6 +873,120 @@ use_certificate_test () {
 	echo ok
 }
 
+reset_pins () {
+	token="$1"
+	UPIN="$2"
+	SOPIN="$3"
+	NEWPIN=88654321
+	LARGE_NEWPIN="1234123412341234123412341234123" #31 chars
+	TOO_LARGE_NEWPIN="12341234123412341234123412341234" #32 chars
+
+	echo -n "* Setting SO PIN... "
+	# Test admin PIN
+	GNUTLS_NEW_SO_PIN="${NEWPIN}" \
+	GNUTLS_SO_PIN="${SOPIN}" \
+	${P11TOOL} ${ADDITIONAL_PARAM} --login --initialize-so-pin "${token}" >>"${LOGFILE}" 2>&1
+	if test $? != 0; then
+		echo failed
+		exit_error
+	fi
+	echo ok
+
+	# reset back
+	echo -n "* Re-setting SO PIN... "
+	TMP="${NEWPIN}"
+	GNUTLS_SO_PIN="${TMP}" \
+	GNUTLS_NEW_SO_PIN="${SOPIN}" \
+	${P11TOOL} ${ADDITIONAL_PARAM} --login --initialize-so-pin "${token}" >>"${LOGFILE}" 2>&1
+	if test $? != 0; then
+		echo failed
+		exit_error
+	fi
+	echo ok
+
+	echo -n "* Setting too large SO PIN... "
+	GNUTLS_NEW_SO_PIN="${TOO_LARGE_NEWPIN}" \
+	GNUTLS_SO_PIN="${SOPIN}" \
+	${P11TOOL} ${ADDITIONAL_PARAM} --login --initialize-so-pin "${token}" >>"${LOGFILE}" 2>&1
+	if test $? = 0; then
+		echo failed
+		exit_error
+	fi
+	echo ok
+
+	echo -n "* Setting large SO PIN... "
+	GNUTLS_NEW_SO_PIN="${LARGE_NEWPIN}" \
+	GNUTLS_SO_PIN="${SOPIN}" \
+	${P11TOOL} ${ADDITIONAL_PARAM} --login --initialize-so-pin "${token}" >>"${LOGFILE}" 2>&1
+	if test $? != 0; then
+		echo failed
+		exit_error
+	fi
+	echo ok
+
+	# reset back
+	echo -n "* Re-setting SO PIN... "
+	TMP="${LARGE_NEWPIN}"
+	GNUTLS_SO_PIN="${TMP}" \
+	GNUTLS_NEW_SO_PIN="${SOPIN}" \
+	${P11TOOL} ${ADDITIONAL_PARAM} --login --initialize-so-pin "${token}" >>"${LOGFILE}" 2>&1
+	if test $? != 0; then
+		echo failed
+		exit_error
+	fi
+	echo ok
+
+	NEWPIN=977654321
+	# Test user PIN
+	echo -n "* Setting user PIN... "
+	export GNUTLS_SO_PIN="${SOPIN}"
+	export GNUTLS_PIN="${NEWPIN}"
+	${P11TOOL} ${ADDITIONAL_PARAM} --login --initialize-pin "${token}" >>"${LOGFILE}" 2>&1
+	if test $? != 0; then
+		echo failed
+		exit_error
+	fi
+	echo ok
+
+	echo -n "* Re-setting user PIN... "
+	export GNUTLS_PIN="${UPIN}"
+	${P11TOOL} ${ADDITIONAL_PARAM} --login --initialize-pin "${token}" >>"${LOGFILE}" 2>&1
+	if test $? != 0; then
+		echo failed
+		exit_error
+	fi
+	echo ok
+
+	echo -n "* Setting too large user PIN... "
+	export GNUTLS_SO_PIN="${SOPIN}"
+	export GNUTLS_PIN="${TOO_LARGE_NEWPIN}"
+	${P11TOOL} ${ADDITIONAL_PARAM} --login --initialize-pin "${token}" >>"${LOGFILE}" 2>&1
+	if test $? = 0; then
+		echo failed
+		exit_error
+	fi
+	echo ok
+
+	echo -n "* Setting large user PIN... "
+	export GNUTLS_SO_PIN="${SOPIN}"
+	export GNUTLS_PIN="${LARGE_NEWPIN}"
+	${P11TOOL} ${ADDITIONAL_PARAM} --login --initialize-pin "${token}" >>"${LOGFILE}" 2>&1
+	if test $? != 0; then
+		echo failed
+		exit_error
+	fi
+	echo ok
+
+	echo -n "* Re-setting user PIN... "
+	export GNUTLS_PIN="${UPIN}"
+	${P11TOOL} ${ADDITIONAL_PARAM} --login --initialize-pin "${token}" >>"${LOGFILE}" 2>&1
+	if test $? != 0; then
+		echo failed
+		exit_error
+	fi
+	echo ok
+}
+
 
 
 echo "Testing PKCS11 support"
@@ -896,10 +1010,11 @@ fi
 . "${srcdir}/testpkcs11.${type}"
 
 export GNUTLS_PIN=12345678
-export GNUTLS_SO_PIN=00000000
+export GNUTLS_SO_PIN=00000001
 
 init_card "${GNUTLS_PIN}" "${GNUTLS_SO_PIN}"
 
+
 # find token name
 TOKEN=`${P11TOOL} ${ADDITIONAL_PARAM} --list-tokens pkcs11:token=Nikos|grep URL|grep token=GnuTLS-Test|sed 's/\s*URL\: //g'`
 
@@ -909,6 +1024,8 @@ if test "x${TOKEN}" = x; then
 	exit_error
 fi
 
+reset_pins "${TOKEN}" "${GNUTLS_PIN}" "${GNUTLS_SO_PIN}"
+
 #write a given privkey
 write_privkey "${TOKEN}" "${GNUTLS_PIN}" "${srcdir}/testpkcs11-certs/client.key"