Merge pull request #154 from NLnetLabs/0_4_1_pre

Merge release 0.4.1

Author: Tim Bruijnzeels

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name    = "krill"
-version = "0.5.0-pre"
+version = "0.4.1"
 authors = [ "The NLnet Labs RPKI team <[email protected]>" ]
 description = "Resource Public Key Infrastructure (RPKI) daemon"
 license = "MPL-2.0"
@@ -26,7 +26,7 @@ openssl         = { version = "^0.10", features = ["v110"] }
 pretty          = "0.5.2"
 rand            = "^0.5"
 reqwest         = "^0.9.17"
-rpki            = "0.8.1"
+rpki            = "0.8.2"
 serde           = { version = "^1.0", features = ["derive"] }
 serde_json      = "^1.0"
 syslog          = "^4.0"
@@ -44,4 +44,8 @@ ignore          = "^0.4"
 
 [features]
 default = []
-extra-debug = [ "rpki/extra-debug" ]
+extra-debug = [ "rpki/extra-debug" ]
+
+# Used when depending on development branches of rpki-rs or bcder
+#[patch.crates-io]
+#rpki = { git = "https://github.com/NLnetLabs/rpki-rs.git", branch = "resource-set-fix" }

Changelog.md

@@ -3,6 +3,15 @@
 Please see [here](https://github.com/NLnetLabs/krill/projects?query=is%3Aopen+sort%3Aname-asc)
 for planned releases. 
 
+## 0.4.1 'Fogo de Krill'
+
+This release fixes two issues:
+* Certain resource sets were handled incorrectly (#152)
+* Krill should not allow impossible max length values for ROAs (#153)
+
+We recommend that all users upgrade to this release. There were no configuration or data model
+changes introduced, so the binary can just be used to replace any installed 0.4.0 release.  
+
 ## 0.4.0 'The Krill Factor'
 
 This release focuses on stabilising the API and internal data format, which allows upgrades to 

doc/openapi.yaml

@@ -1,7 +1,7 @@
 openapi: "3.0.2"
 info:
   title: Krill RPKI Server API
-  version: 0.4.0
+  version: 0.4.1
   description: |
     # Introduction
     Welcome to the documentation for the Krill server API, a JSON based
@@ -942,6 +942,7 @@ paths:
                   - $ref: '#/components/schemas/InvalidROADeltaAddingDefinitionAlreadyPresent'
                   - $ref: '#/components/schemas/InvalidROADeltaRemovingUnknownDefinition'
                   - $ref: '#/components/schemas/InvalidROADeltaNotAllResourcesHeld'
+                  - $ref: '#/components/schemas/InvalidROADeltaInvalidMaxLength'
         '403':
           $ref: '#/components/responses/Forbidden'
         '404':
@@ -1495,6 +1496,18 @@ components:
         msg:
           type: string
           example: 'Invalid ROA delta: not all resources held.'
+    InvalidROADeltaInvalidMaxLength:
+      type: object
+      required:
+        - code
+        - msg
+      properties:
+        code:
+          type: integer
+          enum: [2404]
+        msg:
+          type: string
+          example: 'Invalid ROA definition: max length not legal for prefix.'
     CAHandleAlreadyInUse:
       type: object
       required:
@@ -1617,6 +1630,12 @@ components:
         application/json:
           schema:
             $ref: '#/components/schemas/InvalidROADeltaNotAllResourcesHeld'
+    InvalidROADeltaInvalidMaxLength:
+      description: 'Invalid ROA delta: not all resources held.'
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/InvalidROADeltaInvalidMaxLength'
     GetCA:
       description: Success.
       content:

src/commons/api/ca.rs

@@ -1949,4 +1949,20 @@ mod test {
 
         assert_eq!(ncc_id_pem.pem(), ncc_id_openssl_pem);
     }
+
+    #[test]
+    fn test_resource_set_intersection() {
+        let child_resources_json =
+            include_str!("../../../test-resources/resources/child_resources.json");
+        let child_resources: ResourceSet = serde_json::from_str(child_resources_json).unwrap();
+
+        let parent_resources_json =
+            include_str!("../../../test-resources/resources/parent_resources.json");
+        let parent_resouces: ResourceSet = serde_json::from_str(parent_resources_json).unwrap();
+
+        let intersection = parent_resouces.intersection(&child_resources);
+
+        assert_eq!(intersection, child_resources);
+    }
+
 }

src/commons/api/mod.rs

@@ -339,6 +339,9 @@ pub enum ErrorCode {
     #[display(fmt = "Invalid ROA delta: not all resources held.")]
     RoaUpdateInvalidResources,
 
+    #[display(fmt = "Invalid ROA definition: max length not legal for prefix")]
+    RoaUpdateInvalidMaxlength,
+
     // 2500s General CA issues
     #[display(fmt = "Unknown CA.")]
     UnknownCa,
@@ -410,6 +413,7 @@ impl From<usize> for ErrorCode {
             2401 => ErrorCode::RoaUpdateInvalidDuplicate,
             2402 => ErrorCode::RoaUpdateInvalidMissing,
             2403 => ErrorCode::RoaUpdateInvalidResources,
+            2404 => ErrorCode::RoaUpdateInvalidMaxlength,
 
             // 2500s -> General CA issues
             2501 => ErrorCode::DuplicateCa,
@@ -469,6 +473,7 @@ impl Into<ErrorResponse> for ErrorCode {
             ErrorCode::RoaUpdateInvalidDuplicate => 2401,
             ErrorCode::RoaUpdateInvalidMissing => 2402,
             ErrorCode::RoaUpdateInvalidResources => 2403,
+            ErrorCode::RoaUpdateInvalidMaxlength => 2404,
 
             // general krill ca errors
             ErrorCode::DuplicateCa => 2501,
@@ -524,7 +529,7 @@ mod tests {
             test_code(n)
         }
 
-        for n in 2401..2404 {
+        for n in 2401..2405 {
             test_code(n)
         }
 

src/commons/api/roas.rs

@@ -42,6 +42,17 @@ impl RoaDefinition {
     pub fn max_length(&self) -> Option<u8> {
         self.max_length
     }
+
+    pub fn max_length_valid(&self) -> bool {
+        if let Some(max_length) = self.max_length {
+            match self.prefix {
+                TypedPrefix::V4(_) => max_length >= self.prefix.addr_len() && max_length <= 32,
+                TypedPrefix::V6(_) => max_length >= self.prefix.addr_len() && max_length <= 128,
+            }
+        } else {
+            true
+        }
+    }
 }
 
 impl FromStr for RoaDefinition {
@@ -446,4 +457,31 @@ mod tests {
         parse_ser_de_print_definition("2001:db8::/32-48 => 64496");
     }
 
+    #[test]
+    fn roa_max_length() {
+        fn valid_max_length(s: &str) {
+            let def = RoaDefinition::from_str(s).unwrap();
+            assert!(def.max_length_valid())
+        }
+
+        fn invalid_max_length(s: &str) {
+            let def = RoaDefinition::from_str(s).unwrap();
+            assert!(!def.max_length_valid())
+        }
+
+        valid_max_length("192.168.0.0/16 => 64496");
+        valid_max_length("192.168.0.0/16-16 => 64496");
+        valid_max_length("192.168.0.0/16-24 => 64496");
+        valid_max_length("192.168.0.0/16-32 => 64496");
+        valid_max_length("2001:db8::/32 => 64496");
+        valid_max_length("2001:db8::/32-32 => 64496");
+        valid_max_length("2001:db8::/32-48 => 64496");
+        valid_max_length("2001:db8::/32-128 => 64496");
+
+        invalid_max_length("192.168.0.0/16-15 => 64496");
+        invalid_max_length("192.168.0.0/16-33 => 64496");
+        invalid_max_length("2001:db8::/32-31 => 64496");
+        invalid_max_length("2001:db8::/32-129 => 64496");
+    }
+
 }

src/constants.rs

@@ -1,4 +1,4 @@
-pub const KRILL_VERSION: &str = "0.4.0";
+pub const KRILL_VERSION: &str = "0.4.1";
 pub const KRILL_SERVER_APP: &str = "Krill";
 pub const KRILL_CLIENT_APP: &str = "Krill Client";
 

src/daemon/ca/certauth.rs

@@ -1364,6 +1364,12 @@ impl<S: Signer> CertAuth<S> {
             self.routes.authorizations().cloned().collect();
 
         for auth in added {
+            if !auth.max_length_valid() {
+                return Err(Error::AuthorisationInvalidMaxlength(
+                    auth,
+                    self.handle.clone(),
+                ));
+            }
             if current_auths.contains(&auth) {
                 return Err(Error::AuthorisationAlreadyPresent(
                     auth,

src/daemon/ca/error.rs

@@ -90,6 +90,13 @@ pub enum Error {
     #[display(fmt = "User tries  to re-add authorization '{}' for CA '{}'", _0, _1)]
     AuthorisationAlreadyPresent(RouteAuthorization, Handle),
 
+    #[display(
+        fmt = "Invalid max length for prefix in authorization: '{}' for CA '{}",
+        _0,
+        _1
+    )]
+    AuthorisationInvalidMaxlength(RouteAuthorization, Handle),
+
     #[display(
         fmt = "User tries to add authorization '{}' for resource not held by CA '{}'",
         _0,

src/daemon/endpoints.rs

@@ -783,6 +783,7 @@ impl ToErrorCode for ca::Error {
             ca::Error::AuthorisationAlreadyPresent(_, _) => ErrorCode::RoaUpdateInvalidDuplicate,
             ca::Error::AuthorisationUnknown(_, _) => ErrorCode::RoaUpdateInvalidMissing,
             ca::Error::AuthorisationNotEntitled(_, _) => ErrorCode::RoaUpdateInvalidResources,
+            ca::Error::AuthorisationInvalidMaxlength(_, _) => ErrorCode::RoaUpdateInvalidMaxlength,
             ca::Error::NewRepoUpdateNoChange => ErrorCode::NewRepoNoChange,
             ca::Error::NewRepoUpdateNotResponsive(_) => ErrorCode::NewRepoNoResponse,
             _ => ErrorCode::CaServerError,

test-resources/resources/child_resources.json

@@ -0,0 +1,5 @@
+{
+  "asn": "AS10906, AS11284, AS11644, AS11752, AS12136, AS14026, AS14650, AS22548, AS26162, AS53035, AS61580",
+  "v4": "45.6.52.0/22, 45.184.144.0/22, 45.227.0.0/22, 168.181.20.0/22, 187.16.192.0/19, 189.76.96.0/19, 200.160.0.0/20, 200.189.40.0/22, 200.192.104.0/24, 200.192.108.0/22, 200.192.232.0/22, 200.194.128.0/19, 200.219.130.0/23, 200.219.138.0-200.219.141.255, 200.219.143.0-200.219.148.255, 200.219.154.0-200.219.157.255, 200.219.158.0/23, 200.229.248.0/23",
+  "v6": "2001:12f8::/48, 2001:12f8:2::-2001:12f8:d:ffff:ffff:ffff:ffff:ffff, 2001:12fe::/31, 2801:80:1700::/40, 2801:80:1e00::/40"
+}

test-resources/resources/parent_resources.json

@@ -83,5 +83,10 @@ fn ca_roas() {
         ca_roll_activate(&child);
         wait_for_key_roll_complete(&child);
         wait_for_published_objects(&child, &[crl_file, mft_file, route3_file]);
+
+        let route_invalid_length = RoaDefinition::from_str("10.0.0.0/24-33 => 64496").unwrap();
+        let mut updates = RoaDefinitionUpdates::empty();
+        updates.add(route_invalid_length);
+        ca_route_authorizations_update_expect_error(&child, updates);
     });
 }