Skip to content

Commit af31aab

Browse files
authored
inbound: Return HTTP-level authorization errors (#1220)
The initial policy/authorization implementation only operates on connections. When an connection is not authorized, the connection is dropped and no information is passed back to the client. This also means that all authorizations are made at connect-time and policy changes--especially those that revoke access--are not observed until a new connection is established. This is at the very least awkward when trying to test policies. This change modifies the inbound proxy's behavior for HTTP connections: when a server configures a port to use (or be detected) as HTTP, the authorization decision is now deferred until a request is processed. As each request is handled, the policy's state is checked to determine whether the connection is still permitted. If the connection is not authorized, a `403 Forbidden` status code is returned for HTTP requests (and the grpc-status `PermissionDenied` is used for gRPC requests). The http error metrics also now reflect an `unauthorized` reason. This change, unfortunately, adds some wrinkles to our transport metric labeling: HTTP connections will no longer have a `saz_name` annotation--as no authorization policy is associated with these connections (though they will continue to include the `srv_name` from when the connection was established).
1 parent acc6e47 commit af31aab

File tree

15 files changed

+348
-205
lines changed

15 files changed

+348
-205
lines changed

linkerd/app/core/src/classify.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ fn grpc_class(headers: &http::HeaderMap) -> Option<Class> {
193193
| grpc::Code::DeadlineExceeded
194194
| grpc::Code::Internal
195195
| grpc::Code::Unavailable
196+
| grpc::Code::PermissionDenied
196197
| grpc::Code::DataLoss => SuccessOrFailure::Failure,
197198
_ => SuccessOrFailure::Success,
198199
};

linkerd/app/core/src/errors.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::transport::DeniedUnauthorized;
12
use http::{header::HeaderValue, StatusCode};
23
use linkerd_errno::Errno;
34
use linkerd_error::Error;
@@ -62,6 +63,7 @@ pub enum Reason {
6263
FailFast,
6364
GatewayLoop,
6465
NotFound,
66+
Unauthorized,
6567
Unexpected,
6668
}
6769

@@ -307,7 +309,7 @@ fn set_http_status(
307309
builder.status(StatusCode::SERVICE_UNAVAILABLE)
308310
} else if error.is::<tower::timeout::error::Elapsed>() {
309311
builder.status(StatusCode::SERVICE_UNAVAILABLE)
310-
} else if error.is::<IdentityRequired>() {
312+
} else if error.is::<IdentityRequired>() || error.is::<DeniedUnauthorized>() {
311313
builder.status(StatusCode::FORBIDDEN)
312314
} else if let Some(source) = error.source() {
313315
set_http_status(builder, source)
@@ -351,6 +353,13 @@ fn set_grpc_status(
351353
HeaderValue::from_static("proxy dispatch timed out"),
352354
);
353355
code
356+
} else if error.is::<DeniedUnauthorized>() {
357+
let code = Code::PermissionDenied;
358+
headers.insert(GRPC_STATUS, code_header(code));
359+
if let Ok(msg) = HeaderValue::from_str(&error.to_string()) {
360+
headers.insert(GRPC_MESSAGE, msg);
361+
}
362+
code
354363
} else if error.is::<IdentityRequired>() {
355364
let code = Code::FailedPrecondition;
356365
headers.insert(GRPC_STATUS, code_header(code));
@@ -433,6 +442,8 @@ impl LabelError {
433442
Reason::FailFast
434443
} else if err.is::<tower::timeout::error::Elapsed>() {
435444
Reason::DispatchTimeout
445+
} else if err.is::<DeniedUnauthorized>() {
446+
Reason::Unauthorized
436447
} else if err.is::<IdentityRequired>() {
437448
Reason::IdentityRequired
438449
} else if let Some(e) = err.downcast_ref::<std::io::Error>() {
@@ -466,6 +477,7 @@ impl FmtLabels for Reason {
466477
Reason::GatewayLoop => "gateway loop",
467478
Reason::NotFound => "not found",
468479
Reason::Io(_) => "i/o",
480+
Reason::Unauthorized => "unauthorized",
469481
Reason::Unexpected => "unexpected",
470482
}
471483
)?;

linkerd/app/inbound/src/accept.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ mod tests {
120120
#[tokio::test(flavor = "current_thread")]
121121
async fn default_allow() {
122122
let (io, _) = io::duplex(1);
123-
let policies = Store::fixed(
123+
let (policies, _tx) = Store::fixed(
124124
ServerPolicy {
125125
protocol: linkerd_server_policy::Protocol::Opaque,
126126
authorizations: vec![Authorization {
@@ -144,7 +144,7 @@ mod tests {
144144

145145
#[tokio::test(flavor = "current_thread")]
146146
async fn default_deny() {
147-
let policies = Store::fixed(DefaultPolicy::Deny, None);
147+
let (policies, _tx) = Store::fixed(DefaultPolicy::Deny, None);
148148
let (io, _) = io::duplex(1);
149149
inbound()
150150
.with_stack(new_ok())
@@ -158,7 +158,7 @@ mod tests {
158158

159159
#[tokio::test(flavor = "current_thread")]
160160
async fn direct() {
161-
let policies = Store::fixed(DefaultPolicy::Deny, None);
161+
let (policies, _tx) = Store::fixed(DefaultPolicy::Deny, None);
162162
let (io, _) = io::duplex(1);
163163
inbound()
164164
.with_stack(new_panic("detect stack must not be built"))

0 commit comments

Comments
 (0)