Skip to content
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

Extension IPAddrBlocks #187

Open
xipki opened this issue Apr 27, 2024 · 7 comments
Open

Extension IPAddrBlocks #187

xipki opened this issue Apr 27, 2024 · 7 comments

Comments

@xipki
Copy link
Contributor

xipki commented Apr 27, 2024

  • Issue 1:

In RFC 3779 (https://datatracker.ietf.org/doc/html/rfc3779#section-2.2.3), the IPAddressBlocksis defined as follows

IPAddrBlocks        ::= SEQUENCE OF IPAddressFamily

   IPAddressFamily     ::= SEQUENCE {    -- AFI & optional SAFI --
      addressFamily        OCTET STRING (SIZE (2..3)),
      ipAddressChoice      IPAddressChoice }

   IPAddressChoice     ::= CHOICE {
      inherit              NULL, -- inherit from issuer --
      addressesOrRanges    SEQUENCE OF IPAddressOrRange }

   IPAddressOrRange    ::= CHOICE {
      addressPrefix        IPAddress,
      addressRange         IPAddressRange }

   IPAddressRange      ::= SEQUENCE {
      min                  IPAddress,
      max                  IPAddress }

   IPAddress           ::= BIT STRING

And the syntax in C509 is as follows

      Address = bytes / uint,
      AddressPrefix = (Address, unusedBits: uint)
      AddressRange =  [Address, Address]
      IPAddressOrRange = AddressPrefix / AddressRange
      IPAddressChoice = [ + IPAddressOrRange ] / null
      IPAddrBlocks = [ AFI: uint, IPAddressChoice ]

According to the syntax above, only IPAddrBlocks with one (ASN.1) IPAddressFamily can be encoded in C509. The syntax may be changed as follows (I added blank lines between the definitions) to allow also multiple IPAddressFamily elements.

      Address = bytes / uint,

      AddressPrefix = (Address, unusedBits: uint)

      AddressRange =  [**min:**Address, **max:**Address]

      IPAddressOrRange = AddressPrefix / AddressRange

      IPAddressChoice = [ + IPAddressOrRange ] / null

      ***IPAddressFamily = (AFI: uint, IPAddressChoice)***

      ***IPAddrBlocks = [ + IPAddressFamily ]***
  • Issue 2:

In Page 15 (of -09),

 * IP Resources (id-pe-ipAddrBlocks).  If rdi and SAFI is not
      present, the extension value can be CBOR encoded.
  1. rdi does not apply here.
  2. What is the purpose not to allow SAFI? The syntax above can be simply adapated to cover addressFamily with SAFI:
  IPAddressFamily = (AFI: uint, ? SAFI:uint, IPAddressChoice)
  • Issue 3:
      ...
      Each
      AddressPrefix is encoded as a CBOR bytes string (without the
      unused bits octet) followed by the number of unused bits encoded
      as a CBOR uint. Each AddressRange is encoded as an array of two
      CBOR byte strings.  The unused bits for min and max are omitted,
      but the unused bits in max IPAddress is set to ones.  With the
      exception of the first Address, if the byte string has the same
      length as the previous Address, the Address is encoded as an uint
      with the the difference to the previous Address.
      ...

This text block needs to be re-written. It is difficult to understand.

  • The condition to encode an IP address as a uint shall be changed from "if the byte string has the same length as the previous Address" to "if the byte string has the same length as the previous Address and the difference is less than 2^64". For IPv6 address this is relevant.

  • The names Address, AddressPrefix, AddressRange shall be prepended with the prefix IP.

@highlunder
Copy link
Collaborator

Current conclusion:
*We will look into making the text and the descriptions more clear BUT
*We're not adding the capabilities to handle sequences until we are presented with a use case where it is used in practice.

@highlunder
Copy link
Collaborator

+Ask Russ! He provided the initial example.

@xipki
Copy link
Contributor Author

xipki commented May 17, 2024

I think we may need to support multiple IPAddressFamily, since one may have one IPAddressFamily for the IPv4 addresses, and one for the IPv6 addresses.

@xipki
Copy link
Contributor Author

xipki commented May 18, 2024

Below is the certificate generated in openssl

-----BEGIN CERTIFICATE-----
MIICYzCCAcygAwIBAgIUAlrOCqB4EiHFS2ifQNWsVjH56TcwDQYJKoZIhvcNAQEL
BQAwIzEhMB8GA1UEAwwYSVAgYWRkcmVzcyBibG9jayBleGFtcGxlMB4XDTI0MDUx
ODA5MzQ0M1oXDTI3MDIxMzA5MzQ0M1owIzEhMB8GA1UEAwwYSVAgYWRkcmVzcyBi
bG9jayBleGFtcGxlMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC1hO0ha0VF
+lfAeJfWTp0KEt+M0tdV1Qg3M+bybbemSRO2LWH84iiuDuD3XK8HcgaDduMAQlqd
Q/b69+eW/5+q+FCinBKFWQ6Ukzj0q1moDsTEExZd17fJcDMLR6Z1n6RADop6zpGK
RPXErCqfKkRysQ0auJhMYmxsmJkSBJbmjwIDAQABo4GTMIGQMAkGA1UdEwQCMAAw
CwYDVR0PBAQDAgWgMFcGCCsGAQUFBwEHAQH/BEgwRjAfBAIAATAZAwUACgAAAQME
AAoAATAKAwMBCgIDAwAKBDAjBAIAAjAdAwkAIAIAAQAAAAAwEAMFASACAAIDBwAg
AgAIAAAwHQYDVR0OBBYEFHqETQrG0imaqSCBib0IySUzTG27MA0GCSqGSIb3DQEB
CwUAA4GBAKKaucLwYiAwO8AD3y25PTcY4mszDE6f+5NVZXmyzSo2eYHC+ABAOMU5
B9ZRDojOtMi3i3zPVQkCkROQwa80n9hDcSimBglckvMHaIYdXOudAXkYIlnAkgug
y7+hE46V4oMHK7xHM0IxwbUYYvEGK2J02hrn8G20QFO4kP/dNW4Y
-----END CERTIFICATE-----

with the command

openssl x509 -in etcd1.csr -out etcd1.pem -req -signkey etcd1-key.pem -days 1001  -extensions v3_req -extfile openssl.conf

Where openssl.conf has content

[v3_req]
basicConstraints = CA:FALSE
keyUsage = digitalSignature, keyEncipherment
sbgp-ipAddrBlock = critical, @addr-section

[addr-section]

IPv4.0 = 10.0.0.1
IPv4.1 = 10.0.1.0/24
IPv4.2 = 10.2.0.0 - 10.4.255.255
IPv6.0 = 2002:1::/64
IPv6.1 = 2002:2:: - 2002:8::ffff:ffff:ffff:ffff:fff

The openssl text ouput is

        X509v3 extensions:
            X509v3 Basic Constraints: 
                CA:FALSE
            X509v3 Key Usage: 
                Digital Signature, Key Encipherment
            sbgp-ipAddrBlock: critical
                IPv4:
                  10.0.0.1/32
                  10.0.1.0/24
                  10.2.0.0-10.4.255.255
                IPv6:
                  2002:1::/64
                  2002:2::-2002:8:0:ffff:ffff:ffff:ffff:ffff

As shown above, we have to IPAddressFamily elements (one for IPv4, and one for IPv6) --> We need to support multipe IPAddressFamily elements.

Look into details with `openssl asn1parse -i -dump -in certfile.pem -strparse 365:

    0:d=0  hl=2 l=  70 cons: SEQUENCE          
    2:d=1  hl=2 l=  31 cons:  SEQUENCE          
    4:d=2  hl=2 l=   2 prim:   OCTET STRING      
      0000 - 00 01                                             ..
    8:d=2  hl=2 l=  25 cons:   SEQUENCE          
   10:d=3  hl=2 l=   5 prim:    BIT STRING        
      0000 - 00 0a 00 00 01                                    .....
   17:d=3  hl=2 l=   4 prim:    BIT STRING        
      0000 - 00 0a 00 01                                       ....
   23:d=3  hl=2 l=  10 cons:    SEQUENCE          
   25:d=4  hl=2 l=   3 prim:     BIT STRING        
      0000 - 01 0a 02                                          ...
   30:d=4  hl=2 l=   3 prim:     BIT STRING        
      0000 - 00 0a 04                                          ...
   35:d=1  hl=2 l=  35 cons:  SEQUENCE          
   37:d=2  hl=2 l=   2 prim:   OCTET STRING      
      0000 - 00 02                                             ..
   41:d=2  hl=2 l=  29 cons:   SEQUENCE          
   43:d=3  hl=2 l=   9 prim:    BIT STRING        
      0000 - 00 20 02 00 01 00 00 00-00                        . .......
   54:d=3  hl=2 l=  16 cons:    SEQUENCE          
   56:d=4  hl=2 l=   5 prim:     BIT STRING        
      0000 - 01 20 02 00 02                                    . ...
   63:d=4  hl=2 l=   7 prim:     BIT STRING        
      0000 - 00 20 02 00 08 00 00                              . .....

As show above, the IPAddress encoding does not take many bytes. In C509, encoding only the difference as uint needs, in general, also 4 to 8 bytes, this does not save many (or even needs more) bytes, but we need to convert between bytes and int, which is quite complex for the IPAddressPrefix.

So I suggest to change Address = bytes / uint to Address = bytes.

Following my suggestion in this comment, the syntax is changed from

      Address = bytes / uint,
      AddressPrefix = (Address, unusedBits: uint)
      AddressRange =  [Address, Address]
      IPAddressOrRange = AddressPrefix / AddressRange
      IPAddressChoice = [ + IPAddressOrRange ] / null
      IPAddrBlocks = [ AFI: uint, IPAddressChoice ]

to

      Address = ***bytes***, -- Removing the uint choice.

      AddressPrefix = (Address, unusedBits: uint)

      AddressRange =  [**min:**Address, **max:**Address]

      IPAddressOrRange = AddressPrefix / AddressRange

      IPAddressChoice = [ + IPAddressOrRange ] / null

      ***IPAddressFamily = (AFI: uint, IPAddressChoice)***

      ***IPAddrBlocks = [ + IPAddressFamily ]***

@highlunder
Copy link
Collaborator

We currently don't have enough input to make a decision. Unless any author has a strong opinion, we need to take it to the WG

@highlunder
Copy link
Collaborator

assigned Göran to follow up with Russ

@gselander
Copy link
Collaborator

gselander commented Dec 13, 2024

My current understanding:

The proposed change

OLD

      Address = bytes / uint,
      AddressPrefix = (Address, unusedBits: uint)
      AddressRange =  [Address, Address]
      IPAddressOrRange = AddressPrefix / AddressRange
      IPAddressChoice = [ + IPAddressOrRange ] / null
      IPAddrBlocks = [ AFI: uint, IPAddressChoice ]

NEW

      Address = bytes
      AddressPrefix = (Address, unusedBits: uint)
      AddressRange =  [min: Address, max: Address]
      IPAddressOrRange = AddressPrefix / AddressRange
      IPAddressChoice = [ + IPAddressOrRange ] / null
      IPAddressFamily = (AFI: uint, IPAddressChoice)
      IPAddrBlocks = [ + IPAddressFamily ]

contains a necessary extension to IPAddressFamily and an clarifying addition to Addres sRange which both are fine by me.

@xipki The other proposed changes are not addressed by this change, which of them are still relevant?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants