Skip to content

Commit

Permalink
Replace SessionFlags with bool to open session
Browse files Browse the repository at this point in the history
There are only two flags supported as arguments when opening a
session.

One of them must always be true, but perversely defaults to false.
This forces client code to construct a trivial value to pass. This
commit now hides this flag, setting it to its only valid value always.
This also removes a single test which checked for failure when the flag
was set to false.

With only one flag (read/write) remaining, the open session call now
accepts a boolean for the option and conversion to a wider integer
type is handled internally.

Signed-off-by: Keith Koskie <vkkoskie@gmail.com>
  • Loading branch information
vkkoskie authored and ionut-arm committed Jan 27, 2022
1 parent ded651c commit 6d2df93
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 33 deletions.
6 changes: 3 additions & 3 deletions cryptoki/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub use locking::*;

use crate::error::{Error, Result, Rv};
use crate::mechanism::{MechanismInfo, MechanismType};
use crate::session::{Session, SessionFlags};
use crate::session::Session;
use crate::slot::{Slot, SlotInfo, TokenInfo};

use derivative::Derivative;
Expand Down Expand Up @@ -159,7 +159,7 @@ impl Pkcs11 {
}

/// Open a new session with no callback set
pub fn open_session_no_callback(&self, slot_id: Slot, flags: SessionFlags) -> Result<Session> {
session_management::open_session_no_callback(self, slot_id, flags)
pub fn open_session_no_callback(&self, slot_id: Slot, read_write: bool) -> Result<Session> {
session_management::open_session_no_callback(self, slot_id, read_write)
}
}
14 changes: 10 additions & 4 deletions cryptoki/src/context/session_management.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,31 @@
// SPDX-License-Identifier: Apache-2.0
//! Session management functions

use cryptoki_sys::{CKF_RW_SESSION, CKF_SERIAL_SESSION};

use crate::context::Pkcs11;
use crate::error::{Result, Rv};
use crate::session::{Session, SessionFlags};
use crate::session::Session;
use crate::slot::Slot;
use std::convert::TryInto;

