Skip to content

Commit 9e1068b

Browse files
authored
fix(client): encode the negotiated hash algorithm for RSA certificates (#764)
1 parent 0a3a63c commit 9e1068b

1 file changed

Lines changed: 105 additions & 3 deletions

File tree

russh/src/client/encrypted.rs

Lines changed: 105 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,7 @@ impl Session {
255255
key: key.clone(),
256256
hash_alg,
257257
},
258+
None,
258259
&mut self.common.buffer,
259260
)?;
260261
let len = self.common.buffer.len();
@@ -284,6 +285,7 @@ impl Session {
284285
let i = enc.client_make_to_sign(
285286
&self.common.auth_user,
286287
&PublicKeyOrCertificate::Certificate(cert.clone()),
288+
hash_alg,
287289
&mut self.common.buffer,
288290
)?;
289291
let len = self.common.buffer.len();
@@ -1211,6 +1213,100 @@ mod tests {
12111213
assert_eq!(Vec::<u8>::decode(&mut mic).unwrap(), b"mic".to_vec());
12121214
ensure_end(&mic).unwrap();
12131215
}
1216+
1217+
1218+
fn rsa_user_certificate() -> ssh_key::Certificate {
1219+
let subject = ssh_key::PrivateKey::random(
1220+
&mut rand::rng(),
1221+
ssh_key::Algorithm::Rsa { hash: None },
1222+
)
1223+
.unwrap();
1224+
let ca = ssh_key::PrivateKey::random(&mut rand::rng(), ssh_key::Algorithm::Ed25519)
1225+
.unwrap();
1226+
let mut builder = ssh_key::certificate::Builder::new_with_random_nonce(
1227+
&mut rand::rng(),
1228+
subject.public_key(),
1229+
0,
1230+
u64::MAX,
1231+
)
1232+
.unwrap();
1233+
builder.key_id("rsa-user-cert").unwrap();
1234+
builder
1235+
.cert_type(ssh_key::certificate::CertType::User)
1236+
.unwrap();
1237+
builder.valid_principal("alice").unwrap();
1238+
builder.sign(&ca).unwrap()
1239+
}
1240+
1241+
fn decode_userauth_certificate_algorithm(
1242+
mut request: &[u8],
1243+
expected_has_signature: bool,
1244+
) -> String {
1245+
assert_eq!(u8::decode(&mut request).unwrap(), msg::USERAUTH_REQUEST);
1246+
assert_eq!(String::decode(&mut request).unwrap(), "alice");
1247+
assert_eq!(String::decode(&mut request).unwrap(), "ssh-connection");
1248+
assert_eq!(String::decode(&mut request).unwrap(), "publickey");
1249+
assert_eq!(
1250+
u8::decode(&mut request).unwrap(),
1251+
u8::from(expected_has_signature)
1252+
);
1253+
let algorithm = String::decode(&mut request).unwrap();
1254+
let _certificate = Vec::<u8>::decode(&mut request).unwrap();
1255+
ensure_end(&request).unwrap();
1256+
algorithm
1257+
}
1258+
1259+
#[test]
1260+
fn rsa_future_certificate_hash_is_encoded_in_probe_and_signed_requests() {
1261+
let certificate = rsa_user_certificate();
1262+
for (hash_alg, expected) in [
1263+
(
1264+
Some(ssh_key::HashAlg::Sha512),
1265+
"rsa-sha2-512-cert-v01@openssh.com",
1266+
),
1267+
(
1268+
Some(ssh_key::HashAlg::Sha256),
1269+
"rsa-sha2-256-cert-v01@openssh.com",
1270+
),
1271+
(None, "ssh-rsa-cert-v01@openssh.com"),
1272+
] {
1273+
let mut encrypted = test_encrypted();
1274+
encrypted
1275+
.write_auth_request(
1276+
"alice",
1277+
&auth::Method::FutureCertificate {
1278+
cert: certificate.clone(),
1279+
hash_alg,
1280+
},
1281+
)
1282+
.unwrap();
1283+
let probe_payloads = payloads(&encrypted.write);
1284+
assert_eq!(probe_payloads.len(), 1);
1285+
assert_eq!(
1286+
decode_userauth_certificate_algorithm(probe_payloads[0], false),
1287+
expected
1288+
);
1289+
1290+
let mut signed = Vec::new();
1291+
encrypted
1292+
.client_make_to_sign(
1293+
"alice",
1294+
&PublicKeyOrCertificate::Certificate(certificate.clone()),
1295+
hash_alg,
1296+
&mut signed,
1297+
)
1298+
.unwrap();
1299+
let mut signed_request = signed.as_slice();
1300+
assert_eq!(
1301+
Vec::<u8>::decode(&mut signed_request).unwrap(),
1302+
b"session-id"
1303+
);
1304+
assert_eq!(
1305+
decode_userauth_certificate_algorithm(signed_request, true),
1306+
expected
1307+
);
1308+
}
1309+
}
12141310
}
12151311

12161312
impl Encrypted {
@@ -1276,13 +1372,14 @@ impl Encrypted {
12761372
key.to_bytes()?.as_slice().encode(&mut self.write)?;
12771373
true
12781374
}
1279-
auth::Method::FutureCertificate { ref cert, .. } => {
1375+
auth::Method::FutureCertificate { ref cert, hash_alg } => {
12801376
user.as_bytes().encode(&mut self.write)?;
12811377
"ssh-connection".encode(&mut self.write)?;
12821378
"publickey".encode(&mut self.write)?;
12831379
self.write.push(0); // This is a probe
12841380

12851381
cert.algorithm()
1382+
.with_hash_alg(hash_alg)
12861383
.to_certificate_type()
12871384
.encode(&mut self.write)?;
12881385
cert.to_bytes()?.as_slice().encode(&mut self.write)?;
@@ -1365,6 +1462,7 @@ impl Encrypted {
13651462
&mut self,
13661463
user: &str,
13671464
key: &PublicKeyOrCertificate,
1465+
certificate_hash_alg: Option<ssh_key::HashAlg>,
13681466
buffer: &mut Vec<u8>,
13691467
) -> Result<usize, crate::Error> {
13701468
buffer.clear();
@@ -1379,7 +1477,10 @@ impl Encrypted {
13791477

13801478
match key {
13811479
PublicKeyOrCertificate::Certificate(cert) => {
1382-
cert.algorithm().to_certificate_type().encode(buffer)?;
1480+
cert.algorithm()
1481+
.with_hash_alg(certificate_hash_alg)
1482+
.to_certificate_type()
1483+
.encode(buffer)?;
13831484
cert.to_bytes()?.encode(buffer)?;
13841485
}
13851486
PublicKeyOrCertificate::PublicKey { key, hash_alg } => {
@@ -1399,7 +1500,7 @@ impl Encrypted {
13991500
match method {
14001501
auth::Method::PublicKey { key } => {
14011502
let i0 =
1402-
self.client_make_to_sign(user, &PublicKeyOrCertificate::from(key), buffer)?;
1503+
self.client_make_to_sign(user, &PublicKeyOrCertificate::from(key), None, buffer)?;
14031504

14041505
// Extend with self-signature.
14051506
sign_with_hash_alg(key, buffer)?.encode(&mut *buffer)?;
@@ -1413,6 +1514,7 @@ impl Encrypted {
14131514
let i0 = self.client_make_to_sign(
14141515
user,
14151516
&PublicKeyOrCertificate::Certificate(cert.clone()),
1517+
None,
14161518
buffer,
14171519
)?;
14181520

0 commit comments

Comments
 (0)