Skip to content

Parsing X509 IpAddressName#4

Merged
StjepanovicSrdjan merged 7 commits intodevelopmentfrom
feature/x509-ipaddressname-parsing
Sep 26, 2025
Merged

Parsing X509 IpAddressName#4
StjepanovicSrdjan merged 7 commits intodevelopmentfrom
feature/x509-ipaddressname-parsing

Conversation

@StjepanovicSrdjan
Copy link
Collaborator

Added methods for parsing IpNetwork from IpAddress name constraint as defined by RFC5280-4.2.1.10

Test examples were taken from Bouncy Castle. One thing that bothers me is that in some examples, when creating a network, the mask is applied to the address. As a result, when I call toX509Octets, it produces a different ByteArray than the one originally used for network creation. In my opinion, that behavior is correct when a user creates a new IpAddressName and passes some IpNetwork, but when parsing an incoming certificate we need to store the original ByteArray in IpAddressName so that we can re-encode it exactly as it was before. That's the case if certificate contains some random address + mask as in the tests.
Also additional thing that can be beneficial if original bytes were stored is that if we parse only address (4 bytes or 16bytes) we can extend fromX509Octets to also parse these bytes but just to set mask as (32 for v4, 124 for v6), than we don't need to store IpAddress as property in IpAddressName at all.

Related stuff in Signum PR

The explanation is a combination of this PR and the GeneralName PR in Signum. If additional context is needed, please feel free to reach out.

Copy link
Contributor

@JesusMcCloud JesusMcCloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

network always masks the IP address to ensure the associated address is really the network address. IpInterface semantics is "any IP address inside the network". I did not look into the X.509 IpAddressName detailed enough, but if we need to express that too, move your X509 encoding/decoding function to IpAddressAndName and your test case woes should go away, because you can then use IpInterface as well.

Also: changelog

@Throws(IllegalArgumentException::class)
fun fromX509Octets(bytes: ByteArray, strict: Boolean = false): IpNetwork<*, *> {
return when (bytes.size) {
8 -> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's constants for all octet sizes in (I think) IpFamily

}

/**
* Encodes this network into X.509 iPAddressName ByteArray (RFC 5280).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

illustrate the byte layout in the API Doc

Copy link
Contributor

@JesusMcCloud JesusMcCloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am greenlighting this, pending my comments

Comment on lines 60 to 61
* IPv4: [4 bytes base address][4 bytes subnet mask] (8 bytes)
* IPv6: [16 bytes base address][16 bytes subnet mask] (32 bytes)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not render correctly, because […] is just MD syntax for a href with an internal link target of the same name as the visible text

Comment on lines 60 to 61
* IPv4: [4 bytes base address][4 bytes subnet mask] (8 bytes)
* IPv6: [16 bytes base address][16 bytes subnet mask] (32 bytes)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* IPv4: [4 bytes base address][4 bytes subnet mask] (8 bytes)
* IPv6: [16 bytes base address][16 bytes subnet mask] (32 bytes)
* * IPv4 byte layout: `AAAANNNN`, where `A` ist an address octet and `N` is a netmask octet (8 bytes total)
* * IPv6 byte layout: `AAAAAAAAAAAAAAAANNNNNNNNNNNNNNNN`, where `A` ist an address octet and `N` is a netmask octet(32 bytes total)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fee free to reformat, introduce spaces of whatever, if you feel it makes it more legible

Comment on lines 48 to 49
* IPv4: [4 bytes base address][4 bytes subnet mask] (8 bytes)
* IPv6: [16 bytes base address][16 bytes subnet mask] (32 bytes)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

Comment on lines 534 to 535
* IPv4: [4 bytes base address][4 bytes subnet mask] (8 bytes)
* IPv6: [16 bytes base address][16 bytes subnet mask] (32 bytes)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

@StjepanovicSrdjan StjepanovicSrdjan merged commit 6c1e023 into development Sep 26, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants