Skip to content

Commit 86f169a

Browse files
davidv1992rnijveld
davidv1992
authored and
rnijveld
committed
Enforce more constraints on hostname,port combo provided by client.
This provides new checks for uniqueness, and not being unwanted names like localhost, .local domains or the pool domain.
1 parent 90e4c4d commit 86f169a

File tree

4 files changed

+148
-54
lines changed

4 files changed

+148
-54
lines changed

‎nts-pool-management/migrations/20251217083055_timesource_unique.down.sql‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
-- Add down migration script here
2+
DROP INDEX time_sources_hostname_port_unique;
3+
ALTER TABLE time_sources ALTER COLUMN port DROP NOT NULL;
4+
ALTER TABLE time_sources ALTER COLUMN port DROP DEFAULT;

‎nts-pool-management/migrations/20251217083055_timesource_unique.up.sql‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
-- Add up migration script here
2+
UPDATE time_sources SET port=4460 WHERE port IS NULL;
3+
ALTER TABLE time_sources ALTER COLUMN port SET DEFAULT 4460;
4+
ALTER TABLE time_sources ALTER COLUMN port SET NOT NULL;
5+
CREATE UNIQUE INDEX time_sources_hostname_port_unique ON time_sources (hostname, port) WHERE NOT deleted;

‎nts-pool-management/src/models/time_source.rs‎

Lines changed: 134 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use sha3::Digest;
99

1010
use crate::{
1111
DbConnLike,
12+
context::AppContext,
1213
error::AppError,
1314
models::{
1415
monitor::MonitorId,
@@ -46,7 +47,7 @@ pub struct TimeSource {
4647
#[derive(Debug, Deserialize, Clone, PartialEq, Eq)]
4748
pub struct NewTimeSource {
4849
pub hostname: String,
49-
pub port: Option<Port>,
50+
pub port: Port,
5051
}
5152

5253
#[derive(Debug, Deserialize, Clone)]
@@ -60,39 +61,47 @@ pub struct UpdateTimeSourceForm {
6061
pub weight: i32,
6162
}
6263

63-
impl TryFrom<NewTimeSourceForm> for NewTimeSource {
64-
type Error = AppError;
64+
const DEFAULT_NTSKE_PORT: u16 = 4460;
6565

66-
fn try_from(form: NewTimeSourceForm) -> Result<Self, Self::Error> {
67-
let port = if form.port.is_empty() {
68-
None
66+
impl NewTimeSourceForm {
67+
pub fn into_new_source(self, context: &AppContext) -> Result<NewTimeSource, AppError> {
68+
let port = if self.port.is_empty() {
69+
DEFAULT_NTSKE_PORT
70+
.try_into()
71+
.wrap_err("Internal app error, 4460 not a valid port")?
6972
} else {
70-
Some(
71-
form.port
72-
.parse::<u16>()
73-
.wrap_err("Could not parse into port number")?
74-
.try_into()
75-
.wrap_err("Could not parse into port number")?,
76-
)
73+
self.port
74+
.parse::<u16>()
75+
.wrap_err("Could not parse into port number")?
76+
.try_into()
77+
.wrap_err("Could not parse into port number")?
7778
};
7879

79-
// Check domain name is reasonable (cf. RFC 1035)
80-
if form.hostname.is_empty()
81-
|| !form
82-
.hostname
83-
.chars()
84-
.all(|c| matches!(c, '0'..='9' | '.' | '-' | 'a'..='z' | 'A'..='Z'))
85-
{
86-
return Err(eyre::eyre!("Invalid domain name").into());
87-
}
80+
check_hostname_reasonable(&self.hostname, context)?;
8881

89-
Ok(Self {
90-
hostname: form.hostname,
82+
Ok(NewTimeSource {
83+
hostname: self.hostname,
9184
port,
9285
})
9386
}
9487
}
9588

89+
fn check_hostname_reasonable(hostname: &str, context: &AppContext) -> Result<(), AppError> {
90+
if hostname.is_empty()
91+
|| !hostname
92+
.chars()
93+
.all(|c| matches!(c, '0'..='9' | '.' | '-' | 'a'..='z' | 'A'..='Z'))
94+
|| hostname.ends_with(".local")
95+
|| hostname.ends_with(".")
96+
|| hostname.ends_with("localhost")
97+
|| hostname.ends_with(&context.ke_domain)
98+
{
99+
return Err(eyre::eyre!("Invalid domain name").into());
100+
}
101+
102+
Ok(())
103+
}
104+
96105
pub async fn infer_regions(
97106
hostname: impl AsRef<str>,
98107
geodb: &maxminddb::Reader<impl AsRef<[u8]>>,
@@ -466,131 +475,208 @@ mod tests {
466475
#[test]
467476
fn test_time_source_form_valid_without_port_1() {
468477
assert_eq!(
469-
NewTimeSource::try_from(NewTimeSourceForm {
478+
NewTimeSourceForm {
470479
hostname: "test".into(),
471480
port: "".into()
472-
})
481+
}
482+
.into_new_source(&AppContext::default())
473483
.unwrap(),
474484
NewTimeSource {
475485
hostname: "test".into(),
476-
port: None
486+
port: 4460.try_into().unwrap()
477487
}
478488
);
479489
}
480490

481491
#[test]
482492
fn test_time_source_form_valid_without_port_2() {
483493
assert_eq!(
484-
NewTimeSource::try_from(NewTimeSourceForm {
494+
NewTimeSourceForm {
485495
hostname: "ExAmPlE.com".into(),
486496
port: "".into()
487-
})
497+
}
498+
.into_new_source(&AppContext::default())
488499
.unwrap(),
489500
NewTimeSource {
490501
hostname: "ExAmPlE.com".into(),
491-
port: None
502+
port: 4460.try_into().unwrap(),
492503
}
493504
);
494505
}
495506

496507
#[test]
497508
fn test_time_source_form_valid_with_port() {
498509
assert_eq!(
499-
NewTimeSource::try_from(NewTimeSourceForm {
510+
NewTimeSourceForm {
500511
hostname: "test".into(),
501512
port: "456".into()
502-
})
513+
}
514+
.into_new_source(&AppContext::default())
503515
.unwrap(),
504516
NewTimeSource {
505517
hostname: "test".into(),
506-
port: Some(456.try_into().unwrap())
518+
port: 456.try_into().unwrap()
507519
}
508520
);
509521
}
510522

511523
#[test]
512524
fn test_time_source_form_reject_weird_characters_1() {
513525
assert!(
514-
NewTimeSource::try_from(NewTimeSourceForm {
526+
NewTimeSourceForm {
515527
hostname: "js(.com".into(),
516528
port: "".into(),
517-
})
529+
}
530+
.into_new_source(&AppContext::default())
518531
.is_err()
519532
);
520533
}
521534

522535
#[test]
523536
fn test_time_source_form_reject_weird_characters_2() {
524537
assert!(
525-
NewTimeSource::try_from(NewTimeSourceForm {
538+
NewTimeSourceForm {
526539
hostname: "jsê.com".into(),
527540
port: "".into(),
528-
})
541+
}
542+
.into_new_source(&AppContext::default())
529543
.is_err()
530544
);
531545
}
532546

533547
#[test]
534548
fn test_time_source_form_reject_weird_characters_3() {
535549
assert!(
536-
NewTimeSource::try_from(NewTimeSourceForm {
550+
NewTimeSourceForm {
537551
hostname: "[email protected]".into(),
538552
port: "".into(),
539-
})
553+
}
554+
.into_new_source(&AppContext::default())
540555
.is_err()
541556
);
542557
}
543558

544559
#[test]
545560
fn test_time_source_form_reject_port_0() {
546561
assert!(
547-
NewTimeSource::try_from(NewTimeSourceForm {
562+
NewTimeSourceForm {
548563
hostname: "example.com".into(),
549564
port: "0".into(),
550-
})
565+
}
566+
.into_new_source(&AppContext::default())
551567
.is_err()
552568
);
553569
}
554570

555571
#[test]
556572
fn test_time_source_form_reject_port_100000() {
557573
assert!(
558-
NewTimeSource::try_from(NewTimeSourceForm {
574+
NewTimeSourceForm {
559575
hostname: "example.com".into(),
560576
port: "100000".into(),
561-
})
577+
}
578+
.into_new_source(&AppContext::default())
562579
.is_err()
563580
);
564581
}
565582

566583
#[test]
567584
fn test_time_source_form_reject_non_numeric_port() {
568585
assert!(
569-
NewTimeSource::try_from(NewTimeSourceForm {
586+
NewTimeSourceForm {
570587
hostname: "example.com".into(),
571588
port: "fe".into(),
572-
})
589+
}
590+
.into_new_source(&AppContext::default())
573591
.is_err()
574592
);
575593
}
576594

577595
#[test]
578596
fn test_time_source_form_reject_port_with_trailing_garbage() {
579597
assert!(
580-
NewTimeSource::try_from(NewTimeSourceForm {
598+
NewTimeSourceForm {
581599
hostname: "example.com".into(),
582600
port: "123 abc".into(),
583-
})
601+
}
602+
.into_new_source(&AppContext::default())
584603
.is_err()
585604
);
586605
}
587606

588607
#[test]
589608
fn test_time_source_form_require_hostname() {
590609
assert!(
591-
NewTimeSource::try_from(NewTimeSourceForm {
610+
NewTimeSourceForm {
592611
hostname: "".into(),
593612
port: "".into(),
613+
}
614+
.into_new_source(&AppContext::default())
615+
.is_err()
616+
);
617+
}
618+
619+
#[test]
620+
fn test_time_source_form_reject_local() {
621+
assert!(
622+
NewTimeSourceForm {
623+
hostname: "example.local".into(),
624+
port: "123".into(),
625+
}
626+
.into_new_source(&AppContext::default())
627+
.is_err()
628+
);
629+
630+
assert!(
631+
NewTimeSourceForm {
632+
hostname: "localhost".into(),
633+
port: "123".into(),
634+
}
635+
.into_new_source(&AppContext::default())
636+
.is_err()
637+
);
638+
639+
assert!(
640+
NewTimeSourceForm {
641+
hostname: "example.local.".into(),
642+
port: "123".into(),
643+
}
644+
.into_new_source(&AppContext::default())
645+
.is_err()
646+
);
647+
648+
assert!(
649+
NewTimeSourceForm {
650+
hostname: "example.localhost".into(),
651+
port: "123".into(),
652+
}
653+
.into_new_source(&AppContext::default())
654+
.is_err()
655+
);
656+
}
657+
658+
#[test]
659+
fn test_time_source_form_reject_ke_domain() {
660+
assert!(
661+
NewTimeSourceForm {
662+
hostname: "ke.example.org".into(),
663+
port: "123".into(),
664+
}
665+
.into_new_source(&AppContext {
666+
ke_domain: "ke.example.org".into(),
667+
..AppContext::default()
668+
})
669+
.is_err()
670+
);
671+
672+
assert!(
673+
NewTimeSourceForm {
674+
hostname: "bla.ke.example.org".into(),
675+
port: "123".into(),
676+
}
677+
.into_new_source(&AppContext {
678+
ke_domain: "ke.example.org".into(),
679+
..AppContext::default()
594680
})
595681
.is_err()
596682
);

‎nts-pool-management/src/routes/management.rs‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ pub async fn create_time_source(
208208
match time_source::create(
209209
&state.db,
210210
user.id,
211-
new_time_source.clone().try_into()?,
211+
new_time_source.clone().into_new_source(&app)?,
212212
state.config.base_secret_index,
213213
&geodb,
214214
)
@@ -224,11 +224,10 @@ pub async fn create_time_source(
224224
),
225225
})
226226
.into_response()),
227-
Err(_) => Ok((
228-
cookies.flash_error("Could not add time source".to_string()),
229-
Redirect::to(TIME_SOURCES_ENDPOINT),
230-
)
231-
.into_response()),
227+
Err(_) => {
228+
cookies.flash_error("Could not add duplicate time source".to_string());
229+
Ok((cookies, Redirect::to(TIME_SOURCES_ENDPOINT)).into_response())
230+
}
232231
}
233232
}
234233

0 commit comments

Comments
 (0)