Skip to content

Commit

Permalink
Remove complexity from slot reinitialization after fork
Browse files Browse the repository at this point in the history
After fork the context handler will reset the PKCS#11 module by
calling C_Initialize. After this all handles from the module should
be considered invalidated.

This combines the session opening and logging after fork to single
function, and removes the relogin flag from functions where it's
not needed. The new pkcs11_reload_slot() properly update the state
so the normal functions operate as expected.

This also fixes a memory leak after fork: if the slot was in
logged-in state, a session was leaked from check_slot_fork_int because:
 1. the "if loggedIn" clears state, and calls pkcs11_relogin() which
    also implicitly opens a session
 2. the next "if haveSession" block fires also, and clears state,
    and calls pkcs11_reopen_session. This function will explicitly
    call C_OpenSession overwriting and leaking the session from step #1
  • Loading branch information
fabled authored and mtrojnar committed Apr 11, 2021
1 parent 079b9cf commit d28387a
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 53 deletions.
7 changes: 3 additions & 4 deletions src/libp11-int.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,7 @@ extern CK_RV C_UnloadModule(void *module);
extern void pkcs11_destroy_keys(PKCS11_TOKEN *, unsigned int);
extern void pkcs11_destroy_certs(PKCS11_TOKEN *);
extern int pkcs11_reload_key(PKCS11_KEY *);
extern int pkcs11_reopen_session(PKCS11_SLOT * slot);
extern int pkcs11_relogin(PKCS11_SLOT * slot);
extern int pkcs11_reload_slot(PKCS11_SLOT * slot);

