Skip to content

Commit 704d911

Browse files
author
Nils Bars
committed
Show students a readable message when container startup fails
Classify web API failures in the SSH auth path by kind: HTTP 4xx on unsigned endpoints (/api/ssh-authenticated) reject auth as before, while 5xx, transport errors, decode errors, and 4xx on signed endpoints (/api/provision, which surface HMAC drift as 400) accept auth and stash an operational-error message. shell/exec/subsystem requests detect the stashed message, reply with channel_success, write it to the session channel's stderr, send exit_status 1, and close the channel. Each failure gets a fresh UUID that is logged alongside the underlying error so staff can correlate a student's error code to the backend trace.
1 parent 946434d commit 704d911

2 files changed

Lines changed: 242 additions & 60 deletions

File tree

ssh-reverse-proxy/src/api.rs

Lines changed: 129 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,80 @@
33
use anyhow::{anyhow, Result};
44
use base64::Engine;
55
use hmac::{Hmac, Mac};
6-
use reqwest::Client;
6+
use reqwest::{Client, StatusCode};
77
use serde::{Deserialize, Serialize};
88
use tracing::{debug, info, error, instrument};
99

10+
/// Error kind for API calls that drive the SSH auth path.
11+
///
12+
/// The SSH proxy uses this to decide whether a backend failure should be
13+
/// surfaced to the student as an auth rejection (they did something wrong)
14+
/// or as an operational error (the backend broke and it's not their fault).
15+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
16+
pub enum ApiErrorKind {
17+
/// HTTP 4xx — the request itself is invalid. Most commonly a key that is
18+
/// not registered for this exercise, or the exercise name is unknown.
19+
/// These are treated as genuine auth rejections.
20+
ClientError,
21+
/// HTTP 5xx, transport failure, or response decoding failure. The student
22+
/// cannot resolve these themselves; the proxy accepts auth and delivers
23+
/// an operational-error message over the channel so they know to
24+
/// contact staff.
25+
ServerError,
26+
}
27+
28+
#[derive(Debug, thiserror::Error)]
29+
#[error("{kind:?}: {detail}")]
30+
pub struct ApiError {
31+
pub kind: ApiErrorKind,
32+
/// Human-readable detail for logging. Includes the HTTP status and raw
33+
/// body when available so staff can correlate with web logs.
34+
pub detail: String,
35+
}
36+
37+
impl ApiError {
38+
/// Classify an HTTP failure by status code: 4xx → `ClientError`,
39+
/// else → `ServerError`. Use this for endpoints where a 4xx genuinely
40+
/// reflects caller input (e.g. a pubkey the backend doesn't recognize).
41+
fn from_status_unsigned(status: StatusCode, body: &str) -> Self {
42+
let kind = if status.is_client_error() {
43+
ApiErrorKind::ClientError
44+
} else {
45+
ApiErrorKind::ServerError
46+
};
47+
Self {
48+
kind,
49+
detail: format!("HTTP {}: {}", status, body),
50+
}
51+
}
52+
53+
/// Any non-2xx is treated as `ServerError`. Use this for HMAC-signed
54+
/// endpoints (e.g. `/api/provision`), where a 4xx typically means the
55+
/// signature check on the web side failed — an operational problem
56+
/// (stale `SSH_TO_WEB_KEY`, clock skew, etc.), not something the
57+
/// student can resolve.
58+
fn from_status_signed(status: StatusCode, body: &str) -> Self {
59+
Self {
60+
kind: ApiErrorKind::ServerError,
61+
detail: format!("HTTP {}: {}", status, body),
62+
}
63+
}
64+
65+
fn transport(err: reqwest::Error) -> Self {
66+
Self {
67+
kind: ApiErrorKind::ServerError,
68+
detail: format!("transport error: {}", err),
69+
}
70+
}
71+
72+
fn decode(err: impl std::fmt::Display) -> Self {
73+
Self {
74+
kind: ApiErrorKind::ServerError,
75+
detail: format!("response decode error: {}", err),
76+
}
77+
}
78+
}
79+
1080
/// API client for communicating with the REF web server.
1181
#[derive(Clone)]
1282
pub struct ApiClient {
@@ -158,12 +228,16 @@ impl ApiClient {
158228
}
159229

160230
/// Authenticate an SSH connection and get user permissions.
231+
///
232+
/// Returns a typed `ApiError` so the SSH layer can distinguish genuine
233+
/// auth failures (4xx — reject the SSH auth) from operational failures
234+
/// (5xx/transport/decode — accept auth and show an error message).
161235
#[instrument(skip(self, pubkey))]
162236
pub async fn ssh_authenticated(
163237
&self,
164238
exercise_name: &str,
165239
pubkey: &str,
166-
) -> Result<SshAuthenticatedResponse> {
240+
) -> std::result::Result<SshAuthenticatedResponse, ApiError> {
167241
let request = SshAuthenticatedRequest {
168242
name: exercise_name.to_string(),
169243
pubkey: pubkey.to_string(),
@@ -178,27 +252,22 @@ impl ApiClient {
178252
.post(&url)
179253
.json(&request)
180254
.send()
181-
.await?;
255+
.await
256+
.map_err(ApiError::transport)?;
182257

183258
let status = response.status();
184259
if !status.is_success() {
185260
let body = response.text().await.unwrap_or_default();
186-
use std::io::Write;
187-
// Escape newlines for single-line logging
188261
let body_escaped = body.replace('\n', "\\n").replace('\r', "\\r");
189-
eprintln!("[SSH-PROXY] ssh_authenticated FAILED: status={}, body={}", status, body_escaped);
190-
std::io::stderr().flush().ok();
191262
error!("[API] ssh_authenticated FAILED: status={}, body={}", status, body_escaped);
192-
return Err(anyhow!(
193-
"SSH authentication failed with status: {}",
194-
status
195-
));
263+
return Err(ApiError::from_status_unsigned(status, &body_escaped));
196264
}
197265

198-
let body_text = response.text().await?;
266+
let body_text = response.text().await.map_err(ApiError::transport)?;
199267
info!("[API] ssh_authenticated response: {}", body_text);
200268

201-
let auth_response: SshAuthenticatedResponse = serde_json::from_str(&body_text)?;
269+
let auth_response: SshAuthenticatedResponse =
270+
serde_json::from_str(&body_text).map_err(ApiError::decode)?;
202271
debug!(
203272
"Authenticated: instance_id={}, forwarding={}",
204273
auth_response.instance_id, auth_response.tcp_forwarding_allowed
@@ -207,41 +276,43 @@ impl ApiClient {
207276
}
208277

209278
/// Provision a container and get connection details.
279+
///
280+
/// Returns a typed `ApiError` for the same reasons as `ssh_authenticated`.
210281
#[instrument(skip(self, pubkey))]
211282
pub async fn provision(
212283
&self,
213284
exercise_name: &str,
214285
pubkey: &str,
215-
) -> Result<ProvisionResponse> {
286+
) -> std::result::Result<ProvisionResponse, ApiError> {
216287
let request = ProvisionRequest {
217288
exercise_name: exercise_name.to_string(),
218289
pubkey: pubkey.to_string(),
219290
};
220-
let payload = serde_json::to_string(&request)?;
291+
let payload = serde_json::to_string(&request).map_err(ApiError::decode)?;
221292
let signed = self.sign_payload(&payload);
222293

223294
let url = format!("{}/api/provision", self.base_url);
224295
debug!("Provisioning container for exercise: {}", exercise_name);
225296

226-
// Send signed string as JSON (Python: requests.post(..., json=signed_string))
227297
let response = self
228298
.client
229299
.post(&url)
230300
.json(&signed)
231301
.send()
232-
.await?;
302+
.await
303+
.map_err(ApiError::transport)?;
233304

234-
if !response.status().is_success() {
235-
let status = response.status();
305+
let status = response.status();
306+
if !status.is_success() {
236307
let body = response.text().await.unwrap_or_default();
237-
return Err(anyhow!(
238-
"Provisioning failed with status {}: {}",
239-
status,
240-
body
241-
));
308+
let body_escaped = body.replace('\n', "\\n").replace('\r', "\\r");
309+
error!("[API] provision FAILED: status={}, body={}", status, body_escaped);
310+
return Err(ApiError::from_status_signed(status, &body_escaped));
242311
}
243312

244-
let provision_response: ProvisionResponse = response.json().await?;
313+
let body_text = response.text().await.map_err(ApiError::transport)?;
314+
let provision_response: ProvisionResponse =
315+
serde_json::from_str(&body_text).map_err(ApiError::decode)?;
245316
debug!("Provisioned container at IP: {}", provision_response.ip);
246317
Ok(provision_response)
247318
}
@@ -277,4 +348,37 @@ mod tests {
277348
let signed2 = client.sign_payload(r#"{"username": "test"}"#);
278349
assert_eq!(signed1, signed2);
279350
}
351+
352+
#[test]
353+
fn unsigned_4xx_is_client_error() {
354+
let err = ApiError::from_status_unsigned(StatusCode::FORBIDDEN, "nope");
355+
assert_eq!(err.kind, ApiErrorKind::ClientError);
356+
}
357+
358+
#[test]
359+
fn unsigned_5xx_is_server_error() {
360+
let err = ApiError::from_status_unsigned(StatusCode::INTERNAL_SERVER_ERROR, "boom");
361+
assert_eq!(err.kind, ApiErrorKind::ServerError);
362+
}
363+
364+
#[test]
365+
fn signed_4xx_is_server_error() {
366+
// HMAC validation failures from signed endpoints surface as 4xx but
367+
// are operational problems (e.g. key rotation / drift), not the
368+
// student's fault.
369+
let err = ApiError::from_status_signed(StatusCode::BAD_REQUEST, "bad signature");
370+
assert_eq!(err.kind, ApiErrorKind::ServerError);
371+
}
372+
373+
#[test]
374+
fn signed_5xx_is_server_error() {
375+
let err = ApiError::from_status_signed(StatusCode::INTERNAL_SERVER_ERROR, "boom");
376+
assert_eq!(err.kind, ApiErrorKind::ServerError);
377+
}
378+
379+
#[test]
380+
fn decode_error_is_server_error() {
381+
let err = ApiError::decode("parse fail");
382+
assert_eq!(err.kind, ApiErrorKind::ServerError);
383+
}
280384
}

0 commit comments

Comments
 (0)