// See public docs on stub in parent mod.rs
#[inline(always)]
pub(super) fn open_session_no_callback(
ctx: &Pkcs11,
slot_id: Slot,
flags: SessionFlags,
read_write: bool,
) -> Result<Session> {
let mut session_handle = 0;

let flags = if read_write {
CKF_SERIAL_SESSION | CKF_RW_SESSION
} else {
CKF_SERIAL_SESSION
};
unsafe {
Rv::from(get_pkcs11!(ctx, C_OpenSession)(
slot_id.try_into()?,
flags.into(),
flags,
// TODO: abstract those types or create new functions for callbacks
std::ptr::null_mut(),
None,
Expand Down
5 changes: 1 addition & 4 deletions cryptoki/src/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ impl Session {
/// use cryptoki::context::CInitializeArgs;
/// use cryptoki::object::AttributeType;
/// use cryptoki::session::UserType;
/// use cryptoki::session::SessionFlags;
/// use std::collections::HashMap;
/// use std::env;
///
Expand All @@ -163,10 +162,8 @@ impl Session {
///
/// pkcs11.initialize(CInitializeArgs::OsThreads).unwrap();
/// let slot = pkcs11.get_slots_with_token().unwrap().remove(0);
/// let mut flags = SessionFlags::new();
/// let _ = flags.set_rw_session(true).set_serial_session(true);
///
/// let session = pkcs11.open_session_no_callback(slot, flags).unwrap();
/// let session = pkcs11.open_session_no_callback(slot, true).unwrap();
/// session.login(UserType::User, Some("fedcba"));
///
/// let empty_attrib= vec![];
Expand Down
32 changes: 11 additions & 21 deletions cryptoki/tests/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ fn sign_verify() -> Result<()> {
let _ = flags.set_rw_session(true).set_serial_session(true);

// open a session
let session = pkcs11.open_session_no_callback(slot, flags)?;
let session = pkcs11.open_session_no_callback(slot, true)?;

// log in the session
session.login(UserType::User, Some(USER_PIN))?;
Expand Down Expand Up @@ -85,7 +85,7 @@ fn encrypt_decrypt() -> Result<()> {
let _ = flags.set_rw_session(true).set_serial_session(true);

// open a session
let session = pkcs11.open_session_no_callback(slot, flags)?;
let session = pkcs11.open_session_no_callback(slot, true)?;

// log in the session
session.login(UserType::User, Some(USER_PIN))?;
Expand Down Expand Up @@ -141,7 +141,7 @@ fn derive_key() -> Result<()> {
let _ = flags.set_rw_session(true).set_serial_session(true);

// open a session
let session = pkcs11.open_session_no_callback(slot, flags)?;
let session = pkcs11.open_session_no_callback(slot, true)?;

// log in the session
session.login(UserType::User, Some(USER_PIN))?;
Expand Down Expand Up @@ -236,7 +236,7 @@ fn import_export() -> Result<()> {
let _ = flags.set_rw_session(true).set_serial_session(true);

// open a session
let session = pkcs11.open_session_no_callback(slot, flags)?;
let session = pkcs11.open_session_no_callback(slot, true)?;

// log in the session
session.login(UserType::User, Some(USER_PIN))?;
Expand Down Expand Up @@ -306,7 +306,7 @@ fn wrap_and_unwrap_key() {
let _ = flags.set_rw_session(true).set_serial_session(true);

// open a session
let session = pkcs11.open_session_no_callback(slot, flags).unwrap();
let session = pkcs11.open_session_no_callback(slot, true).unwrap();

// log in the session
session.login(UserType::User, Some(USER_PIN)).unwrap();
Expand Down Expand Up @@ -400,7 +400,7 @@ fn login_feast() {
for _ in 0..SESSIONS {
let pkcs11 = pkcs11.clone();
threads.push(thread::spawn(move || {
let session = pkcs11.open_session_no_callback(slot, flags).unwrap();
let session = pkcs11.open_session_no_callback(slot, true).unwrap();
match session.login(UserType::User, Some(USER_PIN)) {
Ok(_) | Err(Error::Pkcs11(RvError::UserAlreadyLoggedIn)) => {}
Err(e) => panic!("Bad error response: {}", e),
Expand Down Expand Up @@ -463,19 +463,9 @@ fn get_session_info_test() -> Result<()> {
let (pkcs11, slot) = init_pins();

let mut flags = SessionFlags::new();

// Check that OpenSession errors when CKF_SERIAL_SESSION is not set
if let Err(cryptoki::error::Error::Pkcs11(rv_error)) =
pkcs11.open_session_no_callback(slot, flags)
{
assert_eq!(rv_error, RvError::SessionParallelNotSupported);
} else {
panic!("Should error when CKF_SERIAL_SESSION is not set");
}

let _ = flags.set_serial_session(true);
{
let session = pkcs11.open_session_no_callback(slot, flags)?;
let session = pkcs11.open_session_no_callback(slot, false)?;
let session_info = session.get_session_info()?;
assert!(!session_info.read_write());
assert_eq!(session_info.slot_id(), slot);
Expand Down Expand Up @@ -504,7 +494,7 @@ fn get_session_info_test() -> Result<()> {

let _ = flags.set_rw_session(true);

let session = pkcs11.open_session_no_callback(slot, flags)?;
let session = pkcs11.open_session_no_callback(slot, true)?;
let session_info = session.get_session_info()?;
assert!(session_info.read_write());
assert_eq!(session_info.slot_id(), slot);
Expand Down Expand Up @@ -539,7 +529,7 @@ fn generate_random_test() -> Result<()> {
let mut flags = SessionFlags::new();

let _ = flags.set_serial_session(true);
let session = pkcs11.open_session_no_callback(slot, flags)?;
let session = pkcs11.open_session_no_callback(slot, false)?;

let poor_seed: [u8; 32] = [0; 32];
session.seed_random(&poor_seed)?;
Expand All @@ -566,7 +556,7 @@ fn set_pin_test() -> Result<()> {
let mut flags = SessionFlags::new();

let _ = flags.set_serial_session(true).set_rw_session(true);
let session = pkcs11.open_session_no_callback(slot, flags)?;
let session = pkcs11.open_session_no_callback(slot, true)?;

session.login(UserType::User, Some(USER_PIN))?;
session.set_pin(USER_PIN, new_user_pin)?;
Expand All @@ -585,7 +575,7 @@ fn get_attribute_info_test() -> Result<()> {
let _ = flags.set_rw_session(true).set_serial_session(true);

// open a session
let session = pkcs11.open_session_no_callback(slot, flags)?;
let session = pkcs11.open_session_no_callback(slot, true)?;

// log in the session
session.login(UserType::User, Some(USER_PIN))?;
Expand Down
2 changes: 1 addition & 1 deletion cryptoki/tests/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub fn init_pins() -> (Pkcs11, Slot) {

{
// open a session
let session = pkcs11.open_session_no_callback(slot, flags).unwrap();
let session = pkcs11.open_session_no_callback(slot, true).unwrap();
// log in the session
session.login(UserType::So, Some(SO_PIN)).unwrap();
session.init_pin(USER_PIN).unwrap();
Expand Down

0 comments on commit 6d2df93

Please sign in to comment.