Mistyped flow rule values

[altano]

On zeronsd startup I'm hitting this error:

ERROR - error syncing members: error in serde: invalid value: integer `4294967295`, expected i32 at line 1 column 5850

The rule I have causing this problem:

# Create a tag for which group someone is in
tag group
  id 1000
  default 0
  flag 0 productivity
  flag 1 homelab_mgmt
  flag 2 gaming
  flag 3 media
  enum 4294967295 everything    # max uint, catches all flags
;

In case it isn't clear, what I'm trying to accomplish is having a dropdown value that, when tand'd with any set of flags, is always positive. The rule that makes use of it:

# Drop any traffic between computers that don't share at least one group
break
  tand group 0
;

The ZeroTier Rules Engine documentation states:

[enum ] # value can be any 32-bit unsigned integer

So I believe something is mistyped in zeronsd and should be u32 instead of i32?

For now I am easily working around this by changing the enum to 2147483647, which is u31 effectively and fit's in a i32. (so it'll match flags 0-30, but not flag 31).

[erikh]

This is a bug in our API binding. I'll look into this right away tomorrow. Thanks for the report!

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Sunday, January 30th, 2022 at 8:33 PM, Alan @.***> wrote:

On zeronsd startup I'm hitting this error:

ERROR - error syncing members: error in serde: invalid value: integer 4294967295, expected i32 at line 1 column 5850

The rule I have causing this problem:

Create a tag for which group someone is in

tag group id 1000 default 0 flag 0 productivity flag 1 homelab_mgmt flag 2 gaming flag 3 media enum 4294967295 everything # max uint, catches all flags ;

In case it isn't clear, what I'm trying to accomplish is having a dropdown value that, when tand'd with any set of flags, is always positive. The rule that makes use of it:

Drop any traffic between computers that don't share at least one group

break tand group 0 ;

The ZeroTier Rules Engine documentation states:

[enum ] # value can be any 32-bit unsigned integer

So I believe something is mistyped in zeronsd and should be u32 instead of i32?

For now I am easily working around this by changing the enum to 2147483647, which is u31 effectively and fit's in a i32. (so it'll match flags 0-30, but not flag 31).

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you are subscribed to this thread.Message ID: @.***>

[erikh]

cc @laduke: do you know offhand where this might be in the openapi? Do we just need to do a regen of the rust api? If you don't know, no issues. I'll dive in tomorrow.

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Sunday, January 30th, 2022 at 9:25 PM, @.***> wrote:

This is a bug in our API binding. I'll look into this right away tomorrow. Thanks for the report!

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Sunday, January 30th, 2022 at 8:33 PM, Alan @.***> wrote:

On zeronsd startup I'm hitting this error:

ERROR - error syncing members: error in serde: invalid value: integer 4294967295, expected i32 at line 1 column 5850

The rule I have causing this problem:

Create a tag for which group someone is in

tag group id 1000 default 0 flag 0 productivity flag 1 homelab_mgmt flag 2 gaming flag 3 media enum 4294967295 everything # max uint, catches all flags ;

In case it isn't clear, what I'm trying to accomplish is having a dropdown value that, when tand'd with any set of flags, is always positive. The rule that makes use of it:

Drop any traffic between computers that don't share at least one group

break tand group 0 ;

The ZeroTier Rules Engine documentation states:

[enum ] # value can be any 32-bit unsigned integer

So I believe something is mistyped in zeronsd and should be u32 instead of i32?

For now I am easily working around this by changing the enum to 2147483647, which is u31 effectively and fit's in a i32. (so it'll match flags 0-30, but not flag 31).

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you are subscribed to this thread.Message ID: @.***>

[erikh]

So we looked into it today; OpenAPI has no way of expressing uint values at all, much less uint32! So this may be an ongoing issue using the API. For now, we're going to leave this ticket open and instruct people to not use the high bit of an unsigned int, which sucks as a request, but is necessary.

Sorry!

[altano]

Sounds good. It should be trivial to workaround for most. This was easy to spot and zeronsd failed hard instead of silently processing the rules incorrectly (nice!), so it was easy to workaround. The status quo should be fine but here's some food for thought:

1) If the API doesn't have a way of expressing the datatype, could you avoid the issue entirely by using an i64? It should be able to represent all values the rule engine requires? Since the rules are declarative I assume you don't have to replicate integer overflow behavior or anything? 2) If there is an easy place to validate the current rules as a person is setting up zeronsd it might be nice to provide an error during setup, the same error that pops up during startup when the rules can't be parsed.

[erikh]

It really depends on how the openapi is specified, which affects all APIs, including the ones we use for terraform/etc. I need to discuss this with the team a little more before I can confirm this.

As for the validation, I think this will go hand-in-hand with the above requirement, so I will get back to you when we come up with something.

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Monday, January 31st, 2022 at 6:13 PM, Alan @.***> wrote:

