Add more security checks for challenge (#40)

* Add mTLS identity to challenge

* Client checks pubkey in challenge before signing

rustica-agent/src/rustica/error.rs

@@ -11,6 +11,7 @@ pub struct ServerError {
 pub enum RefreshError {
     TransportError,
     SigningError,
+    ServerChallengeNotForClientKey,
     UnsupportedMode,
     InvalidUri,
     ConfigurationError(String),
@@ -25,6 +26,7 @@ impl fmt::Display for RefreshError {
         match *self {
             RefreshError::ConfigurationError(ref err) => write!(f, "Configuration is invalid: {}", err),
             RefreshError::TransportError => write!(f, "Transport Error. Generally a TLS issue"),
+            RefreshError::ServerChallengeNotForClientKey => write!(f, "Server challenge is not for your key"),
             RefreshError::SigningError => write!(f, "Signing or verification failed"),
             RefreshError::UnsupportedMode => write!(f, "Attempted to use a curve or cipher not supported by rustica-agent"),
             RefreshError::InvalidUri => write!(f, "Provided address of remote service was invalid"),

rustica-agent/src/rustica/mod.rs

@@ -100,6 +100,14 @@ pub async fn complete_rustica_challenge(
         SSHCertificate::from_string(&response.challenge).map_err(|_| RefreshError::SigningError)?;
     challenge_certificate.signature_key = challenge_certificate.key.clone();
 
+    // We assert that the pubkey in the challenge belongs to the client
+    // This prevents a malicious Rustica server from tricking the client into signing a
+    // malicious SSH certificate for some unknown key.
+    if challenge_certificate.key.fingerprint().hash != ssh_pubkey.fingerprint().hash {
+        error!("The public key in the challenge doesn't match the client's public key");
+        return Err(RefreshError::ServerChallengeNotForClientKey);
+    }
+
     let resigned_certificate = match signatory {
         Signatory::Yubikey(signer) => {
             let signature = signer

rustica/src/server.rs

@@ -209,7 +209,7 @@ fn validate_request(
     })?;
 
     let hmac_challenge = &parsed_certificate.key_id;
-    let hmac_verification = format!("{}-{}", request_time, challenge.pubkey);
+    let hmac_verification = format!("{}-{}-{}", request_time, challenge.pubkey, cert_info.identities.join(","));
     let decoded_challenge =
         hex::decode(&hmac_challenge).map_err(|_| RusticaServerError::BadChallenge)?;
 
@@ -364,7 +364,7 @@ impl Rustica for RusticaServer {
             .as_secs()
             .to_string();
         let pubkey = &request.pubkey;
-        let challenge = format!("{}-{}", timestamp, pubkey);
+        let challenge = format!("{}-{}-{}", timestamp, pubkey, mtls_identities.join(","));
         let tag = hmac::sign(&self.hmac_key, challenge.as_bytes());
 
         // Build an SSHCertificate as a challenge