Explicitly logging out the token to avoid leaving it in a non-stable state.
Just calling session.close() will, sometimes, leave the TPM in an inconsistent state which will cause it to generate a bad RSA key, despite reporting key generation successful. Further use of the key will generate incorrect signatures.
Problem does not reproduce all the time, and the likelihood of happening appears to vary depending on the hardware used (Cr-48 > AGZ > L13). Calling session.logout() before session.close() fixes the issue.
Change-Id: Ib601818e180243ecf1f59e40f46d141e1d826286
BUG=chromium-os:10536
TEST=enroll a certificate and successfully connect to Google-A on Cr-48 device.
Review URL: http://codereview.chromium.org/6338003
diff --git a/base_policy/policy-utils.js b/base_policy/policy-utils.js
index e328323..03d9964 100644
--- a/base_policy/policy-utils.js
+++ b/base_policy/policy-utils.js
@@ -494,7 +494,7 @@
this.stop(token, 'error', 'Failed to login to token');
} catch (ex) {
this.stop(token, 'error', 'Exception initializing PIN: ' + ex);
- session.close();
+ session.logoutAndClose();
return false;
}
@@ -505,7 +505,7 @@
this.info('PIN Reset complete.');
} catch (ex) {
this.stop(token, 'error', 'Exception changing PIN: ' + ex);
- session.close();
+ session.logoutAndClose();
return false;
}
@@ -515,7 +515,7 @@
this.stop(token, pinType);
}
- session.close();
+ session.logoutAndClose();
return true;
}
@@ -813,6 +813,8 @@
var session = null;
+ this.info('Opening session and logging into token.');
+
try {
token.closeAllSessions();
session = token.openSession(Token.CKF_RW_SESSION);
@@ -824,12 +826,12 @@
try {
if (!session.login(sessionType, pin)) {
this.error('Unable to log in user into token.');
- session.close();
+ session.logoutAndClose();
return null;
}
} catch (ex) {
this.error('Failed to login user into token: ' + ex);
- session.close();
+ session.logoutAndClose();
return null;
}
@@ -952,11 +954,11 @@
var cert = policy.findCertificateById(session, this.id);
var key = policy.findPrivateKeyById(session, this.id);
- session.close();
+ session.logoutAndClose();
return (cert != null && key != null);
} catch(ex) {
- session.close();
+ session.logoutAndClose();
this.error('Error checking certificate is installed: ', ex);
}
@@ -1002,13 +1004,13 @@
// Generate a key pair.
this.policy.generateKeyPair(session, this.id, this.label);
} catch(e) {
- session.close();
+ session.logoutAndClose();
return this.stop('error', 'Failed to create key on PKCS#11 device: ' + e);
}
this.stop('key', 'Key generation complete');
- session.close();
+ session.logoutAndClose();
this.start('csr', 'Initiating Certificate Signing Request');
@@ -1157,7 +1159,7 @@
this.stop('cert');
- session.close();
+ session.logoutAndClose();
};
Policy.prototype.setBrowserPolicy =
diff --git a/crypto_pkcs11.cc b/crypto_pkcs11.cc
index a31aeb9..0f5502b 100644
--- a/crypto_pkcs11.cc
+++ b/crypto_pkcs11.cc
@@ -499,6 +499,8 @@
BindMethod(instance_t, &Pkcs11::Session::FindObjects, "findObjects");
BindMethod(instance_t, &Pkcs11::Session::CreateObject, "createObject");
+ BindMethod(instance_t, &Pkcs11::Session::LogoutAndClose, "logoutAndClose");
+
return true;
}
@@ -543,6 +545,12 @@
if (!session_handle_)
return ThrowException("Not open");
+ if (logged_in_) {
+ if (!OkOrThrow(C_Logout(session_handle_)))
+ return v8::Undefined();
+ logged_in_ = false;
+ }
+
OkOrThrow(C_CloseSession(session_handle_));
session_handle_ = 0;
return v8::Undefined();
@@ -561,6 +569,8 @@
v8::String::AsciiValue ascii_pin(args[1]);
+ logged_in_ = false;
+
CK_RV rv = C_Login(session_handle_, user_type,
reinterpret_cast<CK_CHAR_PTR>(*ascii_pin),
ascii_pin.length());
@@ -571,11 +581,20 @@
if (!OkOrThrow(rv))
return v8::Undefined();
+ logged_in_ = true;
return v8::True();
}
v8::Handle<v8::Value> Pkcs11::Session::Logout(const v8::Arguments& args) {
OkOrThrow(C_Logout(session_handle_));
+ logged_in_ = false;
+ return v8::Undefined();
+}
+
+v8::Handle<v8::Value> Pkcs11::Session::LogoutAndClose(
+ const v8::Arguments& args) {
+ Logout(args);
+ Close(args);
return v8::Undefined();
}
diff --git a/crypto_pkcs11.h b/crypto_pkcs11.h
index 5032f4a..0dd4820 100644
--- a/crypto_pkcs11.h
+++ b/crypto_pkcs11.h
@@ -125,8 +125,10 @@
return "entd.crypto.Pkcs11.Session";
};
- Session() : session_handle_(0) {}
+ Session() : session_handle_(0), logged_in_(false) {}
~Session() {
+ if (logged_in_)
+ C_Logout(session_handle_);
if (session_handle_)
C_CloseSession(session_handle_);
}
@@ -152,12 +154,16 @@
v8::Handle<v8::Value> CreateObject(const v8::Arguments& args);
v8::Handle<v8::Value> GenerateKeyPair(const v8::Arguments& args);
+ // Helper functions.
+ v8::Handle<v8::Value> LogoutAndClose(const v8::Arguments& args);
+
// Invoke the 'bool Refresh()' method from script.
v8::Handle<v8::Value> CallRefresh(const v8::Arguments& args);
private:
CK_SLOT_ID slot_id_;
CK_SESSION_HANDLE session_handle_;
+ bool logged_in_;
};
class Object : public Scriptable<Object> {