Sounds good. It should be trivial to workaround for most. This was easy to spot and zeronsd failed hard instead of silently processing the rules incorrectly (nice!), so it was easy to workaround. The status quo should be fine but here's some food for thought:

• If the API doesn't have a way of expressing the datatype, could you avoid the issue entirely by using an i64? It should be able to represent all values the rule engine requires? Since the rules are declarative I assume you don't have to replicate integer overflow behavior or anything?
• If there is an easy place to validate the current rules as a person is setting up zeronsd it might be nice to provide an error during setup, the same error that pops up during startup when the rules can't be parsed.

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you commented.Message ID: @.***>

[DePingus]

Is this issue the same as:

Apr 26 02:14:01.489 ERROR zeronsd::authority: error syncing members: error in serde: invalid type: boolean `false`, expected i32 at line 1 column 5727
Apr 26 02:14:01.576 ERROR zeronsd::authority: error syncing members: error in serde: invalid type: boolean `false`, expected i32 at line 1 column 5727
Apr 26 02:14:31.475 ERROR zeronsd::authority: error syncing members: error in serde: invalid type: boolean `false`, expected i32 at line 1 column 5687
Apr 26 02:15:01.512 ERROR zeronsd::authority: error syncing members: error in serde: invalid type: boolean `false`, expected i32 at line 1 column 5687
Apr 26 02:15:31.479 ERROR zeronsd::authority: error syncing members: error in serde: invalid type: boolean `false`, expected i32 at line 1 column 5687
Apr 26 02:16:01.482 ERROR zeronsd::authority: error syncing members: error in serde: invalid type: boolean `false`, expected i32 at line 1 column 5717
Apr 26 02:16:31.479 ERROR zeronsd::authority: error syncing members: error in serde: invalid type: boolean `false`, expected i32 at line 1 column 5717
Apr 26 02:17:01.480 ERROR zeronsd::authority: error syncing members: error in serde: invalid type: boolean `false`, expected i32 at line 1 column 5718

I tested this on Debian and Alpine, both running in an unprivileged LXD container. I get the same errors on both. This is what my ruleset looks like:

# My Custom Ruleset

# Allow only IPv4, IPv4 ARP, and IPv6 Ethernet frames.
drop
    not ethertype ipv4
    and not ethertype arp
    and not ethertype ipv6
    and not chr ipauth
;

# Create a tag for which clientgrp someone is in
tag clientgrp
  id 1000
    default 100
  enum 100 user
  enum 200 server
  enum 800 dns
  enum 999 admin
;

# Allow all TCP and UDP packets (including SYN/!ACK) to these ports on dns clientgrp
accept
  dport 53
  and ipprotocol tcp or ipprotocol udp
    and treq clientgrp 800
;

# Allow all TCP packets (including SYN/!ACK) to these ports on servers clientgrp
accept
  dport 8096 or dport 80
  and ipprotocol tcp
    and treq clientgrp 200
;

# Drop TCP SYN,!ACK packets (new connections) not explicitly whitelisted above
break                     # break can be overridden by a capability
  chr tcp_syn             # TCP SYN (TCP flags will never match non-TCP packets)
  and not chr tcp_ack     # AND not TCP ACK
;

# Create a capability called superuser that lets its holders override all but the initial drop
cap superuser
  id 1000
  accept; # allow with no match conditions means allow anything and everything
;

# Accept anything else. This is required since default is drop.
accept;

[erikh]

Let me look into this, expect a result in the next few days. Please ping me if I do not get back to you!

[erikh]

Ok, this occurs when clientgrp is unset.

Let me look into how I can resolve this for you.

[erikh]

Ok, I have some bad news here and I'm really sorry. It might take longer to fix this than I had hoped; the tools we depend on don't accommodate for the type of work we need to do with openapi to account for this API.

