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

Simplify defaults #480

Merged
merged 29 commits into from
Jun 25, 2024
Merged

Simplify defaults #480

merged 29 commits into from
Jun 25, 2024

Conversation

troglobit
Copy link
Contributor

@troglobit troglobit commented Jun 16, 2024

Description

This PR primarily adds support for:

Secondary objectives:

Checklist

Tick relevant boxes, this PR is-a or has-a:

  • Bugfix
    • Regression tests
    • ChangeLog updates (for next release)
  • Feature
    • YANG model change => revision updated?
    • Regression tests added?
    • ChangeLog updates (for next release)
    • Documentation added?
  • Code style update (formatting, renaming)
  • Refactoring (please detail in commit messages)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

@troglobit troglobit requested review from wkz and mattiaswal June 16, 2024 23:07
@troglobit troglobit marked this pull request as draft June 17, 2024 07:42
Copy link
Contributor

@wkz wkz left a comment

Choose a reason for hiding this comment

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

I envy your productivity, great job! 🥇

src/confd/src/ietf-system.c Outdated Show resolved Hide resolved
src/confd/src/ietf-system.c Outdated Show resolved Hide resolved
src/confd/src/ietf-system.c Outdated Show resolved Hide resolved
src/confd/src/ietf-system.c Show resolved Hide resolved
test/case/ietf_system/user_admin.py Outdated Show resolved Hide resolved
test/case/ietf_system/hostname.py Outdated Show resolved Hide resolved
@troglobit troglobit force-pushed the simplify-defaults branch 2 times, most recently from aae5466 to 17c963c Compare June 20, 2024 05:35
This patch bumps klish-plugins-sysrepo to fix the annoying issue with
the CLI not supporting, e.g. "show hostname" in system config context.

Details: kernelkit/klish-plugin-sysrepo#3

The problem was two-fold, first we used the wrong ptype for 'show'
(and 'diff') commands, so srp_show() was not even called.  Then the
klish-plugin-sysrepo code did not account for show being called for
leaf nodes.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
This patch simplifies the handling of factory default password for the
admin user by overloading the ietf-system password type.  The new type,
$factory$, acts a system hint to use any device specific (or built-in
software/device-tree) default password hash.

Fixes #435

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Fixes #447

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
With this patch, users not in the NACM admin group and a login shell set
to clish can now log in to the CLI.  Prior to this change only members
of the UNIX group 'wheel' could open the klishd socket.

The patch adds another group, 'sys-cli', which acts as a capability for
users.  An admin user (member of the admin group) will now be member of
both the UNIX 'wheel' and 'sys-cli' groups.  A non-admin user that has
shell set to 'clish' will only be member of the 'sys-cli' group.  Other
users will not be mapped to any UNIX group and have only permissions set
in NACM.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
The following specifiers are currently supported:

 - %h default hostname from /etc/os-release
 - %i value of ID from /etc/os-release
 - %m last three octets of base MAC, e.g., c0-ff-ee

Fixes #435

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
For consistency with UNKNOWN base mac address, and making it easier for
the user to actually notice the fault condition.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
This patch silences the previously very verbose setup.sh script, and
crucially also fixes a bug in UPDATE_MODULE().  Missing closing '

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Fixes #478

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit troglobit marked this pull request as ready for review June 20, 2024 06:16
Copy link
Contributor

@mattiaswal mattiaswal left a comment

Choose a reason for hiding this comment

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

Massive work, 👍 some small things.

board/common/rootfs/libexec/infix/init.d/30-cfg-migrate Outdated Show resolved Hide resolved
utils/sysrepo-load-modules.sh Show resolved Hide resolved
All typedefs and identities should be declared in the module that uses
them unless other modules need them.  This change also makes it easier
to work with tools like yanglint and pyang.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Add metadata 'version' field to ietf-system to be able to detect .cfg
file version.  Fixes #308

Add early boot script to automatically migrate configuration files of
older version to new syntax.  Fixes #178

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Allow tests to fetch expected factory default password of devices.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit
Copy link
Contributor Author

There, final comments and regressions fixed! Tests pass locally.

@troglobit
Copy link
Contributor Author

troglobit commented Jun 20, 2024

Aaand now tests also pass on the server. Best solstice giftoffering ever! 🌞 🥳

Copy link
Contributor

@mattiaswal mattiaswal left a comment

Choose a reason for hiding this comment

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

LGTM Great work 👍

@troglobit troglobit added this to the Infix v24.06 milestone Jun 24, 2024
Copy link
Contributor

@wkz wkz left a comment

Choose a reason for hiding this comment

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

Awesome work! Sorry about the review lag.

src/confd/README.md Show resolved Hide resolved
src/confd/src/ietf-keystore.c Outdated Show resolved Hide resolved
src/confd/src/ietf-keystore.c Show resolved Hide resolved
This patch adds support for an empty "genkey" pair in the keystore for
the NETCONF hostkeys.  Primarily intended for static factory-config.

When a configuration is loaded and confd detects a missing public or
private key in the "genkey" asymmetric key, it loads generated keys
from disk and store in the running datastore.

Fixes #435

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Drop annoying escape characters from ixinit when logging to syslog.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
With the recent changes to confd to support: hostname format specifiers,
$factory$ default keyword for password, and on-the-fly generation of the
NETCONF SSH host keys, the system no longer depend on the very intricate
confd gen-* scripts to create factory-config and failure-config.

This patch set adds support for detecting and installing product/vendor
specific static factory-config and failure-config files.  See the confd
README for details.

To facilitate generation, e.g., of the NETCONF SSH host keys, the confd
daemon must be running when bootstrapping the first startup-config from
eithe generated or static factory-config.  This is why the bootstrap and
load helper scripts have been changed.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Finalize support for IETF System YANG incl. all IANA crypt-hash types.

This patch builds on the earlier work adding yescrypt and $factory$ key
word.  The YANG description for the crypt-hash type override has been
significantly udpated to discourage use of $0$cleartext passwords.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
First, actually remove setup.sh.  No reason to have the old version
lingering in the repo confusing devs.  Also simplify script heading
dropping previous netopeer2 text and distilling the comment before
sourcing the .inc file.

Silence the install/update, dropping -v -- no need to be overly verbose
now that we now the new yang loader works at build time.

Crucially -- fix a bug in UPDATE_MODULE(), missing closing '

Simplify naming and location of .inc files.  No need for the long
filenames, or the new directory, the directory name gives plenty of
context.

Add reminder of duplicate infix-interfaces.yang in .inc files -- this
duplication is unfortunate and we should try to fix this better.  We
will forget to update one or the other any day ...

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
We recently dropped Augas from confd and this branch adds support for
$0$cleartext passwords, which require libxcrypt.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
Ubuntu 22.04 (ubuntu-latest runner on GitHub) has a slightly earlier
version of libxcrypt.  The only hard requirement we have is yescrypt
support, so we can relax the configure check slightly to be able to
run coverity scan.

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@troglobit troglobit merged commit 3e045e6 into main Jun 25, 2024
4 checks passed
@troglobit troglobit deleted the simplify-defaults branch June 25, 2024 15:22
@troglobit troglobit mentioned this pull request Jun 25, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants