[Git][NTPsec/ntpsec][issue-579] 4 commits: Fix for #580 (not checking length from cookie file)

Matt Selsky gitlab at mg.gitlab.com
Thu Mar 21 06:46:33 UTC 2019 


Matt Selsky pushed to branch issue-579 at NTPsec / ntpsec


Commits:
a99200cc by Hal Murray at 2019-03-20T23:10:27Z
Fix for #580 (not checking length from cookie file)

- - - - -
83830442 by Hal Murray at 2019-03-20T23:11:25Z
Fix for #581 (unnecessary NULL check after deref in handle_procpkt)

- - - - -
9ceda5cc by Hal Murray at 2019-03-20T23:47:15Z
Add nts cookie <filename> config option

- - - - -
87cd0930 by Matt Selsky at 2019-03-21T06:46:20Z
Update Coverity CI build to include all refclocks

Fixes GitLab #579
- - - - -


7 changed files:

- .gitlab-ci.yml
- docs/includes/auth-commands.adoc
- ntpd/keyword-gen.c
- ntpd/ntp_config.c
- ntpd/ntp_parser.y
- ntpd/ntp_proto.c
- ntpd/nts_cookie.c


Changes:

=====================================
.gitlab-ci.yml
=====================================
@@ -578,7 +578,7 @@ gentoo-hardened-refclocks:
 
 coverity-scan:
   script:
-    - ./waf configure
+    - ./waf configure --refclock=all
     - /opt/cov-analysis/bin/cov-build --dir cov-int ./waf build
     - tar czf ntpsec_coverity.tgz cov-int
     - curl --form token=$COVERITY_TOKEN --form email=security at ntpsec.org --form file=@ntpsec_coverity.tgz --form version="$(git rev-parse --short HEAD)" --form description="Automatic submission by gitlab-ci" https://scan.coverity.com/builds?project=ntpsec


=====================================
docs/includes/auth-commands.adoc
=====================================
@@ -51,6 +51,11 @@ The options are as follows:
   validate NTS-KE server certificates instead of the system
   default root certificates.
 
++cookie+ _location_::
+  Use the file (or directory) specified by _location_ to
+  store the keys used to make and decode cookies.  The default
+  is _/var/lib/ntp/nts-keys_.
+
 +enable+::
   Enable NTS-KE server.
   When enabled, _cert_ and _key_ are required.
@@ -103,7 +108,7 @@ The following options of the +server+ command configure NTS.
   That is probably a FQDN rather than a short alias that you would
   probably use to talk to an internal server.
 
-+ask+ _address_::
++ask+ _address_:: (not implemented)
   Use Network Time Security for authentication.  Ask
   for a specific NTS server, which may differ from the NTP server.
   Conforms to RFC 3896 section 3.2.2 prescription for the Host part of
@@ -111,7 +116,7 @@ The following options of the +server+ command configure NTS.
   numeric address, or an IPv6 numeric address (in square brackets).
   The address may have the suffix +:port+ to specify a UDP port.
 
-+require+ _address_::
++require+ _address_:: (not implemented)
   Use Network Time Security for authentication and encryption.
   Require a specific NTS server, which may differ from the NTP server.
   Address syntax is as for +ask+.
@@ -119,15 +124,15 @@ The following options of the +server+ command configure NTS.
 +noval+::
   Do not validate the server certificate.
 
-+expire+::
++expire+:: (not implemented)
   How long to use a secured NTP association before rekeying with the
   NTS-KE server.
 
-+cert+ _file_::
++cert+ _file_:: (not implemented)
   Present the certificate in _file_ as our client certificate,
   overriding the site default.
 
-+ca+ _location_::
++ca+ _location_:: (not implemented)
   Use the file, or directory, specified by _location_ to validate the
   NTS-KE server certificate, overriding the site default.  Do not use
   any other CA.


=====================================
ntpd/keyword-gen.c
=====================================
@@ -28,6 +28,7 @@ struct key_tok ntp_keywords[] = {
 { "bias",		T_Bias,			FOLLBY_TOKEN },
 { "baud",		T_Baud,			FOLLBY_TOKEN },
 { "clock",		T_Clock,		FOLLBY_STRING },
+{ "cookie",		T_Cookie,		FOLLBY_TOKEN },
 { "ctl",		T_Ctl,			FOLLBY_TOKEN },
 { "disable",		T_Disable,		FOLLBY_TOKEN },
 { "driftfile",		T_Driftfile,		FOLLBY_STRING },


=====================================
ntpd/ntp_config.c
=====================================
@@ -2009,6 +2009,10 @@ config_nts(
 			ntsconfig.cert = estrdup(nts->value.s);
 			break;
 
+		case T_Cookie:
+			ntsconfig.KI = estrdup(nts->value.s);
+			break;
+
 		case T_Disable:
 			ntsconfig.ntsenable = false;
 			break;


=====================================
ntpd/ntp_parser.y
=====================================
@@ -68,6 +68,7 @@
 %token	<Integer>	T_Clock
 %token	<Integer>	T_Clockstats
 %token	<Integer>	T_Cohort
+%token	<Integer>	T_Cookie
 %token	<Integer>	T_ControlKey
 %token	<Integer>	T_Ctl
 %token	<Integer>	T_Day
@@ -1138,6 +1139,7 @@ nts_string_option_keyword
 	:	T_Aead
 	|	T_Ca
 	|	T_Cert
+	|	T_Cookie
 	|	T_Key
 	|	T_Tlsciphers
 	|	T_Tlsciphersuites


=====================================
ntpd/ntp_proto.c
=====================================
@@ -419,9 +419,6 @@ handle_procpkt(
 {
 	int outcount = peer->outcount;
 
-	/* Shouldn't happen, but include this for safety. */
-	if(peer == NULL) { return; }
-
 	peer->flash &= ~PKT_BOGON_MASK;
 
 	/* Duplicate detection */


=====================================
ntpd/nts_cookie.c
=====================================
@@ -161,6 +161,8 @@ bool nts_read_cookie_keys(void) {
   if (1 != fscanf(in, "T: %lu\n", &templ)) goto bail;
   K_time = templ;
   if (1 != fscanf(in, "L: %d\n", &K_length)) goto bail;
+  if (NTS_MAX_KEYLEN < K_length) goto bail;
+  // FIXME check K_length is 32, 48, or 64
   if (1 != fscanf(in, "I: %u\n", &I)) goto bail;
   if (0 != fscanf(in, "K: ")) goto bail;
   for (int i=0; i< K_length; i++) {



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/compare/5217cfb26eacde59a0544edca756bc61f57d8947...87cd0930ed50364cbb73d1d3b54908df1c981089

-- 
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/compare/5217cfb26eacde59a0544edca756bc61f57d8947...87cd0930ed50364cbb73d1d3b54908df1c981089
You're receiving this email because of your account on gitlab.com.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/vc/attachments/20190321/bcdab4fc/attachment-0001.html>

More information about the vc mailing list