This is a snippet from a modified openAPI that accommodates for the false value yielded from situations where the clientgrp isn't set. Central could change this behavior but may have reasons for doing otherwise.

          "tags": {
            "type": "array",
            "items": {
              "type": "array",
              "items": {
                "anyOf": [
                  {
                    "type": "integer"
                  },
                  {
                    "type": "boolean"
                  }
                ]
              }
            },

The original version looks more like this:

          "tags": {
            "type": "array",
            "items": {
              "type": "array",
              "items": {
                    "type": "integer"
              }
            },

which is why you're seeing what you're seeing. the false comes through in the API response and serde (the de-facto serialization library for rust, and should be used for this purpose) doesn't know what to do with a boolean. Rust is a strongly typed language so floating casting it to 0 or something like that would be a big no-no for serde.

So far this hasn't been too much of an issue, except for the previous issue that lead to this ticket. However, in your case it's the actual generator not generating appropriate code for anyOf, and instead just punting. Here's the struct out for both MemberConfig (where your tags lie) and the struct it refers to in MemberConfig, which was auto-generated:

#[derive(Clone, Debug, PartialEq, Default, Serialize, Deserialize)]
pub struct MemberConfig {
    /// Allow the member to be a bridge on the network
    #[serde(rename = "activeBridge", skip_serializing_if = "Option::is_none")]
    pub active_bridge: Option<bool>,
    /// Is the member authorized on the network
    #[serde(rename = "authorized", skip_serializing_if = "Option::is_none")]
    pub authorized: Option<bool>,
    #[serde(rename = "capabilities", skip_serializing_if = "Option::is_none")]
    pub capabilities: Option<Vec<i32>>,
    /// Time the member was created or first tried to join the network
    #[serde(rename = "creationTime", skip_serializing_if = "Option::is_none")]
    pub creation_time: Option<i64>,
    /// ID of the member node.  This is the 10 digit identifier that identifies a ZeroTier node.
    #[serde(rename = "id", skip_serializing_if = "Option::is_none")]
    pub id: Option<String>,
    /// Public Key of the member's Identity
    #[serde(rename = "identity", skip_serializing_if = "Option::is_none")]
    pub identity: Option<String>,
    /// List of assigned IP addresses
    #[serde(rename = "ipAssignments", skip_serializing_if = "Option::is_none")]
    pub ip_assignments: Option<Vec<String>>,
    /// Time the member was authorized on the network
    #[serde(rename = "lastAuthorizedTime", skip_serializing_if = "Option::is_none")]
    pub last_authorized_time: Option<i64>,
    /// Time the member was deauthorized on the network
    #[serde(rename = "lastDeauthorizedTime", skip_serializing_if = "Option::is_none")]
    pub last_deauthorized_time: Option<i64>,
    /// Exempt this member from the IP auto assignment pool on a Network
    #[serde(rename = "noAutoAssignIps", skip_serializing_if = "Option::is_none")]
    pub no_auto_assign_ips: Option<bool>,
    /// Member record revision count
    #[serde(rename = "revision", skip_serializing_if = "Option::is_none")]
    pub revision: Option<i32>,
    /// Array of 2 member tuples of tag [ID, tag value]
    #[serde(rename = "tags", skip_serializing_if = "Option::is_none")]
    pub tags: Option<Vec<Vec<crate::models::MemberConfigTagsItemsInner>>>,
    /// Major version of the client
    #[serde(rename = "vMajor", skip_serializing_if = "Option::is_none")]
    pub v_major: Option<i32>,
    /// Minor version of the client
    #[serde(rename = "vMinor", skip_serializing_if = "Option::is_none")]
    pub v_minor: Option<i32>,
    /// Revision number of the client
    #[serde(rename = "vRev", skip_serializing_if = "Option::is_none")]
    pub v_rev: Option<i32>,
    /// Protocol version of the client
    #[serde(rename = "vProto", skip_serializing_if = "Option::is_none")]
    pub v_proto: Option<i32>,
}

#[derive(Clone, Debug, PartialEq, Default, Serialize, Deserialize)]
pub struct MemberConfigTagsItemsInner {
}

If it's not obvious, the inner struct contains no actual... data... of any kind.

So I need to use a new openapi generator library. I'll source one, but this ticket may take some time. In the meantime, I would encourage you to make sure all your tag ids are named and that you have all of the members of your network assigned to a tag.

[erikh]

There's a ticket open in the generator we use here already. I will keep an eye on this ticket and resolve this issue then.

I took a look at the current state of the art replacements and nothing really solves for openapi 3 yet, although paperclip may eventually. Not much I can do without a gigantic amount of engineering effort.

[DePingus]

On your suggestion I went into ZT Central and checked. I had one client that did not have a tag. Adding a tag to that client resolved the issue. Thanks!

[erikh]

I'm working on resolving the issue permanently through other means, but for now this is unfortunately what you will have to do.

[erikh]

Ok; I have a fix for this by changing openapi libraries. Expect a new release in the next few days that resolves your issue.

[SorCelien]

Hello, I also had the error in serde error on my ZeroNSD because I had tags with false values. However these tags were tags that I had removed from my flow rules. Why does the controller leave tags on members when these tags have been deleted? The only way I found to remove these old tags was to reapply the only tag I use via an api post.

[erikh]

I don't know; but I think @laduke or @glimberg might. ------- Original Message ------- On Saturday, May 7th, 2022 at 9:26 AM, SorCelien @.***> wrote:

Hello, I also had thie error in serde error on my ZeroNSD because I had tags with false values. However these tags were tags that I had removed from my flow rules. Why does the controller leave tags on members when these tags have been deleted? The only way I found to remove these old tags was to reapply the only tag I use via an api post.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

[laduke]

I don't think there's a reason other than it would be a pain to fix. The UI will delete defunct tags from members, like your api post, if you fiddle with them after changing the ruleset.

[erikh]

FWIW, This should all get resolved once https://github.com/oxidecomputer/progenitor/issues/43 happens; once it's made into a proper package I've tested solutions that work fine with all the scenarios above best I can tell.