-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Auto merge of #12310 - weihanglo:asymmetric-challenge, r=Eh2406
feat(crates-io): expose HTTP headers and Error type ### What does this PR try to resolve? This is part of #11521. [RFC 3231] mentions the authentication process could have an additional **challenge-response mechanism** to prevent potential replay attacks. The challenge usually comes from HTTP `www-authenticate` header as a opaque string. When a client gets a 401/403 response with such a challenge, it may attach the `challenge` to the payload and request again to anwser the challenge. ``` ➡️ cargo requests ⬅️ server responds with `www-authenticate` containing some opaque challenge string ➡️ cargo automatically requests again without any user perception ⬅️ server responds ok ``` However, `crates-io` crate doesn't expose HTTP headers. There is no access to `www-authenticate` header. This PR make it expose HTTP headers and the custom `Error` type, so `cargo` can access and do further on the authentication process. [RFC 3231]: https://rust-lang.github.io/rfcs/3231-cargo-asymmetric-tokens.html#the-authentication-process
- Loading branch information
Showing
5 changed files
with
87 additions
and
95 deletions.
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,18 +1,18 @@ | ||
| #![allow(clippy::all)] | ||
|
|
||
| use std::collections::BTreeMap; | ||
| use std::fmt; | ||
| use std::fs::File; | ||
| use std::io::prelude::*; | ||
| use std::io::{Cursor, SeekFrom}; | ||
| use std::time::Instant; | ||
|
|
||
| use anyhow::{bail, format_err, Context, Result}; | ||
| use curl::easy::{Easy, List}; | ||
| use percent_encoding::{percent_encode, NON_ALPHANUMERIC}; | ||
| use serde::{Deserialize, Serialize}; | ||
| use url::Url; | ||
|
|
||
| pub type Result<T> = std::result::Result<T, Error>; | ||
|
|
||
| pub struct Registry { | ||
| /// The base URL for issuing API requests. | ||
| host: String, | ||
|
|
@@ -125,67 +125,62 @@ struct Crates { | |
| meta: TotalCrates, | ||
| } | ||
|
|
||
| #[derive(Debug)] | ||
| pub enum ResponseError { | ||
| Curl(curl::Error), | ||
| /// Error returned when interacting with a registry. | ||
| #[derive(Debug, thiserror::Error)] | ||
| pub enum Error { | ||
| /// Error from libcurl. | ||
| #[error(transparent)] | ||
| Curl(#[from] curl::Error), | ||
|
|
||
| /// Error from seriailzing the request payload and deserialzing the | ||
| /// response body (like response body didn't match expected structure). | ||
| #[error(transparent)] | ||
| Json(#[from] serde_json::Error), | ||
|
|
||
| /// Error from IO. Mostly from reading the tarball to upload. | ||
| #[error("failed to seek tarball")] | ||
| Io(#[from] std::io::Error), | ||
|
|
||
| /// Response body was not valid utf8. | ||
| #[error("invalid response body from server")] | ||
| Utf8(#[from] std::string::FromUtf8Error), | ||
|
|
||
| /// Error from API response containing JSON field `errors.details`. | ||
| #[error( | ||
| "the remote server responded with an error{}: {}", | ||
| status(*code), | ||
| errors.join(", "), | ||
| )] | ||
| Api { | ||
| code: u32, | ||
| headers: Vec<String>, | ||
| errors: Vec<String>, | ||
| }, | ||
|
|
||
| /// Error from API response which didn't have pre-programmed `errors.details`. | ||
| #[error( | ||
| "failed to get a 200 OK response, got {code}\nheaders:\n\t{}\nbody:\n{body}", | ||
| headers.join("\n\t"), | ||
| )] | ||
| Code { | ||
| code: u32, | ||
| headers: Vec<String>, | ||
| body: String, | ||
| }, | ||
| Other(anyhow::Error), | ||
| } | ||
|
|
||
| impl std::error::Error for ResponseError { | ||
| fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { | ||
| match self { | ||
| ResponseError::Curl(..) => None, | ||
| ResponseError::Api { .. } => None, | ||
| ResponseError::Code { .. } => None, | ||
| ResponseError::Other(e) => Some(e.as_ref()), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl fmt::Display for ResponseError { | ||
| fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { | ||
| match self { | ||
| ResponseError::Curl(e) => write!(f, "{}", e), | ||
| ResponseError::Api { code, errors } => { | ||
| f.write_str("the remote server responded with an error")?; | ||
| if *code != 200 { | ||
| write!(f, " (status {} {})", code, reason(*code))?; | ||
| }; | ||
| write!(f, ": {}", errors.join(", ")) | ||
| } | ||
| ResponseError::Code { | ||
| code, | ||
| headers, | ||
| body, | ||
| } => write!( | ||
| f, | ||
| "failed to get a 200 OK response, got {}\n\ | ||
| headers:\n\ | ||
| \t{}\n\ | ||
| body:\n\ | ||
| {}", | ||
| code, | ||
| headers.join("\n\t"), | ||
| body | ||
| ), | ||
| ResponseError::Other(..) => write!(f, "invalid response from server"), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl From<curl::Error> for ResponseError { | ||
| fn from(error: curl::Error) -> Self { | ||
| ResponseError::Curl(error) | ||
| } | ||
| /// Reason why the token was invalid. | ||
| #[error("{0}")] | ||
| InvalidToken(&'static str), | ||
|
|
||
| /// Server was unavailable and timeouted. Happened when uploading a way | ||
| /// too large tarball to crates.io. | ||
| #[error( | ||
| "Request timed out after 30 seconds. If you're trying to \ | ||
| upload a crate it may be too large. If the crate is under \ | ||
| 10MB in size, you can email [email protected] for assistance.\n\ | ||
| Total size was {0}." | ||
| )] | ||
| Timeout(u64), | ||
| } | ||
|
|
||
| impl Registry { | ||
|
|
@@ -221,10 +216,9 @@ impl Registry { | |
| } | ||
|
|
||
| fn token(&self) -> Result<&str> { | ||
| let token = match self.token.as_ref() { | ||
| Some(s) => s, | ||
| None => bail!("no upload token found, please run `cargo login`"), | ||
| }; | ||
| let token = self.token.as_ref().ok_or_else(|| { | ||
| Error::InvalidToken("no upload token found, please run `cargo login`") | ||
| })?; | ||
| check_token(token)?; | ||
| Ok(token) | ||
| } | ||
|
|
@@ -270,12 +264,8 @@ impl Registry { | |
| // This checks the length using seeking instead of metadata, because | ||
| // on some filesystems, getting the metadata will fail because | ||
| // the file was renamed in ops::package. | ||
| let tarball_len = tarball | ||
| .seek(SeekFrom::End(0)) | ||
| .with_context(|| "failed to seek tarball")?; | ||
| tarball | ||
| .seek(SeekFrom::Start(0)) | ||
| .with_context(|| "failed to seek tarball")?; | ||
| let tarball_len = tarball.seek(SeekFrom::End(0))?; | ||
| tarball.seek(SeekFrom::Start(0))?; | ||
| let header = { | ||
| let mut w = Vec::new(); | ||
| w.extend(&(json.len() as u32).to_le_bytes()); | ||
|
|
@@ -300,18 +290,12 @@ impl Registry { | |
| let body = self | ||
| .handle(&mut |buf| body.read(buf).unwrap_or(0)) | ||
| .map_err(|e| match e { | ||
| ResponseError::Code { code, .. } | ||
| Error::Code { code, .. } | ||
| if code == 503 | ||
| && started.elapsed().as_secs() >= 29 | ||
| && self.host_is_crates_io() => | ||
| { | ||
| format_err!( | ||
| "Request timed out after 30 seconds. If you're trying to \ | ||
| upload a crate it may be too large. If the crate is under \ | ||
| 10MB in size, you can email [email protected] for assistance.\n\ | ||
| Total size was {}.", | ||
| tarball_len | ||
| ) | ||
| Error::Timeout(tarball_len) | ||
| } | ||
| _ => e.into(), | ||
| })?; | ||
|
|
@@ -410,10 +394,7 @@ impl Registry { | |
| } | ||
| } | ||
|
|
||
| fn handle( | ||
| &mut self, | ||
| read: &mut dyn FnMut(&mut [u8]) -> usize, | ||
| ) -> std::result::Result<String, ResponseError> { | ||
| fn handle(&mut self, read: &mut dyn FnMut(&mut [u8]) -> usize) -> Result<String> { | ||
| let mut headers = Vec::new(); | ||
| let mut body = Vec::new(); | ||
| { | ||
|
|
@@ -427,28 +408,29 @@ impl Registry { | |
| // Headers contain trailing \r\n, trim them to make it easier | ||
| // to work with. | ||
| let s = String::from_utf8_lossy(data).trim().to_string(); | ||
| // Don't let server sneak extra lines anywhere. | ||
| if s.contains('\n') { | ||
| return true; | ||
| } | ||
| headers.push(s); | ||
| true | ||
| })?; | ||
| handle.perform()?; | ||
| } | ||
|
|
||
| let body = match String::from_utf8(body) { | ||
| Ok(body) => body, | ||
| Err(..) => { | ||
| return Err(ResponseError::Other(format_err!( | ||
| "response body was not valid utf-8" | ||
| ))) | ||
| } | ||
| }; | ||
| let body = String::from_utf8(body)?; | ||
| let errors = serde_json::from_str::<ApiErrorList>(&body) | ||
| .ok() | ||
| .map(|s| s.errors.into_iter().map(|s| s.detail).collect::<Vec<_>>()); | ||
|
|
||
| match (self.handle.response_code()?, errors) { | ||
| (0, None) | (200, None) => Ok(body), | ||
| (code, Some(errors)) => Err(ResponseError::Api { code, errors }), | ||
| (code, None) => Err(ResponseError::Code { | ||
| (code, Some(errors)) => Err(Error::Api { | ||
| code, | ||
| headers, | ||
| errors, | ||
| }), | ||
| (code, None) => Err(Error::Code { | ||
| code, | ||
| headers, | ||
| body, | ||
|
|
@@ -457,6 +439,15 @@ impl Registry { | |
| } | ||
| } | ||
|
|
||
| fn status(code: u32) -> String { | ||
| if code == 200 { | ||
| String::new() | ||
| } else { | ||
| let reason = reason(code); | ||
| format!(" (status {code} {reason})") | ||
| } | ||
| } | ||
|
|
||
| fn reason(code: u32) -> &'static str { | ||
| // Taken from https://developer.mozilla.org/en-US/docs/Web/HTTP/Status | ||
| match code { | ||
|
|
@@ -520,7 +511,7 @@ pub fn is_url_crates_io(url: &str) -> bool { | |
| /// registries only create tokens in that format so that is as less restricted as possible. | ||
| pub fn check_token(token: &str) -> Result<()> { | ||
| if token.is_empty() { | ||
| bail!("please provide a non-empty token"); | ||
| return Err(Error::InvalidToken("please provide a non-empty token")); | ||
| } | ||
| if token.bytes().all(|b| { | ||
| // This is essentially the US-ASCII limitation of | ||
|
|
@@ -531,9 +522,9 @@ pub fn check_token(token: &str) -> Result<()> { | |
| }) { | ||
| Ok(()) | ||
| } else { | ||
| Err(anyhow::anyhow!( | ||
| Err(Error::InvalidToken( | ||
| "token contains invalid characters.\nOnly printable ISO-8859-1 characters \ | ||
| are allowed as it is sent in a HTTPS header." | ||
| are allowed as it is sent in a HTTPS header.", | ||
| )) | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters