Merge ~utkarsh/ubuntu/+source/keyutils:apply-default-ttl-to-records-bionic into ubuntu/+source/keyutils:ubuntu/bionic-devel

[Utkarsh Gupta]

Hiya,

This is a minimalistic patch of what we uploaded to Focal and Impish (and also, Jammy).

Below are the MPs of what I had uploaded to Focal and Impish, which are in the SRU queue, so that you can see how much we've stripped down and also verify that the stripping is indeed correct or not.

MP for Focal: https://code.launchpad.net/~utkarsh/ubuntu/+source/keyutils/+git/keyutils/+merge/416132

MP for Impish: https://code.launchpad.net/~utkarsh/ubuntu/+source/keyutils/+git/keyutils/+merge/416129

[Robie Basak (racb)]

This goes a bit further I think. The upstream patch used the TTL from an SRV record or "afsdb" (not sure what that is), and falls back to 5. But your backport will use 5 all the time? Is this intentional?

The use of global variables means I'm not clear when it is reset for a subsequent query, but I assume the upstream code is correct there.

review: Needs Information

[Miriam España Acebal (mirespace)]

Hi Utkarsh,

congratulations on landing this MP!

The code, at first glance, looks correct. I have been able to build the package locally.

What I am struggling with is the verification of the fix. I have not been able to reproduce the good case.

Also, the test plan has an oversight in the ss command: it has to be ``` ss -K dport = :445 ```. I'm not sure if it's a reconnect or something (admittedly, after running that, I still see the file that the samba server is sharing)... but when it says "after a reboot of the client machine to make sure the kernel keys are refreshed "the behaviour is the same.

Maybe I'm missing things in the configuration to check the cases. What I did:

a) Samba server

  vim /etc/netplan10-lxc.yaml <-- duplicate the ens entry with different number to
  have 2 ifaces
  netplan try
  apt install samba
  vim /etc/samba/smb.conf <-- edit path on [sambashare] section
  mkdir <path_on_the_step_before> an create an archive there (a .txt for example)
  create user and register it in samba : adduser <user> ; smbpasswd -a <user>
  vim /etc/hosts <-- put here the 2 ips with a name (sambaserver.example.com),
  comment one line

b) Client

  apt install keyutils
  apt install cifs-utils dnsutils
  mkdir <path_local_share_folder>
  vim /etc/hosts <-- add the no comment line from a)
  nslookup sambaserver
  mount -t cifs -o user=<user> //sambaserver/sambashare <path_local_share_folder>
  ls <path_local_share_folder> <-- check you can see the files
  ss dport = :445

c) Use the commented line for the IP in both server and client

d) Client
  ss -K dport = :445

Maybe the problem for me is using the /etc/hosts, so systemd-resolv is doing nothing ... what do you think? What would the step there instead of using /etc/hosts? At least, I hope I have given you some more detailed steps to reproduce.

review: Needs Information

diff --git a/debian/changelog b/debian/changelog
index 5dfbb4c..ab8a167 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+keyutils (1.5.9-9.2ubuntu2.1) bionic; urgency=medium
+
+  * d/p/apply-default-ttl-to-records.patch: Add patch
+    to apply default TTL to records obtained from
+    getaddrinfo(). (LP: #1962453)
+
+ -- Utkarsh Gupta <[e-mail]>  Tue, 08 Mar 2022 13:26:12 +0530
+
 keyutils (1.5.9-9.2ubuntu2) bionic; urgency=medium
 
   * debian/patches/endianness-and-PIE.patch: fix regexp match against
diff --git a/debian/patches/apply-default-ttl-to-records.patch b/debian/patches/apply-default-ttl-to-records.patch
new file mode 100644
index 0000000..5a049ac
--- /dev/null
+++ b/debian/patches/apply-default-ttl-to-records.patch
@@ -0,0 +1,50 @@
+From 75e7568dc516db698093b33ea273e1b4a30b70be Mon Sep 17 00:00:00 2001
+From: David Howells <[e-mail]>
+Date: Tue, 14 Apr 2020 16:07:26 +0100
+Subject: dns: Apply a default TTL to records obtained from getaddrinfo()
+ Address records obtained from getaddrinfo() don't come with any TTL
+ information, even if they're obtained from the DNS, with the result that
+ key.dns_resolver upcall program doesn't set an expiry time on dns_resolver
+ records unless they include a component obtained directly from the DNS,
+ such as an SRV or AFSDB record.
+ .
+ Fix this to apply a default TTL of 10mins in the event that we haven't got
+ one.  This can be configured in /etc/keyutils/key.dns_resolver.conf by
+ adding the line:
+ .
+   default_ttl = <number-of-seconds>
+ .
+ to the file.
+ .
+ Signed-off-by: David Howells <[e-mail]>
+ Reviewed-by: Ben Boeckel <[e-mail]>
+ Reviewed-by: Jeff Layton <[e-mail]>
+Description: The upstream patch doesn't apply when cherry-picked, as-is,
+ so we've manually stripped down the configuration bits and only taken
+ the hunk which actually addresses the problem, that is, setting the
+ timeout.
+Co-author: Utkarsh Gupta <[e-mail]>
+Origin: upstream, https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/keyutils.git/commit/?id=75e7568dc516db698093b33ea273e1b4a30b70be
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/keyutils/+bug/1962453
+Last-Updated: 2022-02-28
+
+--- a/key.dns_resolver.c
++++ b/key.dns_resolver.c
+@@ -67,6 +67,7 @@
+ static key_serial_t key;
+ static int verbose;
+ static int debug_mode;
++unsigned int key_expiry = 5;
+ 
+ 
+ #define	MAX_VLS			15	/* Max Volume Location Servers Per-Cell */
+@@ -584,6 +585,9 @@
+ 
+ 	/* load the key with data key */
+ 	if (!debug_mode) {
++		ret = keyctl_set_timeout(key, key_expiry);
++		if (ret == -1)
++			error("%s: keyctl_set_timeout: %m", __func__);
+ 		ret = keyctl_instantiate_iov(key, payload, payload_index, 0);
+ 		if (ret == -1)
+ 			error("%s: keyctl_instantiate: %m", __func__);
diff --git a/debian/patches/series b/debian/patches/series
index 6af5e5b..f370345 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -17,3 +17,4 @@ Make-build-reproducible.patch
 Drop-tests-requiring-CONFIG_BIG_KEYS.patch
 Adjust-tests-for-3.18-kernel-change.patch
 endianness-and-PIE.patch
+apply-default-ttl-to-records.patch