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

Jump to bottom

chrony: 3.5.1 -> 4.0 #102185

Merged
merged 1 commit into from Nov 1, 2020
Merged

chrony: 3.5.1 -> 4.0 #102185

merged 1 commit into from Nov 1, 2020

Conversation

bbigras
Copy link
Contributor

@bbigras bbigras commented Oct 30, 2020

Motivation for this change

Add support for Network Time Security (NTS) authentication

https://chrony.tuxfamily.org/news.html

Things done

I have no idea if the patches are still needed.

I didn't test the module yet. I'll do soon.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

/nix/store/rdslg5cpzffdv8jrg79ylk8pzzfj3k08-chrony-3.5.1 47.8M
/nix/store/17jzdhmz5rxxw7rbf820l13cqrr901lk-chrony-4.0 44.1M

@bbigras
Copy link
Contributor Author

bbigras commented Oct 30, 2020

@fpletz

@ofborg ofborg bot requested a review from fpletz October 30, 2020 21:11
@bbigras
Copy link
Contributor Author

bbigras commented Oct 30, 2020

For NTS, gnutls (or maybe nettle) is required.

from the configure file:

if [ $feat_ntp = "1" ] && [ $feat_nts = "1" ] && [ $try_gnutls = "1" ]; then
  test_cflags="`pkg_config --cflags gnutls`"
  test_link="`pkg_config --libs gnutls`"
  if test_code 'gnutls' 'gnutls/gnutls.h' \
    "$test_cflags" "$test_link" '
      return gnutls_init(NULL, 0) + GNUTLS_TLS1_3 +
        gnutls_priority_init2(NULL, "", NULL, GNUTLS_PRIORITY_INIT_DEF_APPEND) +
        gnutls_prf_rfc5705(NULL, 0, "", 0, "", 16, NULL);'
  then
    if test_code 'SIV in nettle' \
      'nettle/siv-cmac.h' "" "$LIBS" \
      'siv_cmac_aes128_set_key(NULL, NULL);'
    then
      EXTRA_OBJECTS="$EXTRA_OBJECTS siv_nettle.o"
      add_def HAVE_SIV
      add_def HAVE_NETTLE_SIV_CMAC
    else
      if test_code 'SIV in gnutls' 'gnutls/gnutls.h' \
        "$test_cflags" "$test_link" '
          return gnutls_aead_cipher_init(NULL, GNUTLS_CIPHER_AES_128_SIV, NULL);'
      then
        EXTRA_OBJECTS="$EXTRA_OBJECTS siv_gnutls.o"
        add_def HAVE_SIV
      else
        if test_code 'AES128 in nettle' 'nettle/aes.h' '' "$LIBS" \
          'aes128_set_encrypt_key(NULL, NULL);'
        then
          EXTRA_OBJECTS="$EXTRA_OBJECTS siv_nettle.o"
          add_def HAVE_SIV
        fi
      fi
    fi

    if grep '#define HAVE_SIV' config.h > /dev/null; then
      EXTRA_OBJECTS="$EXTRA_OBJECTS nts_ke_client.o nts_ke_server.o nts_ke_session.o"
      EXTRA_OBJECTS="$EXTRA_OBJECTS nts_ntp_auth.o nts_ntp_client.o nts_ntp_server.o"
      LIBS="$LIBS $test_link"
      MYCPPFLAGS="$MYCPPFLAGS $test_cflags"
      add_def FEAT_NTS
    fi
  fi
fi

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 102185 run on x86_64-linux 1

1 package built:
  • chrony

@bbigras
Copy link
Contributor Author

bbigras commented Oct 31, 2020

chrony seems to work without both patches.

The module will need to be updated for the nts keyword. I might do a PR later.

This config file seems to work but I don't see saved key files in /var/lib/chrony/nts.

server time.cloudflare.com iburst nts

initstepslew 1000 time.cloudflare.com

driftfile /var/lib/chrony/chrony.drift
keyfile /var/lib/chrony/chrony.keys

ntsdumpdir /var/lib/chrony/nts

Two helpful commands:

  • sudo chronyc -N sources
  • sudo chronyc -N authdata

and a blog post https://fedoramagazine.org/secure-ntp-with-nts/

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/352

@thoughtpolice
Copy link
Member

Two notes:

The fix for seccomp's clock_adjtime fix is definitively in 4.0; the patch was extracted from upstream and you can see it as part of the tag (ref mlichvar/chrony@0cf506c)

Second, as for the patch to fix the build: it's a little more complex. Please see 5d8bd74 and the comments on that commit; in short, it was originally added to fix a build failure introduced by a libseccomp update (undefined ref to __SNR_ppoll). That turned out to be a bug in libseccomp, which was later fixed (seccomp/libseccomp@e3647f5), as part libseccomp 2.5.0, which we use today in nixpkgs. Therefore this patch is also redundant as well.

@thoughtpolice thoughtpolice merged commit 4e57249 into NixOS:master Nov 1, 2020
@bbigras bbigras deleted the chrony branch November 1, 2020 00:33
@bbigras
Copy link
Contributor Author

bbigras commented Nov 1, 2020

Thank you very much everyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thoughtpolice thoughtpolice thoughtpolice approved these changes

@fpletz fpletz Awaiting requested review from fpletz

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@bbigras @SuperSandro2000 @nixos-discourse @thoughtpolice