-
Notifications
You must be signed in to change notification settings - Fork 0
Misc changes to defaults #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
dongresource
wants to merge
2
commits into
main
Choose a base branch
from
defaults
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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 |
---|---|---|
|
@@ -2,3 +2,4 @@ | |
*.db | ||
*.pem | ||
secret | ||
supply-chain/ |
This file contains hidden or 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 hidden or 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,21 +1,22 @@ | ||
[core] | ||
db_path = "../OpenFusion/database.db" | ||
port = 80 | ||
|
||
[tls] | ||
cert_path = "cert.pem" # app-level TLS only | ||
key_path = "key.pem" # app-level TLS only | ||
port = 443 | ||
|
||
[rankinfo] | ||
route = "/getranks" | ||
placeholders = true | ||
|
||
[auth] | ||
route = "/auth" | ||
secret_path = "secret" | ||
valid_secs = 604_800 # one week | ||
|
||
[cookie] | ||
route = "/cookie" | ||
valid_secs = 60 | ||
[core] | ||
db_path = "../OpenFusion/database.db" | ||
port = 8080 | ||
|
||
# App-terminated TLS, as opposed to proxy-terminated TLS. | ||
[app_tls] | ||
cert_path = "cert.pem" | ||
key_path = "key.pem" | ||
port = 443 | ||
|
||
[rankinfo] | ||
route = "/getranks" | ||
placeholders = false | ||
|
||
[auth] | ||
route = "/auth" | ||
secret_path = "secret" | ||
valid_secs = 604_800 # one week | ||
|
||
[cookie] | ||
route = "/cookie" | ||
valid_secs = 60 |
This file contains hidden or 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 hidden or 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 hidden or 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 |
---|---|---|
|
@@ -8,7 +8,7 @@ use simplelog::{ColorChoice, LevelFilter, TermLogger, TerminalMode}; | |
use sqlite::Connection; | ||
use tokio::sync::Mutex; | ||
|
||
#[cfg(feature = "tls")] | ||
#[cfg(feature = "app-tls")] | ||
use {axum_server::tls_rustls::RustlsConfig, rustls::crypto::ring}; | ||
|
||
mod auth; | ||
|
@@ -17,6 +17,12 @@ mod rankinfo; | |
mod statics; | ||
mod util; | ||
|
||
const DEFAULT_HTTP_PORT: u16 = 8080; | ||
const CONFIG_PATH: &str = "config.toml"; | ||
|
||
#[cfg(feature = "app-tls")] | ||
const DEFAULT_HTTPS_PORT: u16 = 443; | ||
|
||
#[derive(Deserialize, Clone)] | ||
struct CoreConfig { | ||
db_path: String, | ||
|
@@ -34,14 +40,13 @@ struct TlsConfig { | |
#[derive(Deserialize, Clone)] | ||
struct Config { | ||
core: CoreConfig, | ||
tls: Option<TlsConfig>, | ||
app_tls: Option<TlsConfig>, | ||
rankinfo: Option<rankinfo::RankInfoConfig>, | ||
auth: Option<auth::AuthConfig>, | ||
cookie: Option<cookie::CookieConfig>, | ||
} | ||
impl Config { | ||
fn load() -> Self { | ||
const CONFIG_PATH: &str = "config.toml"; | ||
let config = std::fs::read_to_string(CONFIG_PATH).expect("Failed to open config file"); | ||
toml::from_str(&config).expect("Failed to parse config file") | ||
} | ||
|
@@ -51,7 +56,6 @@ impl Config { | |
struct AppState { | ||
db: Arc<Mutex<Connection>>, | ||
rng: Arc<SystemRandom>, | ||
is_tls: bool, | ||
config: Config, | ||
} | ||
impl AppState { | ||
|
@@ -64,10 +68,19 @@ impl AppState { | |
Self { | ||
db: Arc::new(Mutex::new(conn)), | ||
rng: Arc::new(SystemRandom::new()), | ||
is_tls: false, | ||
config: config.clone(), | ||
} | ||
} | ||
|
||
fn is_using_app_tls(self: &Self) -> bool { | ||
#[cfg(not(feature = "app-tls"))] | ||
{ | ||
return false; | ||
} | ||
|
||
#[allow(unreachable_code)] | ||
self.config.app_tls.is_some() | ||
} | ||
} | ||
|
||
#[tokio::main] | ||
|
@@ -96,26 +109,49 @@ async fn main() { | |
routes = rankinfo::register(routes, rankinfo_config); | ||
} | ||
|
||
// register HTTPS-only endpoints | ||
let mut routes_tls = Router::new().merge(routes.clone()); | ||
// register secure endpoints | ||
let mut routes_secure = Router::new(); | ||
if let Some(ref auth_config) = config.auth { | ||
routes_tls = auth::register(routes_tls, auth_config, &state.rng); | ||
routes_secure = auth::register(routes_secure, auth_config, &state.rng); | ||
} | ||
if let Some(ref cookie_config) = config.cookie { | ||
routes_tls = cookie::register(routes_tls, cookie_config); | ||
routes_secure = cookie::register(routes_secure, cookie_config); | ||
} | ||
|
||
// init both protocols | ||
// N.B. these listen concurrently, but NOT in parallel (see tokio::join!) | ||
let http = init_http(routes, &config, state.clone()); | ||
let https = init_https(routes_tls, &config, state); | ||
let _ = tokio::join!(http, https); | ||
#[cfg(not(feature = "app-tls"))] | ||
{ | ||
warn!("OFAPI was not compiled with the `app-tls` feature. Encryption should be done at the proxy level!"); | ||
|
||
let routes = Router::new().merge(routes).merge(routes_secure); | ||
let http = init_http(routes, &config, state.clone()); | ||
let _ = tokio::join!(http); | ||
} | ||
|
||
#[cfg(feature = "app-tls")] | ||
yungcomputerchair marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
info!("Using application-level TLS termination."); | ||
|
||
// init secure endpoints on a separate port | ||
// N.B. these listen concurrently, but NOT in parallel (see tokio::join!) | ||
|
||
if state.is_using_app_tls() { | ||
let routes_secure = Router::new().merge(routes.clone()).merge(routes_secure); | ||
let http = init_http(routes, &config, state.clone()); | ||
let https = init_https(routes_secure, &config, state); | ||
let _ = tokio::join!(http, https); | ||
} else { | ||
warn!("Application-level TLS termination disabled. Encryption should be done at the proxy level!"); | ||
|
||
let routes = Router::new().merge(routes).merge(routes_secure); | ||
let http = init_http(routes, &config, state.clone()); | ||
let _ = tokio::join!(http); | ||
} | ||
} | ||
} | ||
|
||
const BIND_IP: [u8; 4] = [127, 0, 0, 1]; | ||
|
||
async fn init_http(routes: Router<Arc<AppState>>, config: &Config, state: AppState) { | ||
const DEFAULT_HTTP_PORT: u16 = 80; | ||
let addr = SocketAddr::from((BIND_IP, config.core.port.unwrap_or(DEFAULT_HTTP_PORT))); | ||
|
||
let app = routes.with_state(Arc::new(state)); | ||
|
@@ -125,55 +161,39 @@ async fn init_http(routes: Router<Arc<AppState>>, config: &Config, state: AppSta | |
axum::serve(listener, app).await.unwrap(); | ||
} | ||
|
||
async fn init_https(routes: Router<Arc<AppState>>, config: &Config, mut state: AppState) { | ||
const DEFAULT_HTTPS_PORT: u16 = 443; | ||
|
||
state.is_tls = true; | ||
#[cfg(feature = "app-tls")] | ||
async fn init_https(routes: Router<Arc<AppState>>, config: &Config, state: AppState) { | ||
let app = routes.with_state(Arc::new(state)); | ||
|
||
let Some(ref tls_config) = config.tls else { | ||
let Some(ref tls_config) = config.app_tls else { | ||
warn!("Missing or malformed TLS config. HTTPS disabled"); | ||
return; | ||
}; | ||
|
||
let addr = SocketAddr::from((BIND_IP, tls_config.port.unwrap_or(DEFAULT_HTTPS_PORT))); | ||
|
||
#[cfg(not(feature = "tls"))] | ||
{ | ||
warn!("TLS APIs enabled but OFAPI was not compiled with the `tls` feature. Encryption should be done at the proxy level!"); | ||
} | ||
|
||
info!("HTTPS listening on {}", addr); | ||
|
||
#[cfg(feature = "tls")] | ||
{ | ||
ring::default_provider().install_default().unwrap(); | ||
let rustls_cfg = | ||
match RustlsConfig::from_pem_file(&tls_config.cert_path, &tls_config.key_path).await { | ||
Err(e) => { | ||
warn!("Failed to activate TLS ({}); HTTPS disabled", e); | ||
return; | ||
} | ||
Ok(cfg) => cfg, | ||
}; | ||
|
||
axum_server::bind_rustls(addr, rustls_cfg) | ||
.serve(app.into_make_service()) | ||
.await | ||
.unwrap(); | ||
} | ||
|
||
#[cfg(not(feature = "tls"))] | ||
{ | ||
let listener = tokio::net::TcpListener::bind(addr).await.unwrap(); | ||
axum::serve(listener, app).await.unwrap(); | ||
} | ||
ring::default_provider().install_default().unwrap(); | ||
let rustls_cfg = | ||
match RustlsConfig::from_pem_file(&tls_config.cert_path, &tls_config.key_path).await { | ||
Err(e) => { | ||
warn!("Failed to activate TLS ({}); HTTPS disabled", e); | ||
return; | ||
} | ||
Ok(cfg) => cfg, | ||
}; | ||
|
||
axum_server::bind_rustls(addr, rustls_cfg) | ||
.serve(app.into_make_service()) | ||
.await | ||
.unwrap(); | ||
} | ||
|
||
async fn get_info(State(state): State<Arc<AppState>>) -> String { | ||
format!( | ||
"OFAPI v{}\ntls: {:?}", | ||
"OFAPI v{}\ntls: {:?}\n", | ||
env!("CARGO_PKG_VERSION"), | ||
state.is_tls | ||
state.is_using_app_tls() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See previous comment; this is supposed to indicate whether secure APIs are enabled, not whether app-terminated TLS is enabled |
||
) | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would actually keep this (and the assertions) but maybe rename to something like
is_secure
to align with our discussion