Skip to content

Commit 8de1a8a

Browse files
committed
Consolidate session exhaustion logic.
1 parent 93ca0ba commit 8de1a8a

File tree

1 file changed

+60
-64
lines changed

1 file changed

+60
-64
lines changed

src/ca.rs

Lines changed: 60 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,10 @@ pub enum CaError {
5151
BadPurpose,
5252
#[error("path not a directory")]
5353
BadSpecDirectory,
54-
#[error("failed to generate certificate")]
55-
CertGenFail,
56-
#[error("failed to create self signed cert for key")]
57-
SelfCertGenFail,
5854
#[error("CA state directory has no key.spec")]
5955
NoKeySpec,
56+
#[error("openssl command failed")]
57+
OpensslCmdFail,
6058
}
6159

6260
// This is a template for the openssl config file used for all CAs.
@@ -386,20 +384,9 @@ impl Ca {
386384
.arg("-out")
387385
.arg(csr.path());
388386

389-
debug!("executing command: \"{:#?}\"", cmd);
390-
let output = match cmd.output() {
391-
Ok(o) => o,
392-
Err(e) => {
393-
teardown_warn_only(connector, pwd);
394-
return Err(e.into());
395-
}
396-
};
397-
398-
if !output.status.success() {
399-
warn!("command failed with status: {}", output.status);
400-
warn!("stderr: \"{}\"", String::from_utf8_lossy(&output.stderr));
387+
if let Err(e) = execute_command(&mut cmd) {
401388
teardown_warn_only(connector, pwd);
402-
return Err(CaError::SelfCertGenFail.into());
389+
return Err(e);
403390
}
404391

405392
// return the path to the artifact created
@@ -408,9 +395,6 @@ impl Ca {
408395
// else we'll get back the path to the CSR so that it can be exported
409396
// and eventually certified by some external process
410397
let pem = if spec.self_signed {
411-
// sleep to let sessions cycle
412-
thread::sleep(Duration::from_millis(1500));
413-
414398
info!("Generating self-signed cert for CA root");
415399
let mut cmd = Command::new("openssl");
416400
cmd.arg("ca")
@@ -432,29 +416,11 @@ impl Ca {
432416
.arg("-in")
433417
.arg(csr.path())
434418
.arg("-out")
435-
.arg(CA_CERT)
436-
.output()?;
437-
438-
debug!("executing command: \"{:#?}\"", cmd);
439-
let output = match cmd
440-
.output()
441-
.context("Failed to self sign cert with `openssl ca`")
442-
{
443-
Ok(o) => o,
444-
Err(e) => {
445-
teardown_warn_only(connector, pwd);
446-
return Err(e);
447-
}
448-
};
449-
450-
if !output.status.success() {
451-
warn!("command failed with status: {}", output.status);
452-
warn!(
453-
"stderr: \"{}\"",
454-
String::from_utf8_lossy(&output.stderr)
455-
);
419+
.arg(CA_CERT);
420+
421+
if let Err(e) = execute_command(&mut cmd) {
456422
teardown_warn_only(connector, pwd);
457-
return Err(CaError::SelfCertGenFail.into());
423+
return Err(e);
458424
}
459425

460426
let cert_pem =
@@ -498,9 +464,6 @@ impl Ca {
498464
debug!("writing CSR to: {}", csr.path().display());
499465
fs::write(&csr, &spec.csr)?;
500466

501-
// sleep to let sessions cycle
502-
thread::sleep(Duration::from_millis(2500));
503-
504467
info!(
505468
"Generating cert from CSR & signing with key: {}",
506469
self.name()
@@ -532,28 +495,17 @@ impl Ca {
532495
.arg("-out")
533496
.arg(cert.path());
534497

535-
debug!("executing command: \"{:#?}\"", cmd);
536-
let output = match cmd.output() {
537-
Ok(o) => o,
538-
Err(e) => {
539-
teardown_warn_only(connector, pwd);
540-
return Err(e.into());
541-
}
542-
};
543-
544-
if !output.status.success() {
545-
warn!("command failed with status: {}", output.status);
546-
warn!("stderr: \"{}\"", String::from_utf8_lossy(&output.stderr));
498+
if let Err(e) = execute_command(&mut cmd) {
547499
teardown_warn_only(connector, pwd);
548-
return Err(CaError::CertGenFail.into());
549-
} else {
550-
debug!(
551-
"Successfully signed CsrSpec \"{}\" producing cert \"{}\"",
552-
csr.path().display(),
553-
cert.path().display()
554-
);
500+
return Err(e);
555501
}
556502

503+
debug!(
504+
"Successfully signed CsrSpec \"{}\" producing cert \"{}\"",
505+
csr.path().display(),
506+
cert.path().display()
507+
);
508+
557509
teardown_warn_only(connector, pwd);
558510
fs::read(cert.path()).with_context(|| {
559511
format!("failed to read file {}", cert.as_ref().display())
@@ -715,3 +667,47 @@ fn teardown_warn_only<P: AsRef<Path>>(mut conn: Child, ret_path: P) {
715667
);
716668
}
717669
}
670+
671+
const RETRY_MAX: usize = 7;
672+
const RETRY_SLEEP: u64 = 5000;
673+
const ALWAYS_SLEEP: u64 = 2000;
674+
675+
/// Execute a command with sleep / retry logic specifically tuned for this
676+
/// use-case. The YubiHSM supports 16 sessions. Unused sessions are reclaimed
677+
/// after 30 seconds of inactivity. For whatever reason using the YubiHSM as a
678+
/// backend for openssl through the pkcs#11 provider causes session exhaustion.
679+
/// Likely someone isn't cleaning up after themselves. To work around this we
680+
/// do two things:
681+
/// - Before executing any openssl command we sleep for 2 seconds. This is
682+
/// intended to keep us from using up all of the available sessions rapidly.
683+
/// - Implement a retry loop as a last line of defense that will run out the
684+
/// session clock in its entirety if necessary.
685+
fn execute_command(cmd: &mut Command) -> Result<()> {
686+
for retry in 0..RETRY_MAX {
687+
debug!("attempt {retry} at executing command: \"{:#?}\"", cmd);
688+
let output = cmd
689+
.output()
690+
.with_context(|| format!("failed to execute command: {cmd:?}"))?;
691+
692+
if output.status.success() {
693+
debug!("command executed successfully");
694+
// Sleep unconditionally after success to run down the session
695+
// reclamation timer on the YubiHSM a bit.
696+
thread::sleep(Duration::from_millis(ALWAYS_SLEEP));
697+
break;
698+
} else {
699+
// sleep & retry on any failure
700+
warn!("command failed with status: {}", output.status);
701+
warn!("stderr: \"{}\"", String::from_utf8_lossy(&output.stderr));
702+
if retry == RETRY_MAX - 1 {
703+
error!("giving up after {RETRY_MAX} attempts");
704+
return Err(CaError::OpensslCmdFail.into());
705+
}
706+
707+
info!("attempt {retry} failed, waiting before retrying ...");
708+
thread::sleep(Duration::from_millis(RETRY_SLEEP));
709+
}
710+
}
711+
712+
Ok(())
713+
}

0 commit comments

Comments
 (0)