/* Managing object attributes */
extern int pkcs11_getattr_var(PKCS11_TOKEN *, CK_OBJECT_HANDLE,
Expand Down Expand Up @@ -232,7 +231,7 @@ extern void pkcs11_CTX_unload(PKCS11_CTX * ctx);
extern void pkcs11_CTX_free(PKCS11_CTX * ctx);

/* Open a session in RO or RW mode */
extern int pkcs11_open_session(PKCS11_SLOT * slot, int rw, int relogin);
extern int pkcs11_open_session(PKCS11_SLOT * slot, int rw);

/* Get a list of all slots */
extern int pkcs11_enumerate_slots(PKCS11_CTX * ctx,
Expand All @@ -258,7 +257,7 @@ extern PKCS11_SLOT *pkcs11_find_next_token(PKCS11_CTX * ctx,
extern int pkcs11_is_logged_in(PKCS11_SLOT * slot, int so, int * res);

/* Authenticate to the card */
extern int pkcs11_login(PKCS11_SLOT * slot, int so, const char *pin, int relogin);
extern int pkcs11_login(PKCS11_SLOT * slot, int so, const char *pin);

/* De-authenticate from the card */
extern int pkcs11_logout(PKCS11_SLOT * slot);
Expand Down
2 changes: 0 additions & 2 deletions src/libp11.h
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,6 @@ P11_DEPRECATED_FUNC extern int PKCS11_private_decrypt(
# define CKR_F_PKCS11_PRIVATE_DECRYPT 121
# define CKR_F_PKCS11_PRIVATE_ENCRYPT 122
# define CKR_F_PKCS11_RELOAD_KEY 123
# define CKR_F_PKCS11_REOPEN_SESSION 124
# define CKR_F_PKCS11_SEED_RANDOM 125
# define CKR_F_PKCS11_STORE_CERTIFICATE 126
# define CKR_F_PKCS11_STORE_KEY 127
Expand Down Expand Up @@ -544,7 +543,6 @@ P11_DEPRECATED_FUNC extern int PKCS11_private_decrypt(
#define PKCS11_F_PKCS11_PRIVATE_DECRYPT CKR_F_PKCS11_PRIVATE_DECRYPT
#define PKCS11_F_PKCS11_PRIVATE_ENCRYPT CKR_F_PKCS11_PRIVATE_ENCRYPT
#define PKCS11_F_PKCS11_RELOAD_KEY CKR_F_PKCS11_RELOAD_KEY
#define PKCS11_F_PKCS11_REOPEN_SESSION CKR_F_PKCS11_REOPEN_SESSION
#define PKCS11_F_PKCS11_SEED_RANDOM CKR_F_PKCS11_SEED_RANDOM
#define PKCS11_F_PKCS11_STORE_CERTIFICATE CKR_F_PKCS11_STORE_CERTIFICATE
#define PKCS11_F_PKCS11_STORE_KEY CKR_F_PKCS11_STORE_KEY
Expand Down
18 changes: 4 additions & 14 deletions src/p11_atfork.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,19 +116,8 @@ static int check_slot_fork_int(PKCS11_SLOT *slot)
if (check_fork_int(SLOT2CTX(slot)) < 0)
return -1;
if (spriv->forkid != cpriv->forkid) {
if (spriv->loggedIn) {
int saved = spriv->haveSession;
spriv->haveSession = 0;
spriv->loggedIn = 0;
if (pkcs11_relogin(slot) < 0)
return -1;
spriv->haveSession = saved;
}
if (spriv->haveSession) {
spriv->haveSession = 0;
if (pkcs11_reopen_session(slot) < 0)
return -1;
}
if (pkcs11_reload_slot(slot) < 0)
return -1;
spriv->forkid = cpriv->forkid;
}
return 0;
Expand All @@ -147,7 +136,8 @@ static int check_key_fork_int(PKCS11_KEY *key)
if (check_slot_fork_int(slot) < 0)
return -1;
if (spriv->forkid != kpriv->forkid) {
pkcs11_reload_key(key);
if (pkcs11_reload_key(key) < 0)
return -1;
kpriv->forkid = spriv->forkid;
}
return 0;
Expand Down
1 change: 0 additions & 1 deletion src/p11_ckr.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ static ERR_STRING_DATA CKR_str_functs[] = {
{ERR_FUNC(CKR_F_PKCS11_PRIVATE_DECRYPT), "pkcs11_private_decrypt"},
{ERR_FUNC(CKR_F_PKCS11_PRIVATE_ENCRYPT), "pkcs11_private_encrypt"},
{ERR_FUNC(CKR_F_PKCS11_RELOAD_KEY), "pkcs11_reload_key"},
{ERR_FUNC(CKR_F_PKCS11_REOPEN_SESSION), "pkcs11_reopen_session"},
{ERR_FUNC(CKR_F_PKCS11_SEED_RANDOM), "pkcs11_seed_random"},
{ERR_FUNC(CKR_F_PKCS11_STORE_CERTIFICATE), "pkcs11_store_certificate"},
{ERR_FUNC(CKR_F_PKCS11_STORE_KEY), "pkcs11_store_key"},
Expand Down
4 changes: 2 additions & 2 deletions src/p11_front.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ int PKCS11_open_session(PKCS11_SLOT *slot, int rw)
{
if (check_slot_fork(slot) < 0)
return -1;
return pkcs11_open_session(slot, rw, 0);
return pkcs11_open_session(slot, rw);
}

int PKCS11_enumerate_slots(PKCS11_CTX *ctx,
Expand Down Expand Up @@ -118,7 +118,7 @@ int PKCS11_login(PKCS11_SLOT *slot, int so, const char *pin)
{
if (check_slot_fork(slot) < 0)
return -1;
return pkcs11_login(slot, so, pin, 0);
return pkcs11_login(slot, so, pin);
}

int PKCS11_logout(PKCS11_SLOT *slot)
Expand Down
52 changes: 22 additions & 30 deletions src/p11_slot.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,17 +147,15 @@ PKCS11_SLOT *pkcs11_find_next_token(PKCS11_CTX *ctx, PKCS11_SLOT *slots,
/*
* Open a session with this slot
*/
int pkcs11_open_session(PKCS11_SLOT *slot, int rw, int relogin)
int pkcs11_open_session(PKCS11_SLOT *slot, int rw)
{
PKCS11_SLOT_private *spriv = PRIVSLOT(slot);
PKCS11_CTX *ctx = SLOT2CTX(slot);
int rv;

if (relogin == 0) {
if (spriv->haveSession) {
CRYPTOKI_call(ctx, C_CloseSession(spriv->session));
spriv->haveSession = 0;
}
if (spriv->haveSession) {
CRYPTOKI_call(ctx, C_CloseSession(spriv->session));
spriv->haveSession = 0;
}
rv = CRYPTOKI_call(ctx,
C_OpenSession(spriv->id,
Expand All @@ -170,22 +168,6 @@ int pkcs11_open_session(PKCS11_SLOT *slot, int rw, int relogin)
return 0;
}

int pkcs11_reopen_session(PKCS11_SLOT *slot)
{
PKCS11_SLOT_private *spriv = PRIVSLOT(slot);
PKCS11_CTX *ctx = SLOT2CTX(slot);
int rv;

rv = CRYPTOKI_call(ctx,
C_OpenSession(spriv->id,
CKF_SERIAL_SESSION | (spriv->prev_rw ? CKF_RW_SESSION : 0),
NULL, NULL, &spriv->session));
CRYPTOKI_checkerr(CKR_F_PKCS11_REOPEN_SESSION, rv);
spriv->haveSession = 1;

return 0;
}

/*
* Determines if user is authenticated with token
*/
Expand Down Expand Up @@ -219,22 +201,21 @@ int pkcs11_is_logged_in(PKCS11_SLOT *slot, int so, int *res)
}

/*
* Authenticate with the card. relogin should be set if we automatically
* relogin after a fork.
* Authenticate with the card.
*/
int pkcs11_login(PKCS11_SLOT *slot, int so, const char *pin, int relogin)
int pkcs11_login(PKCS11_SLOT *slot, int so, const char *pin)
{
PKCS11_CTX *ctx = SLOT2CTX(slot);
PKCS11_SLOT_private *spriv = PRIVSLOT(slot);
int rv;

if (!relogin && spriv->loggedIn)
if (spriv->loggedIn)
return 0; /* Nothing to do */

if (!spriv->haveSession) {
/* SO gets a r/w session by default,
* user gets a r/o session by default. */
if (pkcs11_open_session(slot, so, relogin))
if (pkcs11_open_session(slot, so))
return -1;
}

Expand All @@ -257,13 +238,24 @@ int pkcs11_login(PKCS11_SLOT *slot, int so, const char *pin, int relogin)
}

/*
* Authenticate with the card
* Reopens the slot by creating a session and logging in if needed.
*/
int pkcs11_relogin(PKCS11_SLOT *slot)
int pkcs11_reload_slot(PKCS11_SLOT *slot)
{
PKCS11_SLOT_private *spriv = PRIVSLOT(slot);

return pkcs11_login(slot, spriv->prev_so, spriv->prev_pin, 1);
if (spriv->haveSession) {
spriv->haveSession = 0;
if (pkcs11_open_session(slot, spriv->prev_rw))
return -1;
}
if (spriv->loggedIn) {
spriv->loggedIn = 0;
if (pkcs11_login(slot, spriv->prev_so, spriv->prev_pin))
return -1;
}

return 0;
}

/*
Expand Down

0 comments on commit d28387a

Please sign in to comment.