Skip to content

Commit

Permalink
storage: fix compatibility on fetching token for registry backend
Browse files Browse the repository at this point in the history
The registry backend received an unauthorized error from Harbor registry
when fetching registry token by HTTP GET method, the bug is introduced
from https://github.com/dragonflyoss/image-service/pull/1425/files#diff-f7ce8f265a570c66eae48c85e0f5b6f29fdaec9cf2ee2eded95810fe320d80e1L263.

We should insert the basic auth header to ensure the compatibility of
fetching token by HTTP GET method.

This refers to containerd implementation: https://github.com/containerd/containerd/blob/dc7dba9c20f7210c38e8255487fc0ee12692149d/remotes/docker/auth/fetch.go#L187

The change has been tested for Harbor v2.9.

Signed-off-by: Yan Song <[email protected]>
  • Loading branch information
imeoer committed Oct 18, 2023
1 parent dda2310 commit 369e33f
Showing 1 changed file with 32 additions and 18 deletions.
50 changes: 32 additions & 18 deletions storage/src/backend/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ struct BearerAuth {
realm: String,
service: String,
scope: String,
header: Option<HeaderValue>,
}

#[derive(Debug)]
Expand Down Expand Up @@ -255,8 +254,8 @@ impl RegistryState {
} else {
match self.get_token_with_post(&auth, connection) {
Ok(resp) => resp,
Err(err) => {
warn!("retry http GET method to get auth token: {}", err);
Err(_) => {
warn!("retry http GET method to get auth token");
let resp = self.get_token_with_get(&auth, connection)?;
// Cache http method for next use.
self.cached_auth_using_http_get.set(self.host.clone(), true);
Expand Down Expand Up @@ -301,18 +300,22 @@ impl RegistryState {
form.insert("passward".to_string(), self.password.clone());
form.insert("client_id".to_string(), REGISTRY_CLIENT_ID.to_string());

let mut headers = HeaderMap::new();

let token_resp = connection
.call::<&[u8]>(
Method::POST,
auth.realm.as_str(),
None,
Some(ReqBody::Form(form)),
&mut headers,
&mut HeaderMap::new(),
true,
)
.map_err(|e| einval!(format!("registry auth server request failed {:?}", e)))?;
.map_err(|e| {
warn!(
"failed to request registry auth server by POST method: {:?}",
e
);
einval!()
})?;

Ok(token_resp)
}
Expand All @@ -334,6 +337,16 @@ impl RegistryState {

let mut headers = HeaderMap::new();

// Insert the basic auth header to ensure the compatibility (e.g. Harbor registry)
// of fetching token by HTTP GET method.
// This refers containerd implementation: https://github.com/containerd/containerd/blob/dc7dba9c20f7210c38e8255487fc0ee12692149d/remotes/docker/auth/fetch.go#L187
if let Some(auth) = &self.auth {
headers.insert(
HEADER_AUTHORIZATION,
format!("Basic {}", auth).parse().unwrap(),
);
}

let token_resp = connection
.call::<&[u8]>(
Method::GET,
Expand All @@ -343,7 +356,13 @@ impl RegistryState {
&mut headers,
true,
)
.map_err(|e| einval!(format!("registry auth server request failed {:?}", e)))?;
.map_err(|e| {
warn!(
"failed to request registry auth server by GET method: {:?}",
e
);
einval!()
})?;

Ok(token_resp)
}
Expand All @@ -364,7 +383,7 @@ impl RegistryState {

/// Parse `www-authenticate` response header respond from registry server
/// The header format like: `Bearer realm="https://auth.my-registry.com/token",service="my-registry.com",scope="repository:test/repo:pull,push"`
fn parse_auth(source: &HeaderValue, auth: &Option<String>) -> Option<Auth> {
fn parse_auth(source: &HeaderValue) -> Option<Auth> {
let source = source.to_str().unwrap();
let source: Vec<&str> = source.splitn(2, ' ').collect();
if source.len() < 2 {
Expand Down Expand Up @@ -401,15 +420,10 @@ impl RegistryState {
return None;
}

let header = auth
.as_ref()
.map(|auth| HeaderValue::from_str(&format!("Basic {}", auth)).unwrap());

Some(Auth::Bearer(BearerAuth {
realm: (*paras.get("realm").unwrap()).to_string(),
service: (*paras.get("service").unwrap()).to_string(),
scope: (*paras.get("scope").unwrap()).to_string(),
header,
}))
}
_ => None,
Expand Down Expand Up @@ -571,7 +585,7 @@ impl RegistryReader {

if let Some(resp_auth_header) = resp.headers().get(HEADER_WWW_AUTHENTICATE) {
// Get token from registry authorization server
if let Some(auth) = RegistryState::parse_auth(resp_auth_header, &self.state.auth) {
if let Some(auth) = RegistryState::parse_auth(resp_auth_header) {
let auth_header = self
.state
.get_auth_header(auth, &self.connection)
Expand Down Expand Up @@ -1066,7 +1080,7 @@ mod tests {
fn test_parse_auth() {
let str = "Bearer realm=\"https://auth.my-registry.com/token\",service=\"my-registry.com\",scope=\"repository:test/repo:pull,push\"";
let header = HeaderValue::from_str(str).unwrap();
let auth = RegistryState::parse_auth(&header, &None).unwrap();
let auth = RegistryState::parse_auth(&header).unwrap();
match auth {
Auth::Bearer(auth) => {
assert_eq!(&auth.realm, "https://auth.my-registry.com/token");
Expand All @@ -1078,15 +1092,15 @@ mod tests {

let str = "Basic realm=\"https://auth.my-registry.com/token\"";
let header = HeaderValue::from_str(str).unwrap();
let auth = RegistryState::parse_auth(&header, &None).unwrap();
let auth = RegistryState::parse_auth(&header).unwrap();
match auth {
Auth::Basic(auth) => assert_eq!(&auth.realm, "https://auth.my-registry.com/token"),
_ => panic!("failed to pase `Bearer` authentication header"),
}

let str = "Base realm=\"https://auth.my-registry.com/token\"";
let header = HeaderValue::from_str(str).unwrap();
assert!(RegistryState::parse_auth(&header, &None).is_none());
assert!(RegistryState::parse_auth(&header).is_none());
}

#[test]
Expand Down

0 comments on commit 369e33f

Please sign in to comment.