Commit c83fa3cb authored by Kirill Smelkov's avatar Kirill Smelkov

software/ors-amarisoft: enb/generic: Protect from buildout code injection

We currently have at least the following problem:

1. a malfored shared instance reference leads to instantiation failure and
   potential buildout code injection. For example if reference of shared
   instance contains space then buildout fails with something like

       zc.buildout.configparser.ParsingError: File contains parsing errors: .../instance-enb.cfg
           [line 45]: '[promise-testing partition 0.RU-sdr-busy]\n'

   since, when requesting, it is possible to use arbitrary characters for
   references, including newline, it also opens the door for code injection attacks.

2. we currently use `json` directive of slapos.recipe.template e.g. as

     extra-context =
         json iru_dict   {{ rulib.iru_dict   | tojson }}

   this also potentially leads to instantiation failure if JSON(rulib.iru_dict)
   contains buildout control characters.

Solve this problems.

For 1 we develop "buildout encoding" that encodes arbitrary string into form
that is safe to be used as a name of buildout section. This encoding never fails
and does not loose information, which means it is safe to be applied
unconditionanly and there is no chance for two string inputs to result in the
same encoded form. The encoding also has practical property that it leaves most
of the strings, that we use in our buildot profiles, intact. Which means that
the result of the encoding is readable and the encoding can be applied almost
unconditionally to all strings without hurting instantiation. Then this
encoding is applied wherever reference of a shared instance is used to form
e.g. name of buildout section or inside other codes. We apply this encoding
universally - in config files too to avoid breakage there - e.g. in enb.cfg to
protect from e.g. \n being inserted in the middle of comment resulting in YAML
breakage. See documentation of added xbuildout.py for details.

For 2 we switch from `json` to using `dumps` on an object and referring that
dumps result via `key` directive.

See the following links for preliminary discussions with Jérome and Vincent on
this topic:

kirr/slapos@d5d4f7c8
nexedi/slapos@bf6b2ef3
parent b168cb33
...@@ -78,6 +78,10 @@ md5sum = 2b08bb666c5f3ab287cdddbfdb4c9249 ...@@ -78,6 +78,10 @@ md5sum = 2b08bb666c5f3ab287cdddbfdb4c9249
_update_hash_filename_ = ru/tapsplit _update_hash_filename_ = ru/tapsplit
md5sum = 2b8b57c5771b2a2203c0e7767e629e55 md5sum = 2b8b57c5771b2a2203c0e7767e629e55
[ru_xbuildout.py]
_update_hash_filename_ = ru/xbuildout.py
md5sum = a51171f926edd315a52841c2e7eb9fb7
[ru_capdo.c] [ru_capdo.c]
_update_hash_filename_ = ru/capdo.c _update_hash_filename_ = ru/capdo.c
md5sum = 52da9fe3a569199e35ad89ae1a44c30e md5sum = 52da9fe3a569199e35ad89ae1a44c30e
......
// DRB configuration for LTE cell {{ cell_ref }} @ {{ ru_ref }}. {%- set B = xbuildout.encode -%}
// DRB configuration for LTE cell {{ B(cell_ref) }} @ {{ B(ru_ref) }}.
// DRB configuration vary in beteen FDD and TDD modes. // DRB configuration vary in beteen FDD and TDD modes.
{% set T_REORDERING = {'fdd': 35, 'tdd': 65} [cell.rf_mode] %} {% set T_REORDERING = {'fdd': 35, 'tdd': 65} [cell.rf_mode] %}
......
// DRB configuration for NR cell {{ cell_ref }} @ {{ ru_ref }}. {%- set B = xbuildout.encode -%}
// DRB configuration for NR cell {{ B(cell_ref) }} @ {{ B(ru_ref) }}.
[ [
{ {
......
{%- import 'slaplte.jinja2' as slaplte with context %} {%- import 'slaplte.jinja2' as slaplte with context %}
{%- set B = slaplte.B %}
{%- set J = slaplte.J %} {%- set J = slaplte.J %}
{%- set jcell_ru_ref = slaplte.jcell_ru_ref %} {%- set jcell_ru_ref = slaplte.jcell_ru_ref %}
{%- set ierror = slaplte.ierror %} {%- set ierror = slaplte.ierror %}
...@@ -38,13 +39,13 @@ ...@@ -38,13 +39,13 @@
{ {
{%- if ncell.cell_type == 'lte' %} {%- if ncell.cell_type == 'lte' %}
rat: "eutra", rat: "eutra",
cell_id: {{ ncell.e_cell_id }}, // -> {{ peercell_ref }} cell_id: {{ ncell.e_cell_id }}, // -> {{ B(peercell_ref) }}
n_id_cell: {{ ncell.pci }}, n_id_cell: {{ ncell.pci }},
dl_earfcn: {{ ncell.dl_earfcn }}, dl_earfcn: {{ ncell.dl_earfcn }},
tac: {{ ncell.tac }}, tac: {{ ncell.tac }},
{%- elif ncell.cell_type == 'nr' %} {%- elif ncell.cell_type == 'nr' %}
rat: "nr", rat: "nr",
nr_cell_id: {{ ncell.nr_cell_id }}, // -> {{ peercell_ref }} nr_cell_id: {{ ncell.nr_cell_id }}, // -> {{ B(peercell_ref) }}
gnb_id_bits: {{ ncell.gnb_id_bits }}, gnb_id_bits: {{ ncell.gnb_id_bits }},
n_id_cell: {{ ncell.pci }}, n_id_cell: {{ ncell.pci }},
dl_nr_arfcn: {{ ncell.dl_nr_arfcn }}, dl_nr_arfcn: {{ ncell.dl_nr_arfcn }},
...@@ -209,7 +210,7 @@ ...@@ -209,7 +210,7 @@
{%- set iru = iru_dict[ru_ref] %} {%- set iru = iru_dict[ru_ref] %}
{%- set ru = iru['_'] %} {%- set ru = iru['_'] %}
// {{ cell_ref }} ({{ ru_ref }}) // {{ B(cell_ref) }} ({{ B(ru_ref) }})
{ {
rf_port: {{ ru._rf_port }}, rf_port: {{ ru._rf_port }},
n_antenna_dl: {{ ru.n_antenna_dl }}, n_antenna_dl: {{ ru.n_antenna_dl }},
...@@ -231,7 +232,7 @@ ...@@ -231,7 +232,7 @@
{%- set cell2 = icell2['_'] %} {%- set cell2 = icell2['_'] %}
{%- if cell2_ref != cell_ref %} {%- if cell2_ref != cell_ref %}
{ {
cell_id: {{ cell2.cell_id }}, // + {{ cell2_ref }} cell_id: {{ cell2.cell_id }}, // + {{ B(cell2_ref) }}
cross_carrier_scheduling: false, cross_carrier_scheduling: false,
}, },
{%- endif %} {%- endif %}
...@@ -313,11 +314,11 @@ ...@@ -313,11 +314,11 @@
srs_hopping_bandwidth: 0, srs_hopping_bandwidth: 0,
}, },
drb_config: "{{ '%s-drb.cfg' % cell_ref }}", drb_config: "{{ B('%s-drb.cfg' % cell_ref) }}",
sib_sched_list: [ sib_sched_list: [
{ {
filename: "{{ '%s-sib23.asn' % cell_ref }}", filename: "{{ B('%s-sib23.asn' % cell_ref) }}",
si_periodicity: 16, si_periodicity: 16,
}, },
], ],
...@@ -429,7 +430,7 @@ ...@@ -429,7 +430,7 @@
{%- set iru = iru_dict[ru_ref] %} {%- set iru = iru_dict[ru_ref] %}
{%- set ru = iru['_'] %} {%- set ru = iru['_'] %}
// {{ cell_ref }} ({{ ru_ref }}) // {{ B(cell_ref) }} ({{ B(ru_ref) }})
{ {
rf_port: {{ ru._rf_port }}, rf_port: {{ ru._rf_port }},
n_antenna_dl: {{ ru.n_antenna_dl }}, n_antenna_dl: {{ ru.n_antenna_dl }},
...@@ -686,7 +687,7 @@ ...@@ -686,7 +687,7 @@
], ],
}, },
drb_config: "{{ '%s-drb.cfg' % cell_ref }}", drb_config: "{{ B('%s-drb.cfg' % cell_ref) }}",
}, },
{%- endfor %} {%- endfor %}
], ],
......
/* SIB2/SIB3 for {{ cell.cell_type | upper }} cell {{ cell_ref }} @ {{ ru_ref }}. */ {%- set B = xbuildout.encode -%}
/* SIB2/SIB3 for {{ cell.cell_type | upper }} cell {{ B(cell_ref) }} @ {{ B(ru_ref) }}. */
{ {
message c1: systemInformation: { message c1: systemInformation: {
criticalExtensions systemInformation-r8: { criticalExtensions systemInformation-r8: {
......
...@@ -195,6 +195,7 @@ context = ...@@ -195,6 +195,7 @@ context =
raw gtp_addr_v6 {{ my_ipv6 }} raw gtp_addr_v6 {{ my_ipv6 }}
raw gtp_addr_v4 {{ lan_ipv4 }} raw gtp_addr_v4 {{ lan_ipv4 }}
raw gtp_addr_lo {{ gtp_addr_lo }} raw gtp_addr_lo {{ gtp_addr_lo }}
import xbuildout xbuildout
import netaddr netaddr import netaddr netaddr
${:extra-context} ${:extra-context}
...@@ -206,14 +207,18 @@ url = ${enb-config-dl:target} ...@@ -206,14 +207,18 @@ url = ${enb-config-dl:target}
url = {{ enb_template }} url = {{ enb_template }}
{% endif %} {% endif %}
output = ${directory:etc}/enb.cfg output = ${directory:etc}/enb.cfg
extra-context =
import json_module json
json iru_dict {{ rulib.iru_dict | tojson }}
json icell_dict {{ rulib.icell_dict | tojson }}
json ipeer_dict {{ ipeer_dict | tojson }}
json ipeercell_dict {{ ipeercell_dict | tojson }}
import-list = import-list =
rawfile slaplte.jinja2 {{ slaplte_template }} rawfile slaplte.jinja2 {{ slaplte_template }}
extra-context =
import json_module json
key iru_dict :iru_dict
key icell_dict :icell_dict
key ipeer_dict :ipeer_dict
key ipeercell_dict :ipeercell_dict
iru_dict = {{ dumps(rulib.iru_dict) }}
icell_dict = {{ dumps(rulib.icell_dict) }}
ipeer_dict = {{ dumps(ipeer_dict) }}
ipeercell_dict = {{ dumps(ipeercell_dict) }}
[publish-connection-information] [publish-connection-information]
......
...@@ -24,12 +24,15 @@ cert = $${slap-connection:cert-file} ...@@ -24,12 +24,15 @@ cert = $${slap-connection:cert-file}
recipe = slapos.recipe.template:jinja2 recipe = slapos.recipe.template:jinja2
output = $${buildout:directory}/$${:filename} output = $${buildout:directory}/$${:filename}
extra-context = extra-context =
depends = $${activate-eggs:recipe}
context = context =
import xbuildout xbuildout
import json_module json import json_module json
import netaddr netaddr import netaddr netaddr
key eggs_directory buildout:eggs-directory key eggs_directory buildout:eggs-directory
key develop_eggs_directory buildout:develop-eggs-directory key develop_eggs_directory buildout:develop-eggs-directory
raw buildout_directory ${buildout:directory} raw buildout_directory ${buildout:directory}
section directory directory
raw pythonwitheggs ${buildout:bin-directory}/pythonwitheggs raw pythonwitheggs ${buildout:bin-directory}/pythonwitheggs
section slap_connection slap-connection section slap_connection slap-connection
key slapparameter_dict slap-configuration:configuration key slapparameter_dict slap-configuration:configuration
...@@ -43,6 +46,20 @@ import-list = ...@@ -43,6 +46,20 @@ import-list =
rawfile ru_lopcomm_libinstance.jinja2.cfg ${ru_lopcomm_libinstance.jinja2.cfg:target} rawfile ru_lopcomm_libinstance.jinja2.cfg ${ru_lopcomm_libinstance.jinja2.cfg:target}
rawfile ru_sunwave_libinstance.jinja2.cfg ${ru_sunwave_libinstance.jinja2.cfg:target} rawfile ru_sunwave_libinstance.jinja2.cfg ${ru_sunwave_libinstance.jinja2.cfg:target}
# activate eggs and modules used in jinja2 templates
[activate-eggs]
recipe = slapos.recipe.build
init =
# ~ import xbuildout
import sys, types
def readfile(path):
with open(path) as f:
return f.read()
xbuildout = types.ModuleType('xbuildout')
exec(readfile('${ru_xbuildout.py:target}'), xbuildout.__dict__)
assert 'xbuildout' not in sys.modules
sys.modules['xbuildout'] = xbuildout
[amarisoft] [amarisoft]
recipe = slapos.recipe.build recipe = slapos.recipe.build
......
...@@ -39,3 +39,6 @@ exe = ${netcapdo:exe} ...@@ -39,3 +39,6 @@ exe = ${netcapdo:exe}
[ru_amarisoft-rf-info.jinja2.py] [ru_amarisoft-rf-info.jinja2.py]
<= download-base <= download-base
[ru_xbuildout.py]
<= download-base
{%- set B = xbuildout.encode -%}
dhcp-leasefile={{ directory['etc'] }}/dnsmasq.leases dhcp-leasefile={{ directory['etc'] }}/dnsmasq.leases
port=5354 port=5354
...@@ -7,7 +9,7 @@ port=5354 ...@@ -7,7 +9,7 @@ port=5354
{%- set vtap = json_module.loads(vtap_jdict[ru_tap]) %} {%- set vtap = json_module.loads(vtap_jdict[ru_tap]) %}
{%- set plen = netaddr.IPNetwork(vtap.network).prefixlen %} {%- set plen = netaddr.IPNetwork(vtap.network).prefixlen %}
# {{ ru_ref }} @ {{ ru_tap }} # {{ B(ru_ref) }} @ {{ ru_tap }}
{#- TODO consider using /128 as we give only 1 address to RU #} {#- TODO consider using /128 as we give only 1 address to RU #}
dhcp-range=tag:{{ ru_tap }},{{ vtap.gateway }},{{ vtap.gateway }},static,{{ max(plen,64) }},5m dhcp-range=tag:{{ ru_tap }},{{ vtap.gateway }},{{ vtap.gateway }},static,{{ max(plen,64) }},5m
dhcp-host={{ ru.mac_addr }},tag:{{ ru_tap }},[{{ vtap.gateway }}] dhcp-host={{ ru.mac_addr }},tag:{{ ru_tap }},[{{ vtap.gateway }}]
......
...@@ -23,11 +23,14 @@ ...@@ -23,11 +23,14 @@
{%- set root = slap_configuration['instance-title'] %} {%- set root = slap_configuration['instance-title'] %}
{%- set testing = slapparameter_dict.get("testing", False) %} {%- set testing = slapparameter_dict.get("testing", False) %}
{#- B(name) returns buildout-encoded form of name #}
{%- set B = xbuildout.encode %}
{#- part emits new buildout section and registers it into buildout.parts #} {#- part emits new buildout section and registers it into buildout.parts #}
{%- set parts_list = [] %} {%- set parts_list = [] %}
{%- macro part(name) %} {%- macro part(name) %}
{%- do parts_list.append(name) %} {%- do parts_list.append(B(name)) %}
[{{ name }}] [{{ B(name) }}]
{%- endmacro %} {%- endmacro %}
{#- promise emits new buildout section for a promise #} {#- promise emits new buildout section for a promise #}
...@@ -36,7 +39,8 @@ ...@@ -36,7 +39,8 @@
{%- set pretty_name = name.removeprefix('%s.' % root) %} {%- set pretty_name = name.removeprefix('%s.' % root) %}
{{ part('promise-'+name) }} {{ part('promise-'+name) }}
<= monitor-promise-base <= monitor-promise-base
name = {{ pretty_name }}.py name = {{ dumps('%s.py' % pretty_name) }}
output = {{ dumps('%s/plugin/%s.py' % (directory.etc, pretty_name)) }}
config-testing = {{ testing }} config-testing = {{ testing }}
config-stats-period = {{ slapparameter_dict.get("enb_stats_fetch_period", 60) }} config-stats-period = {{ slapparameter_dict.get("enb_stats_fetch_period", 60) }}
{%- endmacro %} {%- endmacro %}
...@@ -151,11 +155,13 @@ filename = dnsmasq.cfg ...@@ -151,11 +155,13 @@ filename = dnsmasq.cfg
extensions = jinja2.ext.do extensions = jinja2.ext.do
output = ${directory:etc}/${:filename} output = ${directory:etc}/${:filename}
context = context =
import xbuildout xbuildout
import json_module json import json_module json
import netaddr netaddr import netaddr netaddr
section directory directory section directory directory
section vtap_jdict vtap_jdict section vtap_jdict vtap_jdict
json iru_dict {{ iru_dict | tojson }} key iru_dict :iru_dict
iru_dict = {{ dumps(iru_dict) }}
{{ part('dnsmasq-service') }} {{ part('dnsmasq-service') }}
recipe = slapos.cookbook:wrapper recipe = slapos.cookbook:wrapper
...@@ -185,7 +191,7 @@ hash-files = ...@@ -185,7 +191,7 @@ hash-files =
{%- endif %} {%- endif %}
{%- endfor %} {%- endfor %}
# {{ ru_ref }} {{ ru.n_antenna_dl }}T{{ ru.n_antenna_ul }}R ({{ ru.ru_type}}) # {{ dumps(ru_ref) }} {{ ru.n_antenna_dl }}T{{ ru.n_antenna_ul }}R ({{ ru.ru_type }})
{%- if ru.ru_link_type == 'sdr' %} {%- if ru.ru_link_type == 'sdr' %}
{{ promise('%s-sdr-busy' % ru_ref) }} {{ promise('%s-sdr-busy' % ru_ref) }}
promise = check_sdr_busy promise = check_sdr_busy
...@@ -236,31 +242,39 @@ config-max-rx-sample-db = {{ slapparameter_dict.get("max_rx_sample_db", 0) }} ...@@ -236,31 +242,39 @@ config-max-rx-sample-db = {{ slapparameter_dict.get("max_rx_sample_db", 0) }}
{{ part('drb-config-%s' % cell_ref) }} {{ part('drb-config-%s' % cell_ref) }}
<= config-base <= config-base
url = {{ {'lte': drb_lte_template, 'nr': drb_nr_template} [cell.cell_type] }} url = {{ {'lte': drb_lte_template, 'nr': drb_nr_template} [cell.cell_type] }}
output = ${directory:etc}/{{'%s-drb.cfg' % cell_ref}} output = ${directory:etc}/{{B('%s-drb.cfg' % cell_ref)}}
extra-context = extra-context =
json cell_ref {{ cell_ref | tojson }} key cell_ref :cell_ref
json cell {{ cell | tojson }} key cell :cell
json ru_ref {{ ru_ref | tojson }} key ru_ref :ru_ref
json ru {{ ru | tojson }} key ru :ru
cell_ref = {{ dumps(cell_ref) }}
cell = {{ dumps(cell ) }}
ru_ref = {{ dumps(ru_ref ) }}
ru = {{ dumps(ru ) }}
{{ part('sib23-config-%s' % cell_ref) }} {{ part('sib23-config-%s' % cell_ref) }}
<= config-base <= config-base
url = {{ sib23_template }} url = {{ sib23_template }}
output = ${directory:etc}/{{'%s-sib23.asn' % cell_ref}} output = ${directory:etc}/{{B('%s-sib23.asn' % cell_ref)}}
extra-context = extra-context =
json cell_ref {{ cell_ref | tojson }} key cell_ref :cell_ref
json cell {{ cell | tojson }} key cell :cell
json ru_ref {{ ru_ref | tojson }} key ru_ref :ru_ref
json ru {{ ru | tojson }} key ru :ru
cell_ref = {{ dumps(cell_ref) }}
cell = {{ dumps(cell ) }}
ru_ref = {{ dumps(ru_ref ) }}
ru = {{ dumps(ru ) }}
{#- publish information about the cell (skipping synthetic) #} {#- publish information about the cell (skipping synthetic) #}
{%- if icell.slave_reference %} {%- if icell.slave_reference %}
[publish-connection-information] [publish-connection-information]
{%- if cell.cell_type == 'lte' %} {%- if cell.cell_type == 'lte' %}
{{cell_ref}}-dl_earfcn = {{ dumps(cell.dl_earfcn) }} {{B(cell_ref)}}-dl_earfcn = {{ dumps(cell.dl_earfcn) }}
{%- elif cell.cell_type == 'nr' %} {%- elif cell.cell_type == 'nr' %}
{{cell_ref}}-band = {{ dumps('n%d' % cell.nr_band) }} {{B(cell_ref)}}-band = {{ dumps('n%d' % cell.nr_band) }}
{{cell_ref}}-dl_nr_arfcn = {{ dumps(cell.dl_nr_arfcn) }} {{B(cell_ref)}}-dl_nr_arfcn = {{ dumps(cell.dl_nr_arfcn) }}
{%- else %} {%- else %}
{%- do bug('unreachable') %} {%- do bug('unreachable') %}
{%- endif %} {%- endif %}
...@@ -306,7 +320,8 @@ context = ...@@ -306,7 +320,8 @@ context =
raw stats_period {{ slapparameter_dict.get("enb_stats_fetch_period", 60) }} raw stats_period {{ slapparameter_dict.get("enb_stats_fetch_period", 60) }}
raw testing {{ testing }} raw testing {{ testing }}
raw python_path {{ buildout_directory}}/bin/pythonwitheggs raw python_path {{ buildout_directory}}/bin/pythonwitheggs
json iru_dict {{ iru_dict | tojson }} key iru_dict :iru_dict
iru_dict = {{ dumps(iru_dict) }}
mode = 0775 mode = 0775
url = {{ ru_amarisoft_stats_template }} url = {{ ru_amarisoft_stats_template }}
output = ${directory:bin}/amarisoft-stats.py output = ${directory:bin}/amarisoft-stats.py
......
...@@ -22,14 +22,14 @@ config-port = 830 ...@@ -22,14 +22,14 @@ config-port = 830
{#- push firmware to RU #} {#- push firmware to RU #}
[{{ru_ref}}-software-template] {{ part('%s-software-template' % ru_ref) }}
recipe = slapos.recipe.template:jinja2 recipe = slapos.recipe.template:jinja2
extensions = jinja2.ext.do extensions = jinja2.ext.do
_logbase = ${directory:var}/log/{{ru_ref}}-software _logbase = ${directory:var}/log/{{B('%s-software' % ru_ref)}}
log-output = ${:_logbase}.log log-output = ${:_logbase}.log
software-reply-json-log-output = ${:_logbase}-reply.json.log software-reply-json-log-output = ${:_logbase}-reply.json.log
remote-file-path = sftp://${user-info:pw-name}@[${sshd-service:ipv6}]:${sshd-service:port}{{ru_lopcomm_firmware_path}} remote-file-path = sftp://${user-info:pw-name}@[${sshd-service:ipv6}]:${sshd-service:port}{{ru_lopcomm_firmware_path}}
is_firmware_updated = ${directory:etc}/{{ru_ref}}.is_firmware_updated is_firmware_updated = ${directory:etc}/{{B('%s.is_firmware_updated' % ru_ref)}}
context = context =
section directory directory section directory directory
section vtap vtap.{{ ru.cpri_link._tap }} section vtap vtap.{{ ru.cpri_link._tap }}
...@@ -45,41 +45,43 @@ context = ...@@ -45,41 +45,43 @@ context =
import netaddr netaddr import netaddr netaddr
mode = 0775 mode = 0775
url = {{ ru_lopcomm_software_template }} url = {{ ru_lopcomm_software_template }}
output = ${directory:script}/{{ru_ref}}-software.py output = ${directory:script}/{{B('%s-software.py' % ru_ref)}}
{%- if not testing %} {%- if not testing %}
{{ promise('%s-firmware' % ru_ref) }} {{ promise('%s-firmware' % ru_ref) }}
promise = check_command_execute promise = check_command_execute
config-command = [ -f ${ {{-ru_ref}}-software-template:is_firmware_updated} ] config-command = [ -f ${ {{-B('%s-software-template' % ru_ref)}}:is_firmware_updated} ]
{%- endif %} {%- endif %}
{#- push config to RU #} {#- push config to RU #}
{% if ru.get("cu_config_link", None) %} {% if ru.get("cu_config_link", None) %}
[{{ ru_ref }}-cu-config-dl }}] [{{ B('%s-cu-config-dl' % ru_ref) }}]
recipe = slapos.recipe.build:download recipe = slapos.recipe.build:download
url = {{ ru.cu_config_link }} url = {{ ru.cu_config_link }}
version = {{ ru.get("cu_config_version") }} version = {{ ru.get("cu_config_version") }}
offline = false offline = false
{% endif %} {% endif %}
[{{ru_ref}}-cu-config] [{{ B('%s-cu-config' % ru_ref) }}]
<= config-base <= config-base
{% if ru.get("cu_config_link", None) %} {% if ru.get("cu_config_link", None) %}
url = ${ {{-ru_ref}}-cu-config-dl:target} url = ${ {{-B('%s-cu-config-dl' % ru_ref)}}:target}
{% else %} {% else %}
url = {{ ru_lopcomm_cu_config_template }} url = {{ ru_lopcomm_cu_config_template }}
{% endif %} {% endif %}
output = ${directory:etc}/{{ru_ref}}-cu_config.xml output = ${directory:etc}/{{B('%s-cu_config.xml' % ru_ref)}}
extra-context = extra-context =
json ru {{ ru | tojson }} key ru :ru
json cell {{ cell | tojson }} key cell :cell
ru = {{ dumps(ru) }}
cell = {{ dumps(cell) }}
[{{ru_ref}}-config-template] [{{ B('%s-config-template' % ru_ref) }}]
recipe = slapos.recipe.template:jinja2 recipe = slapos.recipe.template:jinja2
extensions = jinja2.ext.do extensions = jinja2.ext.do
log-output = ${directory:var}/log/{{ru_ref}}-config.log log-output = ${directory:var}/log/{{B('%s-config.log' % ru_ref)}}
context = context =
section directory directory section directory directory
section vtap vtap.{{ ru.cpri_link._tap }} section vtap vtap.{{ ru.cpri_link._tap }}
...@@ -88,23 +90,23 @@ context = ...@@ -88,23 +90,23 @@ context =
raw python_path {{ buildout_directory}}/bin/pythonwitheggs raw python_path {{ buildout_directory}}/bin/pythonwitheggs
raw buildout_directory_path {{ buildout_directory }} raw buildout_directory_path {{ buildout_directory }}
raw CreateProcessingEle_template {{ ru_lopcomm_CreateProcessingEle_template }} raw CreateProcessingEle_template {{ ru_lopcomm_CreateProcessingEle_template }}
key cu_config_template {{ru_ref}}-cu-config:output key cu_config_template {{B('%s-cu-config' % ru_ref)}}:output
import netaddr netaddr import netaddr netaddr
mode = 0775 mode = 0775
url = {{ ru_lopcomm_config_template }} url = {{ ru_lopcomm_config_template }}
output = ${directory:script}/{{ru_ref}}-config.py output = ${directory:script}/{{B('%s-config.py' % ru_ref)}}
{{ promise('%s-config-log' % ru_ref) }} {{ promise('%s-config-log' % ru_ref) }}
promise = check_lopcomm_config_log promise = check_lopcomm_config_log
config-config-log = ${ {{- ru_ref}}-config-template:log-output} config-config-log = ${ {{-B('%s-config-template' % ru_ref)}}:log-output}
{#- handle notifications from RU + keep on touching RU watchdog #} {#- handle notifications from RU + keep on touching RU watchdog #}
[{{ru_ref}}-stats-template] [{{ B('%s-stats-template' % ru_ref) }}]
recipe = slapos.recipe.template:jinja2 recipe = slapos.recipe.template:jinja2
extensions = jinja2.ext.do extensions = jinja2.ext.do
_logbase = ${directory:var}/log/{{ru_ref}} _logbase = ${directory:var}/log/{{B('%s' % ru_ref)}}
log-output = ${:_logbase}-stats.log log-output = ${:_logbase}-stats.log
json-log-output = ${:_logbase}-stats.json.log json-log-output = ${:_logbase}-stats.json.log
cfg-json-log-output = ${:_logbase}-config.json.log cfg-json-log-output = ${:_logbase}-config.json.log
...@@ -112,7 +114,7 @@ supervision-json-log-output = ${:_logbase}-supervision.json.log ...@@ -112,7 +114,7 @@ supervision-json-log-output = ${:_logbase}-supervision.json.log
ncsession-json-log-output = ${:_logbase}-ncsession.json.log ncsession-json-log-output = ${:_logbase}-ncsession.json.log
software-json-log-output = ${:_logbase}-software.json.log software-json-log-output = ${:_logbase}-software.json.log
supervision-reply-json-log-output = ${:_logbase}-supervision-reply.json.log supervision-reply-json-log-output = ${:_logbase}-supervision-reply.json.log
is_netconf_connected = ${directory:etc}/{{ru_ref}}.is_netconf_connected is_netconf_connected = ${directory:etc}/{{B('%s.is_netconf_connected' % ru_ref)}}
context = context =
section directory directory section directory directory
section vtap vtap.{{ ru.cpri_link._tap }} section vtap vtap.{{ ru.cpri_link._tap }}
...@@ -131,12 +133,12 @@ context = ...@@ -131,12 +133,12 @@ context =
import netaddr netaddr import netaddr netaddr
mode = 0775 mode = 0775
url = {{ ru_lopcomm_stats_template }} url = {{ ru_lopcomm_stats_template }}
output = ${directory:bin}/{{ru_ref}}-stats.py output = ${directory:bin}/{{B('%s-stats.py' % ru_ref)}}
{{ part('%s-stats-service' % ru_ref) }} {{ part('%s-stats-service' % ru_ref) }}
recipe = slapos.cookbook:wrapper recipe = slapos.cookbook:wrapper
command-line = ${ {{- ru_ref}}-stats-template:output} command-line = ${ {{-B('%s-stats-template' % ru_ref)}}:output}
wrapper-path = ${directory:service}/{{ru_ref}}-stats wrapper-path = ${directory:service}/{{B('%s-stats' % ru_ref)}}
mode = 0775 mode = 0775
hash-files = hash-files =
${:command-line} ${:command-line}
...@@ -144,46 +146,46 @@ hash-files = ...@@ -144,46 +146,46 @@ hash-files =
{%- if not testing %} {%- if not testing %}
{{ promise('%s-netconf-connection' % ru_ref) }} {{ promise('%s-netconf-connection' % ru_ref) }}
promise = check_command_execute promise = check_command_execute
config-command = [ -f ${ {{-ru_ref}}-stats-template:is_netconf_connected} ] config-command = [ -f ${ {{-B('%s-stats-template' % ru_ref)}}:is_netconf_connected} ]
{%- endif %} {%- endif %}
{{ promise('%s-vswr' % ru_ref) }} {{ promise('%s-vswr' % ru_ref) }}
promise = check_lopcomm_vswr promise = check_lopcomm_vswr
config-netconf-log = ${ {{- ru_ref}}-stats-template:json-log-output} config-netconf-log = ${ {{-B('%s-stats-template' % ru_ref)}}:json-log-output}
{{ promise('%s-rssi' % ru_ref) }} {{ promise('%s-rssi' % ru_ref) }}
promise = check_lopcomm_rssi promise = check_lopcomm_rssi
config-netconf-log = ${ {{- ru_ref}}-stats-template:json-log-output} config-netconf-log = ${ {{-B('%s-stats-template' % ru_ref)}}:json-log-output}
{{ promise('%s-pa-current' % ru_ref) }} {{ promise('%s-pa-current' % ru_ref) }}
promise = check_lopcomm_pa_current promise = check_lopcomm_pa_current
config-netconf-log = ${ {{- ru_ref}}-stats-template:json-log-output} config-netconf-log = ${ {{-B('%s-stats-template' % ru_ref)}}:json-log-output}
{{ promise('%s-pa-output-power' % ru_ref) }} {{ promise('%s-pa-output-power' % ru_ref) }}
promise = check_lopcomm_pa_output_power promise = check_lopcomm_pa_output_power
config-netconf-log = ${ {{- ru_ref}}-stats-template:json-log-output} config-netconf-log = ${ {{-B('%s-stats-template' % ru_ref)}}:json-log-output}
{{ promise('%s-sync' % ru_ref) }} {{ promise('%s-sync' % ru_ref) }}
promise = check_lopcomm_sync promise = check_lopcomm_sync
config-netconf-log = ${ {{- ru_ref}}-stats-template:json-log-output} config-netconf-log = ${ {{-B('%s-stats-template' % ru_ref)}}:json-log-output}
{{ promise('%s-lof' % ru_ref) }} {{ promise('%s-lof' % ru_ref) }}
promise = check_lopcomm_lof promise = check_lopcomm_lof
config-netconf-log = ${ {{- ru_ref}}-stats-template:json-log-output} config-netconf-log = ${ {{-B('%s-stats-template' % ru_ref)}}:json-log-output}
{{ promise('%s-stats-log' % ru_ref) }} {{ promise('%s-stats-log' % ru_ref) }}
promise = check_lopcomm_stats_log promise = check_lopcomm_stats_log
config-stats-log = ${ {{- ru_ref}}-stats-template:log-output} config-stats-log = ${ {{-B('%s-stats-template' % ru_ref)}}:log-output}
{#- reset RU periodically #} {#- reset RU periodically #}
{%- if ru.get("reset_schedule") %} {%- if ru.get("reset_schedule") %}
[{{ru_ref}}-reset-info-template] [{{ B('%s-reset-info-template' % ru_ref) }}]
recipe = slapos.recipe.template:jinja2 recipe = slapos.recipe.template:jinja2
extensions = jinja2.ext.do extensions = jinja2.ext.do
_logbase = ${directory:var}/log/{{ru_ref}}-reset-info _logbase = ${directory:var}/log/{{B('%s-reset-info' % ru_ref)}}
log-output = ${:_logbase}.log log-output = ${:_logbase}.log
json-log-output = ${:_logbase}.json.log json-log-output = ${:_logbase}.json.log
context = context =
...@@ -196,12 +198,12 @@ context = ...@@ -196,12 +198,12 @@ context =
import netaddr netaddr import netaddr netaddr
mode = 0775 mode = 0775
url = {{ ru_lopcomm_reset_info_template }} url = {{ ru_lopcomm_reset_info_template }}
output = ${directory:bin}/{{ru_ref}}-reset-info.py output = ${directory:bin}/{{B('%s-reset-info.py' % ru_ref)}}
[{{ru_ref}}-reset-template] [{{ B('%s-reset-template' % ru_ref) }}]
recipe = slapos.recipe.template:jinja2 recipe = slapos.recipe.template:jinja2
extensions = jinja2.ext.do extensions = jinja2.ext.do
_logbase = ${directory:var}/log/{{ru_ref}}-reset _logbase = ${directory:var}/log/{{B('%s-reset' % ru_ref)}}
log-output = ${:_logbase}.log log-output = ${:_logbase}.log
json-log-output = ${:_logbase}.json.log json-log-output = ${:_logbase}.json.log
context = context =
...@@ -212,19 +214,19 @@ context = ...@@ -212,19 +214,19 @@ context =
import netaddr netaddr import netaddr netaddr
mode = 0775 mode = 0775
url = {{ ru_lopcomm_reset_template }} url = {{ ru_lopcomm_reset_template }}
output = ${directory:etc}/{{ru_ref}}-reset.py output = ${directory:etc}/{{B('%s-reset.py' % ru_ref)}}
{{ part('%s-reset-cron' % ru_ref) }} {{ part('%s-reset-cron' % ru_ref) }}
recipe = slapos.cookbook:cron.d recipe = slapos.cookbook:cron.d
cron-entries = ${cron:cron-entries} cron-entries = ${cron:cron-entries}
name = {{ru_ref}}-reset name = {{B('%s-reset' % ru_ref)}}
frequency = {{ ru.reset_schedule }} frequency = {{ ru.reset_schedule }}
command = {{ buildout_directory}}/bin/pythonwitheggs ${ {{- ru_ref}}-reset-template:output} command = {{ buildout_directory}}/bin/pythonwitheggs ${ {{-B('%s-reset-template' % ru_ref)}}:output}
{{ part('%s-reset-info-service' % ru_ref) }} {{ part('%s-reset-info-service' % ru_ref) }}
recipe = slapos.cookbook:wrapper recipe = slapos.cookbook:wrapper
command-line = ${ {{- ru_ref}}-reset-info-template:output} command-line = ${ {{-B('%s-reset-info-template' % ru_ref)}}:output}
wrapper-path = ${directory:service}/{{ru_ref}}-reset-info wrapper-path = ${directory:service}/{{B('%s-reset-info' % ru_ref)}}
mode = 0775 mode = 0775
hash-files = hash-files =
${:command-line} ${:command-line}
......
# Copyright (C) 2023-2024 Nexedi SA and Contributors.
#
# This program is free software: you can Use, Study, Modify and Redistribute
# it under the terms of the GNU General Public License version 3, or (at your
# option) any later version, as published by the Free Software Foundation.
#
# You can also Link and Combine this program with other software covered by
# the terms of any of the Free Software licenses or any of the Open Source
# Initiative approved licenses and Convey the resulting work. Corresponding
# source of such a combination shall include the source code for all other
# software used.
#
# This program is distributed WITHOUT ANY WARRANTY; without even the implied
# warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
#
# See COPYING file for full licensing terms.
# See https://www.nexedi.com/licensing for rationale and options.
"""Package xbuildout provides additional buildout-related utilities.
- encode/decode convert string to/from form, that is suitable to be used in
names of buildout sections.
"""
# encode converts string s into form suitable to be used in names of buildout sections.
#
# Such encoding is needed because buildout forbids to use spaces and many other
# characters in section names, which, in turn, leads to inability to directly
# use arbitrary strings for sections generated based on e.g. instance
# references retrieved from SlapOS Master.
#
# With encoding it becomes possible to use arbitrary names for references
# without leading to instantiation failures like
#
# zc.buildout.configparser.ParsingError: File contains parsing errors: .../instance-enb.cfg
# [line 45]: '[promise-testing partition 0.RU-sdr-busy]\n'
#
# and without being vulnerable to buildout code injection.
#
# The encoding never fails, does not loose information and can be reversed back via decode.
#
# It also leaves all characters allowed by buildout except "_" as is, which
# makes encoding to be identity for 99% of the practical cases in existing
# SlapOS profiles. In other words it is safe to use encode for both generated
# and static buildout sections, without the need to also use encode when
# referring to those static sections.
#
# Recommended usage of encode in buildout profiles is via B as illustrated below:
#
# {#- B(name) escapes name to be safe to use in buildout code. #}
# {%- set B = xbuildout.encode %}
#
# ...
#
# [{{ B('%s-stats' % ru_ref) }}]
# # code for <ru_ref>-stats section
#
# ...
#
# # referring to <ru_ref>-stats
# ${ {{-B('%s-stats' % ru_ref)}}:output}
#
#
# See also `dumps` in buildout for a way to serialize option values with
# protection against code injection:
#
# https://lab.nexedi.com/nexedi/slapos.buildout/commit/4e13dcb9
# https://lab.nexedi.com/nexedi/slapos.recipe.template/commit/84dc7957
def encode(s: str) -> str:
s = s.encode('utf-8')
outv = []
emit = outv.append
for c in s:
c = bytes([c]) # int -> bytechar
# _ serves as escape character
if c == b'_':
emit(b'__')
# other characters allowed by buildout go as is
elif (b'a' <= c <= b'z') or \
(b'A' <= c <= b'Z') or \
(b'0' <= c <= b'9') or \
(c in b'.-'):
emit(c)
# all other bytes go as escaped hex
else:
emit(b'_%02x' % ord(c))
out = b''.join(outv)
out = out.decode('utf-8') # should not fail
return out
# decode provides reverse operation for encode.
def decode(s: str): # -> str | ValueError
try:
return _decode(s)
except Exception as e:
raise ValueError("invalid encoding: %r" % s) from e
def _decode(s):
s = s.encode('utf-8')
outv = []
emit = outv.append
while len(s) > 0:
c = s[:1]
s = s[1:]
if c != b'_':
emit(c)
continue
if len(s) < 1:
raise ValueError("truncated escape sequence")
x = s[:1]
s = s[1:]
if x == b'_':
emit(b'_')
continue
if len(s) < 1:
raise ValueError("truncated escape sequence")
x += s[:1]
s = s[1:]
i = int(x, 16) # raises ValueError if not ok
c = bytes([i])
emit(c)
out = b''.join(outv)
out = out.decode('utf-8') # raises UnicodeDecodeError if it was invalid UTF-8
return out
# ----------------------------------------
import re
def test_encode():
# verify all ascii characters one by one
bok = re.compile(r'[a-zA-Z0-9.-]') # characters that are ok to use in buildout except '_'
for i in range(0x80):
c = chr(i)
e = encode(c)
if bok.match(c):
assert e == c
elif c == '_':
assert e == '__'
else:
assert e == '_%02x' % i
assert decode(e) == c
# also explicitly test several example cases, including unicode
testv = [
# s encoded
('', ''),
('a', 'a'),
('ayzAYZ09.-', 'ayzAYZ09.-'),
('_', '__'),
(' ', '_20'),
('αβγ', '_ce_b1_ce_b2_ce_b3'),
('a b+c_d', 'a_20b_2bc__d'),
]
for (s, encok) in testv:
assert encode(s) == encok
assert decode(encok) == s
# decode errors
from pytest import raises
def checkbad(x, f):
with raises(ValueError, match="invalid encoding") as exci:
decode(x)
cause = exci.value.__cause__
f(cause)
for x in ('_', '_1', 'a_2'):
def _(cause):
assert isinstance(cause, ValueError)
assert cause.args == ("truncated escape sequence",)
checkbad(x, _)
for x in ('_1r', '_r1', 'a_xy'):
def _(cause):
assert isinstance(cause, ValueError)
assert len(cause.args) == 1
assert cause.args[0] .startswith("invalid literal for int() with base 16:")
checkbad(x, _)
for x in ('_c3_28', '_e2_28_a1'):
def _(cause):
assert isinstance(cause, UnicodeDecodeError)
checkbad(x, _)
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
The following utilities are also provided: The following utilities are also provided:
- B escapes string to be safe to use in buildout code.
- J should be used around macro calls to retrieve returned objects. - J should be used around macro calls to retrieve returned objects.
- error reports instantiation error. - error reports instantiation error.
- ierror reports instantiation error caused by shared instance configuration. - ierror reports instantiation error caused by shared instance configuration.
...@@ -74,6 +75,20 @@ ...@@ -74,6 +75,20 @@
%} %}
{#- B(name) escapes name to be safe to use in buildout code.
It escapes buildout control characters in the string so that the result
could be used in buildout code without the risk of buildout profile to
become broken and/or with injected code when handling string input from
outside.
The most often case when B needs to be used is when handling references of
shared instances in generated buildout code.
See xbuildout.encode documentation for details.
#}
{%- set B = xbuildout.encode %}
{#- J is used around macro calls to retrieve returned objects. {#- J is used around macro calls to retrieve returned objects.
It is needed to workaround jinja2 limitation that macro can return only It is needed to workaround jinja2 limitation that macro can return only
...@@ -359,7 +374,7 @@ ...@@ -359,7 +374,7 @@
{%- set rx_gainv = [] %} {#- rx_gain by rx channel #} {%- set rx_gainv = [] %} {#- rx_gain by rx channel #}
{%- for (ru_ref, iru) in iru_dict.items() | sort(attribute="1._._rf_port") %} {%- for (ru_ref, iru) in iru_dict.items() | sort(attribute="1._._rf_port") %}
{%- set ru = iru['_'] %} {%- set ru = iru['_'] %}
// {{ ru_ref }} {{ ru.n_antenna_dl }}T{{ ru.n_antenna_ul }}R ({{ ru.ru_type }}) // {{ B(ru_ref) }} {{ ru.n_antenna_dl }}T{{ ru.n_antenna_ul }}R ({{ ru.ru_type }})
{%- if ru.ru_link_type == 'sdr' %} {%- if ru.ru_link_type == 'sdr' %}
{%- do ru_sdr_dict.update({len(dev_argv): ru}) %} {%- do ru_sdr_dict.update({len(dev_argv): ru}) %}
{%- do dev_argv.append("dev%d=/dev/sdr%d" % (len(dev_argv), ru.sdr_dev)) %} {%- do dev_argv.append("dev%d=/dev/sdr%d" % (len(dev_argv), ru.sdr_dev)) %}
......
...@@ -8,6 +8,11 @@ from slapos.recipe.template import jinja2_template ...@@ -8,6 +8,11 @@ from slapos.recipe.template import jinja2_template
import json, copy, sys, os, pprint, shutil import json, copy, sys, os, pprint, shutil
sys.path.insert(0, './ru')
import xbuildout
B = xbuildout.encode
# j2render renders config/<src> into config/out/<out> with provided json parameters. # j2render renders config/<src> into config/out/<out> with provided json parameters.
def j2render(src, out, jcfg): def j2render(src, out, jcfg):
...@@ -21,6 +26,7 @@ def j2render(src, out, jcfg): ...@@ -21,6 +26,7 @@ def j2render(src, out, jcfg):
textctx = '' textctx = ''
for k, v in ctx.items(): for k, v in ctx.items():
textctx += 'json %s %s\n' % (k, json.dumps(v)) textctx += 'json %s %s\n' % (k, json.dumps(v))
textctx += 'import xbuildout xbuildout\n'
textctx += 'import json_module json\n' textctx += 'import json_module json\n'
buildout = None # stub buildout = None # stub
r = jinja2_template.Recipe(buildout, "recipe", { r = jinja2_template.Recipe(buildout, "recipe", {
...@@ -534,11 +540,11 @@ def _do_enb_with(iru_icell_func): ...@@ -534,11 +540,11 @@ def _do_enb_with(iru_icell_func):
'ru': ru, 'ru': ru,
}) })
j2render('drb_%s.jinja2.cfg' % cell['cell_type'], j2render('drb_%s.jinja2.cfg' % cell['cell_type'],
'%s/%s-drb.cfg' % (out, cell_ref), '%s/%s-drb.cfg' % (out, B(cell_ref)),
jctx) jctx)
j2render('sib23.jinja2.asn', j2render('sib23.jinja2.asn',
'%s/%s-sib23.asn' % (out, cell_ref), '%s/%s-sib23.asn' % (out, B(cell_ref)),
jctx) jctx)
......
...@@ -40,6 +40,8 @@ import pcpp ...@@ -40,6 +40,8 @@ import pcpp
import xmltodict import xmltodict
import sys import sys
sys.path.insert(0, '../ru')
import xbuildout
import unittest import unittest
from slapos.testing.testcase import makeModuleSetUpAndTestCaseClass from slapos.testing.testcase import makeModuleSetUpAndTestCaseClass
...@@ -131,8 +133,11 @@ def XN_PEER(xn_addr): ...@@ -131,8 +133,11 @@ def XN_PEER(xn_addr):
class AmariTestCase(_AmariTestCase): class AmariTestCase(_AmariTestCase):
maxDiff = None # show full diff in test run log on an error maxDiff = None # show full diff in test run log on an error
# XXX temporary workaround for breakage when partition reference contains space. # stress correctness of ru_ref/cell_ref/... usage throughout all places in
default_partition_reference = _AmariTestCase.default_partition_reference.replace(' ','-') # buildout code - special characters should not lead to wrong templates or
# code injection.
default_partition_reference = _AmariTestCase.default_partition_reference + \
' ${a:b}\n[c]\n;'
# faster edit/try cycle when enabled (handy during development) # faster edit/try cycle when enabled (handy during development)
if 0: if 0:
...@@ -432,7 +437,7 @@ class Lopcomm4: ...@@ -432,7 +437,7 @@ class Lopcomm4:
_(4, uctx('TDD', 'NR', 470400, 20000000)) _(4, uctx('TDD', 'NR', 470400, 20000000))
def _test_ru_cu_config_xml(t, i, uctx): def _test_ru_cu_config_xml(t, i, uctx):
cu_xml = t.ipath('etc/%s-cu_config.xml' % t.ref('RU%d' % i)) cu_xml = t.ipath('etc/%s' % xbuildout.encode('%s-cu_config.xml' % t.ref('RU%d' % i)))
with open(cu_xml, 'r') as f: with open(cu_xml, 'r') as f:
cu = f.read() cu = f.read()
cu = xmltodict.parse(cu) cu = xmltodict.parse(cu)
......
...@@ -38,9 +38,6 @@ setUpModule, ORSTestCase = makeModuleSetUpAndTestCaseClass( ...@@ -38,9 +38,6 @@ setUpModule, ORSTestCase = makeModuleSetUpAndTestCaseClass(
os.path.abspath( os.path.abspath(
os.path.join(os.path.dirname(__file__), '..', 'software-ors.cfg'))) os.path.join(os.path.dirname(__file__), '..', 'software-ors.cfg')))
# XXX temporary workaround for breakage when partition reference contains space.
ORSTestCase.default_partition_reference = ORSTestCase.default_partition_reference.replace(' ','-')
param_dict = { param_dict = {
'testing': True, 'testing': True,
'sim_algo': 'milenage', 'sim_algo': 'milenage',
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment