Skip to content

Commit

Permalink
refactor: remove stale self signed certificate interval
Browse files Browse the repository at this point in the history
  • Loading branch information
vicanso committed Dec 17, 2024
1 parent 9ddccb1 commit a75af2a
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/config/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ pub struct CertificateConf {
pub tls_key: Option<String>,
pub tls_chain: Option<String>,
pub is_default: Option<bool>,
pub is_root: Option<bool>,
pub is_ca: Option<bool>,
pub acme: Option<String>,
pub remark: Option<String>,
}
Expand Down
9 changes: 8 additions & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ use otel::TracerService;
use pingora::server;
use pingora::server::configuration::Opt;
use pingora::services::background::background_service;
use proxy::{new_upstream_health_check_task, Server, ServerConf};
use proxy::{
new_self_signed_cert_validity_service, new_upstream_health_check_task,
Server, ServerConf,
};
use state::{get_admin_addr, get_start_time, set_admin_addr};
use std::collections::HashMap;
use std::error::Error;
Expand Down Expand Up @@ -555,6 +558,10 @@ fn run() -> Result<(), Box<dyn Error>> {
"TlsValidity",
new_tls_validity_service(),
));
my_server.add_service(background_service(
"SelfSignedStale",
new_self_signed_cert_validity_service(),
));
my_server.add_service(background_service(
"UpstreamHc",
new_upstream_health_check_task(Duration::from_secs(10)),
Expand Down
92 changes: 78 additions & 14 deletions src/proxy/dynamic_certificate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

use crate::acme::Certificate;
use crate::config::CertificateConf;
use crate::service::{CommonServiceTask, ServiceTask};
use crate::{util, webhook};
use ahash::AHashMap;
use arc_swap::ArcSwap;
Expand All @@ -26,7 +27,9 @@ use pingora::tls::ssl::{NameType, SslRef};
use pingora::tls::x509::X509;
use snafu::Snafu;
use std::collections::HashMap;
use std::sync::atomic::{AtomicBool, AtomicU32, Ordering};
use std::sync::Arc;
use std::time::Duration;
use substring::Substring;
use tracing::{debug, error, info};

Expand All @@ -43,10 +46,52 @@ type DynamicCertificates = AHashMap<String, Arc<DynamicCertificate>>;
static DYNAMIC_CERTIFICATE_MAP: Lazy<ArcSwap<DynamicCertificates>> =
Lazy::new(|| ArcSwap::from_pointee(AHashMap::new()));

type SelfSignedCertKey = AHashMap<String, Arc<(X509, PKey<Private>)>>;
struct SelfSignedCert {
x509: X509,
key: PKey<Private>,
stale: AtomicBool,
count: AtomicU32,
}
type SelfSignedCertKey = AHashMap<String, Arc<SelfSignedCert>>;
static SELF_SIGNED_CERT_KEY_MAP: Lazy<ArcSwap<SelfSignedCertKey>> =
Lazy::new(|| ArcSwap::from_pointee(AHashMap::new()));

struct SelfSiginedStaleCertChecker {}

#[async_trait]
impl ServiceTask for SelfSiginedStaleCertChecker {
async fn run(&self) -> Option<bool> {
let mut m = AHashMap::new();
for (k, v) in SELF_SIGNED_CERT_KEY_MAP.load().iter() {
let count = v.count.load(Ordering::Relaxed);
let stale = v.stale.load(Ordering::Relaxed);
if stale && count == 0 {
continue;
}
if count == 0 {
v.stale.store(true, Ordering::Relaxed);
} else {
v.stale.store(false, Ordering::Relaxed);
v.count.store(0, Ordering::Relaxed);
}
m.insert(k.to_string(), v.clone());
}
SELF_SIGNED_CERT_KEY_MAP.store(Arc::new(m));
Some(false)
}
fn description(&self) -> String {
"Self signed certificate stale checker".to_string()
}
}

pub fn new_self_signed_cert_validity_service() -> CommonServiceTask {
CommonServiceTask::new(
// check interval: one day
Duration::from_secs(24 * 60 * 60),
SelfSiginedStaleCertChecker {},
)
}

// https://letsencrypt.org/certificates/
const E5: &[u8] = include_bytes!("../assets/e5.pem");
const E6: &[u8] = include_bytes!("../assets/e6.pem");
Expand Down Expand Up @@ -130,7 +175,7 @@ fn parse_certificate(
domains: info.domains.clone(),
certificate: Some((cert, key)),
info: Some(info),
is_root: certificate_config.is_root.unwrap_or_default(),
is_ca: certificate_config.is_ca.unwrap_or_default(),
..Default::default()
})
}
Expand Down Expand Up @@ -224,7 +269,7 @@ pub struct DynamicCertificate {
domains: Vec<String>,
info: Option<Certificate>,
hash_key: String,
is_root: bool,
is_ca: bool,
}

pub struct TlsSettingParams {
Expand Down Expand Up @@ -263,6 +308,8 @@ fn new_certificate_with_ca(
message: e.to_string(),
category: "parse_ca_key".to_string(),
})?;
let not_before = ca_params.not_before;

This comment has been minimized.

Copy link
@atinm

atinm Dec 18, 2024

@vicanso not_before should be now_utc(). not_after should not be the CA either - use a default of 1 or 2 years (CA expiry can be quite long, and it might be best to allow user to set the default time for expiry). Since these are really MITM certificates, they shouldn't have a long expiry.

