)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000010,"name":"Mateusz Zalega","display_name":"msgctl","email":"mateusz@monogon.tech","username":"mateusz","avatars":[{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"2fac66f8aebfa797f25d94584eb14c88e25591f9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"803273b3_d696c482","updated":"2022-04-25 14:27:07.000000000","message":"To recap last week\u0027s discussion: this is needed for the node that\u0027s bootstrapping the cluster to be able to reconnect after a reboot.\n\nLooking at it now, I\u0027m not entirely satisfied with this patch, as the resulting ClusterDirectory won\u0027t contain the local node\u0027s external address, and as such won\u0027t be identical across nodes. Changing that would require adjusting some of the cluster code, but I\u0027m not sure it\u0027s worth going into this now. Let me know what you think.","commit_id":"29035deccdcfa983fe22e499bc202ab437211bd1"},{"author":{"_account_id":1000010,"name":"Mateusz Zalega","display_name":"msgctl","email":"mateusz@monogon.tech","username":"mateusz","avatars":[{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"696e278e92df73ffe3b70e76795e3466bc6f0918","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"829a6ad0_195d37e5","updated":"2022-04-25 15:15:58.000000000","message":"Oops, I messed up the rebase. Give me a second.","commit_id":"8870b2f42f3398d6bc3e2ff1408625296592859c"},{"author":{"_account_id":1000010,"name":"Mateusz Zalega","display_name":"msgctl","email":"mateusz@monogon.tech","username":"mateusz","avatars":[{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"36b19761a7632bb6e2da5fbf37acd49287c53823","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"60709e30_38930b3a","updated":"2022-04-26 11:22:04.000000000","message":"Also, at this point we probably should also rename this service.\n\nI still believe that extending this service to also handle ClusterDirectory leads to a more compact solution. Otherwise I\u0027d need to somehow refactor this piece of code around the rpc stream. ","commit_id":"32b10bb5e9fe3c243d4c3fb62ae9468f26540475"},{"author":{"_account_id":1000010,"name":"Mateusz Zalega","display_name":"msgctl","email":"mateusz@monogon.tech","username":"mateusz","avatars":[{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"d8570f58df87ca8815a1c3fb6055585fbaeec8f0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5cd38926_e5bc45f5","updated":"2022-04-25 15:32:46.000000000","message":"There you go.","commit_id":"32b10bb5e9fe3c243d4c3fb62ae9468f26540475"}],"metropolis/node/core/network/hostsfile/hostsfile.go":[{"author":{"_account_id":1000001,"name":"Lorenz Brun","display_name":"Lorenz","email":"lorenz@monogon.tech","username":"lorenz","avatars":[{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/75c04f6e9881c24ee621fba80667eed8.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"507be5713e80edf6f81889e1dc0a36043dcaf520","unresolved":true,"context_lines":[{"line_number":122,"context_line":"\t\t\t{Host: ni.address},"},{"line_number":123,"context_line":"\t\t}"},{"line_number":124,"context_line":"\t\tnode :\u003d \u0026cpb.ClusterDirectory_Node{"},{"line_number":125,"context_line":"\t\t\tPublicKey: ni.pubkey,"},{"line_number":126,"context_line":"\t\t\tAddresses: addresses,"},{"line_number":127,"context_line":"\t\t}"},{"line_number":128,"context_line":"\t\tdirectory.Nodes \u003d append(directory.Nodes, node)"}],"source_content_type":"text/x-go","patch_set":4,"id":"831d79b1_e5c2279c","line":125,"range":{"start_line":125,"start_character":3,"end_line":125,"end_character":12},"updated":"2022-04-27 11:18:47.000000000","message":"Unless I\u0027m missing something, you can\u0027t do that. The Cluster Directory is untrusted as it is both unsigned and unencrypted. Storing the connection public key in it could allow one to put a malicious public key in it and then the node would connect to a node not actually in the cluster.","commit_id":"32b10bb5e9fe3c243d4c3fb62ae9468f26540475"},{"author":{"_account_id":1000002,"name":"Serge Bazanski","display_name":"Serge","email":"serge@monogon.tech","username":"serge","avatars":[{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/52c41428b6369f2c02b9717425216f7d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"e4d8d792da5ec0adeb97fef59cee2021f461f2da","unresolved":true,"context_lines":[{"line_number":122,"context_line":"\t\t\t{Host: ni.address},"},{"line_number":123,"context_line":"\t\t}"},{"line_number":124,"context_line":"\t\tnode :\u003d \u0026cpb.ClusterDirectory_Node{"},{"line_number":125,"context_line":"\t\t\tPublicKey: ni.pubkey,"},{"line_number":126,"context_line":"\t\t\tAddresses: addresses,"},{"line_number":127,"context_line":"\t\t}"},{"line_number":128,"context_line":"\t\tdirectory.Nodes \u003d append(directory.Nodes, node)"}],"source_content_type":"text/x-go","patch_set":4,"id":"b578b820_0ffc0bec","line":125,"range":{"start_line":125,"start_character":3,"end_line":125,"end_character":12},"in_reply_to":"10c525c9_fd09fda7","updated":"2022-04-27 13:35:11.000000000","message":"The confusion seems to stem from the fact that the cluster directory proto definition is not just used to be saved on the ESP, but generally used as a summarized state of the cluster as reported by the curator to nodes which want it. \n\nFor example, the cluster directory as retrieved by the GetClusterInfo is synthesized from etcd state on request, not saved as is or made up of untrusted data. Nodes don\u0027t ever report a cluster directory to the server, they just get it from various sources (eg. the curator as above, or now from ESP on startup).\n\nWe choose to save the cluster directory on the ESP not because we explicitly need that proto structure for something, only because it\u0027s easy to retrieve from the cluster and contains all the data we need to join the cluster. We don\u0027t ever use the node public keys during the join flow. In fact, for now, we only use the node public key just as a generic unique key in some operations on the cluster directory within cluster logic.\n\nThe general issue is that the cluster directory, as retrieved from the ESP and used for the join flow, should be treated as radioactive (since it\u0027s explicitly attacker controlled), and we should attempt to minimize the likelihood of anyone ever accidentally using the public keys possibly contained within for anything serious. Currently they aren\u0027t used for anything, as on join flow we trust a TPM sealed CA, not the public keys of any nodes. But it\u0027s about preventing an accident in the future. There\u0027s a few ways I can think of to achieve this:\n\n 1. During the join flow, explicitly sanitize the cluster directory to only contain the bare minimum of information and dispose of the unsanitized copy;\n 2. Use a different proto definition entirely on the ESP, one that will never contain anything but the bare minimum required to join the cluster; or\n 3. Ensure the cluster directory proto definition contains only fields that cannot be misused (I don\u0027t like that option, as the cluster directory is used in trusted scenarios more often than not)\n\nFinally, a drive-by comment on this CL: I don\u0027t like that we\u0027re re-synthesizing a cluster directory here from the nodemap instead of just taking something directly from the Curator. But I think I understand why we\u0027re doing this: because the ClusterDirectory can only be retrieved from a unary GetClusterInfo call, not a streaming Watch call - which makes saving an up to date ClusterDirectory painful. If that\u0027s the case, then whatever, let\u0027s do that.","commit_id":"32b10bb5e9fe3c243d4c3fb62ae9468f26540475"},{"author":{"_account_id":1000010,"name":"Mateusz Zalega","display_name":"msgctl","email":"mateusz@monogon.tech","username":"mateusz","avatars":[{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"f8a3d1f652577c5f90a4c71d696c50d813e58469","unresolved":false,"context_lines":[{"line_number":122,"context_line":"\t\t\t{Host: ni.address},"},{"line_number":123,"context_line":"\t\t}"},{"line_number":124,"context_line":"\t\tnode :\u003d \u0026cpb.ClusterDirectory_Node{"},{"line_number":125,"context_line":"\t\t\tPublicKey: ni.pubkey,"},{"line_number":126,"context_line":"\t\t\tAddresses: addresses,"},{"line_number":127,"context_line":"\t\t}"},{"line_number":128,"context_line":"\t\tdirectory.Nodes \u003d append(directory.Nodes, node)"}],"source_content_type":"text/x-go","patch_set":4,"id":"16ebca7e_7a613331","line":125,"range":{"start_line":125,"start_character":3,"end_line":125,"end_character":12},"in_reply_to":"54c3f248_9311db81","updated":"2022-04-29 17:22:27.000000000","message":"Done","commit_id":"32b10bb5e9fe3c243d4c3fb62ae9468f26540475"},{"author":{"_account_id":1000010,"name":"Mateusz Zalega","display_name":"msgctl","email":"mateusz@monogon.tech","username":"mateusz","avatars":[{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"2889ccf508f509bcfb29f5d1d863952505f7244e","unresolved":true,"context_lines":[{"line_number":122,"context_line":"\t\t\t{Host: ni.address},"},{"line_number":123,"context_line":"\t\t}"},{"line_number":124,"context_line":"\t\tnode :\u003d \u0026cpb.ClusterDirectory_Node{"},{"line_number":125,"context_line":"\t\t\tPublicKey: ni.pubkey,"},{"line_number":126,"context_line":"\t\t\tAddresses: addresses,"},{"line_number":127,"context_line":"\t\t}"},{"line_number":128,"context_line":"\t\tdirectory.Nodes \u003d append(directory.Nodes, node)"}],"source_content_type":"text/x-go","patch_set":4,"id":"54c3f248_9311db81","line":125,"range":{"start_line":125,"start_character":3,"end_line":125,"end_character":12},"in_reply_to":"7bdd053d_b0e68d19","updated":"2022-04-28 13:21:38.000000000","message":"Intuitively 1. still seems kind of bad, so maybe I\u0027ll make it a 2. after all, OTOH I don\u0027t like the code bloat.","commit_id":"32b10bb5e9fe3c243d4c3fb62ae9468f26540475"},{"author":{"_account_id":1000010,"name":"Mateusz Zalega","display_name":"msgctl","email":"mateusz@monogon.tech","username":"mateusz","avatars":[{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"7bcaf289cd62dcc685dae06882f74c2df90be8c2","unresolved":true,"context_lines":[{"line_number":122,"context_line":"\t\t\t{Host: ni.address},"},{"line_number":123,"context_line":"\t\t}"},{"line_number":124,"context_line":"\t\tnode :\u003d \u0026cpb.ClusterDirectory_Node{"},{"line_number":125,"context_line":"\t\t\tPublicKey: ni.pubkey,"},{"line_number":126,"context_line":"\t\t\tAddresses: addresses,"},{"line_number":127,"context_line":"\t\t}"},{"line_number":128,"context_line":"\t\tdirectory.Nodes \u003d append(directory.Nodes, node)"}],"source_content_type":"text/x-go","patch_set":4,"id":"10c525c9_fd09fda7","line":125,"range":{"start_line":125,"start_character":3,"end_line":125,"end_character":12},"in_reply_to":"831d79b1_e5c2279c","updated":"2022-04-27 13:13:10.000000000","message":"That\u0027s a good catch. I did this, because Public Keys were already being supplied to nodes\u0027 cluster directories in the cluster launch test.\n\nmetropolis/test/launch/cluster/cluster.go:\n```\n709   resI, err :\u003d mgmt.GetClusterInfo(ctx, \u0026apb.GetClusterInfoRequest{})\n```\n\nThese public keys, however, are being used only to log node IDs during startup. If that\u0027s the only use case we could ever have, should we remove public keys from the ClusterDirectory protobuf entirely?","commit_id":"32b10bb5e9fe3c243d4c3fb62ae9468f26540475"},{"author":{"_account_id":1000010,"name":"Mateusz Zalega","display_name":"msgctl","email":"mateusz@monogon.tech","username":"mateusz","avatars":[{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/30cae8ca0782f23ce0a60ac80fda3dd9.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"598657780b7cf3b3f2feb784fdee11e96e84bb48","unresolved":true,"context_lines":[{"line_number":122,"context_line":"\t\t\t{Host: ni.address},"},{"line_number":123,"context_line":"\t\t}"},{"line_number":124,"context_line":"\t\tnode :\u003d \u0026cpb.ClusterDirectory_Node{"},{"line_number":125,"context_line":"\t\t\tPublicKey: ni.pubkey,"},{"line_number":126,"context_line":"\t\t\tAddresses: addresses,"},{"line_number":127,"context_line":"\t\t}"},{"line_number":128,"context_line":"\t\tdirectory.Nodes \u003d append(directory.Nodes, node)"}],"source_content_type":"text/x-go","patch_set":4,"id":"7bdd053d_b0e68d19","line":125,"range":{"start_line":125,"start_character":3,"end_line":125,"end_character":12},"in_reply_to":"b578b820_0ffc0bec","updated":"2022-04-28 13:18:18.000000000","message":"I\u0027ll go with 1. Judging by the comment, ClusterDirectory was never meant to be trusted, and it really isn\u0027t now (as the public keys are used only to log node IDs). If we\u0027d like to enforce that rule and guide development by making it impossible to access and depend on the public keys being there, that\u0027s fine.\n\n```\n// The directory explicitly doesn\u0027t carry any information about the cluster\u0027s\n// identity or security - these should be configured and checked by higher\n// level configuration and processes. The directory can be stored and\n// transmitted in cleartext and without an integrity checks (like saved to the\n// EFI system partition across reboots) and any malicious change to it will\n// cause no more than a denial of service against the consumer of this\n// directory. This is because all nodes contacted must present a valid cluster\n// identity/certificate before they are trusted by the consumers of this\n// directory.\nmessage ClusterDirectory {\n    message Node {\n        bytes public_key \u003d 1;\n        message Address {\n            string host \u003d 1;\n        };\n        repeated Address addresses \u003d 2;\n    };\n    repeated Node nodes \u003d 1;\n}\n```","commit_id":"32b10bb5e9fe3c243d4c3fb62ae9468f26540475"}]}