This comment has been minimized.

Copy link
@vicanso

vicanso Dec 19, 2024

Author Owner

I will modify it later.

let not_after = ca_params.not_after;
let ca_cert =
ca_params.self_signed(&ca_kp).map_err(|e| Error::Invalid {
message: e.to_string(),
Expand All @@ -276,13 +323,24 @@ fn new_certificate_with_ca(
})?;
let mut dn = rcgen::DistinguishedName::new();
dn.push(rcgen::DnType::CommonName, cn.to_string());
dn.push(rcgen::DnType::OrganizationName, "Pingap".to_string());
dn.push(rcgen::DnType::OrganizationalUnitName, "Pingap".to_string());
if let Some(organ) = ca_cert
.params()
.distinguished_name
.get(&rcgen::DnType::OrganizationName)
{
dn.push(rcgen::DnType::OrganizationName, organ.clone());
};
if let Some(unit) = ca_cert
.params()
.distinguished_name
.get(&rcgen::DnType::OrganizationalUnitName)
{
dn.push(rcgen::DnType::OrganizationalUnitName, unit.clone());
};

params.distinguished_name = dn;
params.not_before = time::OffsetDateTime::now_utc();
params.not_after =
time::OffsetDateTime::now_utc() + time::Duration::days(365 * 20);
params.not_before = not_before;

This comment has been minimized.

Copy link
@atinm

atinm Dec 18, 2024

@vicanso This should just be now_utc() like before, you should not have a not_before that is before the certificate was generated.

params.not_after = not_after;

This comment has been minimized.

Copy link
@atinm

atinm Dec 18, 2024

@vicanso you should still add

    params.subject_alt_names = vec![
        SanType::DnsName(Ia5String::try_from(cn.to_string()).unwrap()),
    ];

let cert_key = rcgen::KeyPair::generate().map_err(|e| Error::Invalid {
message: e.to_string(),
Expand Down Expand Up @@ -384,7 +442,7 @@ impl DynamicCertificate {
fn get_self_signed_cert(
&self,
server_name: &str,
) -> Result<Arc<(X509, PKey<Private>)>> {
) -> Result<Arc<SelfSignedCert>> {
let arr: Vec<&str> = server_name.split('.').collect();
let cn = if arr.len() > 2 {
format!("*.{}", arr[1..].join("."))
Expand All @@ -401,7 +459,12 @@ impl DynamicCertificate {
for (k, v) in SELF_SIGNED_CERT_KEY_MAP.load().iter() {
m.insert(k.to_string(), v.clone());
}
let v = Arc::new((cert, key));
let v = Arc::new(SelfSignedCert {
x509: cert,
key,
stale: AtomicBool::new(false),
count: AtomicU32::new(0),
});
m.insert(k, v.clone());
SELF_SIGNED_CERT_KEY_MAP.store(Arc::new(m));

Expand Down Expand Up @@ -480,16 +543,17 @@ impl pingora::listeners::TlsAccept for DynamicCertificate {
return;
};

// root ca
if d.is_root {
// ca
if d.is_ca {
match d.get_self_signed_cert(server_name.unwrap_or_default()) {
Ok(result) => {
ssl_certificate(
ssl,
&result.0,
&result.1,
&result.x509,
&result.key,
&d.chain_certificate,
);
result.count.fetch_add(1, Ordering::Relaxed);
},
Err(err) => {
error!(
Expand Down
5 changes: 4 additions & 1 deletion src/proxy/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ mod upstream;
#[allow(unused_imports)]
pub use location::Location;

pub use dynamic_certificate::{get_certificate_info_list, init_certificates};
pub use dynamic_certificate::{
get_certificate_info_list, init_certificates,
new_self_signed_cert_validity_service,
};
pub use location::try_init_locations;
pub use logger::Parser;
pub use server::*;
Expand Down
2 changes: 1 addition & 1 deletion web/src/i18n/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ export default {
"Input the domains of acme, multiple domains separated by comma",
acme: "Acme",
isDefault: "Default Certificate",
isRoot: "Root Certificate Authority",
isCa: "Certificate Authority",
},
plugin: {
name: "Name",
Expand Down
2 changes: 1 addition & 1 deletion web/src/i18n/zh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ export default {
domainsPlaceholder: "输入使用acme的域名列表,多个域名以`,`分隔",
acme: "acme",
isDefault: "是否默认证书",
isRoot: "是否根证书",
isCa: "CA证书",
},
plugin: {
name: "名称",
Expand Down
6 changes: 3 additions & 3 deletions web/src/pages/Certificates.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,10 @@ export default function Certificates() {
options: newBooleanOptions(),
},
{
name: "is_root",
label: certificateI18n("isRoot"),
name: "is_ca",
label: certificateI18n("isCa"),
placeholder: "",
defaultValue: certificateConfig.is_root,
defaultValue: certificateConfig.is_ca,
span: 3,
category: ExFormItemCategory.RADIOS,
options: newBooleanOptions(),
Expand Down
2 changes: 1 addition & 1 deletion web/src/states/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export interface Certificate {
certificate_file?: string;
acme?: string;
is_default?: boolean;
is_root?: boolean;
is_ca?: boolean;
remark?: string;
}

Expand Down

0 comments on commit a75af2a

Please sign in to comment.