Merge ~paelzer/ubuntu/+source/libpam-mount:lp-1804408-LUKS2-support-cosmic into ubuntu/+source/libpam-mount:ubuntu/cosmic-devel

Proposed by Christian Ehrhardt 
Status: Merged
Approved by: Andreas Hasenack
Approved revision: 066ee38328c05cc65754c10632297ebba5d4feee
Merge reported by: Christian Ehrhardt 
Merged at revision: 056f42054d076a11eded92b5e358af388ae6ad2f
Proposed branch: ~paelzer/ubuntu/+source/libpam-mount:lp-1804408-LUKS2-support-cosmic
Merge into: ubuntu/+source/libpam-mount:ubuntu/cosmic-devel
Diff against target: 157 lines (+123/-0)
5 files modified
debian/changelog (+9/-0)
debian/patches/0015-Use-crypt_get_type-to-get-type-and-support-CRYPT_LUK.patch (+49/-0)
debian/patches/series (+1/-0)
debian/tests/control (+3/-0)
debian/tests/local-luks (+61/-0)
Reviewer Review Type Date Requested Status
Andreas Hasenack Approve
Canonical Server Pending
git-ubuntu developers Pending
Review via email: mp+362810@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

PPA available at: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3635

I added autopkgtests as I realized what I wrote for the bug would almost already make a good test.
For Disco I'd not upload the tests as Delta, but instead open them as MP/Bug to Debian to be included there. Only if there is no activity I'd consider adding a Delta.

But lets get a review here first :-)

The PPA worked on my box as outlined in the SRU template.

Tests will run soon (the PPA just rebuilds) and will be available in Bileto then.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I added an dep8 test for some extra confidence
Result example:
Cosmic with fix: http://paste.ubuntu.com/p/nfwBTqBdP9/
Cosmic without: http://paste.ubuntu.com/p/2rzYZJ3RYJ/
Bionic with fix: http://paste.ubuntu.com/p/WwFDY99dNh/

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

For Disco I submitted the test to Debian https://salsa.debian.org/debian/libpam-mount/merge_requests/1

Revision history for this message
Andreas Hasenack (ahasenack) :
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

There is no Debian bug (and there won't be as it is fixed in latest).
I'll remove the line.

be1116b... by Christian Ehrhardt 

there is no Debian bug for lp 1804408, remove Bug-Debian

Signed-off-by: Christian Ehrhardt <email address hidden>

Revision history for this message
Andreas Hasenack (ahasenack) :
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

+1, just some suggestions inside.

I was also going to suggest that the script be made idempotent, that helps a lot when iterating over it with enhancements or changes without having to clean up manually before each new run, but that makes it a bit more complicated than needed at this time (check user before adding, or remove user at the end, add traps, etc).

review: Approve
fed41c5... by Christian Ehrhardt 

d/t/local-luks: also check for an active mount

Signed-off-by: Christian Ehrhardt <email address hidden>

056f420... by Christian Ehrhardt 

changelog: mark dep8 bug as fixed by the new autopkgtest

Signed-off-by: Christian Ehrhardt <email address hidden>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 2797541..3d4a0c5 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,12 @@
6+libpam-mount (2.16-5ubuntu0.1) cosmic; urgency=medium
7+
8+ * d/p/0015-Use-crypt_get_type-to-get-type-and-support-CRYPT_LUK.patch:
9+ allow more than just LUKS1 by detecting the type (LP: #1804408)
10+ * d/tests/control, d/tests/local-luks: add autopkgtest for luks1/luks2
11+ automount on user login. (LP: #1679327)
12+
13+ -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Wed, 06 Feb 2019 09:56:49 +0100
14+
15 libpam-mount (2.16-5) unstable; urgency=medium
16
17 * Upload to unstable.
18diff --git a/debian/patches/0015-Use-crypt_get_type-to-get-type-and-support-CRYPT_LUK.patch b/debian/patches/0015-Use-crypt_get_type-to-get-type-and-support-CRYPT_LUK.patch
19new file mode 100644
20index 0000000..de3095c
21--- /dev/null
22+++ b/debian/patches/0015-Use-crypt_get_type-to-get-type-and-support-CRYPT_LUK.patch
23@@ -0,0 +1,49 @@
24+From: Jochen Sprickerhof <git@jochen.sprickerhof.de>
25+Date: Sun, 30 Sep 2018 18:48:33 +0200
26+Subject: Use crypt_get_type to get type and support CRYPT_LUKS2 as well
27+
28+Origin: debian, https://git.launchpad.net/ubuntu/+source/libpam-mount/plain/debian/patches/0015-Use-crypt_get_type-to-get-type-and-support-CRYPT_LUK.patch
29+
30+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1804408
31+Last-Update: 2019-02-06
32+
33+---
34+ src/crypto-dmc.c | 4 ++--
35+ src/ehd.c | 2 +-
36+ 2 files changed, 3 insertions(+), 3 deletions(-)
37+
38+diff --git a/src/crypto-dmc.c b/src/crypto-dmc.c
39+index d0ab6ca..4754c2d 100644
40+--- a/src/crypto-dmc.c
41++++ b/src/crypto-dmc.c
42+@@ -48,7 +48,7 @@ EXPORT_SYMBOL int ehd_is_luks(const char *path, bool blkdev)
43+
44+ ret = crypt_init(&cd, device);
45+ if (ret == 0) {
46+- ret = crypt_load(cd, CRYPT_LUKS1, NULL);
47++ ret = crypt_load(cd, crypt_get_type(cd), NULL);
48+ if (ret == -EINVAL)
49+ ret = false;
50+ else if (ret == 0)
51+@@ -106,7 +106,7 @@ static bool dmc_run(const struct ehd_mount_request *req,
52+ #endif
53+ }
54+
55+- ret = crypt_load(cd, CRYPT_LUKS1, NULL);
56++ ret = crypt_load(cd, crypt_get_type(cd), NULL);
57+ if (ret == 0) {
58+ ret = crypt_activate_by_passphrase(cd, mt->crypto_name,
59+ CRYPT_ANY_SLOT, req->key_data, req->key_size, flags);
60+diff --git a/src/ehd.c b/src/ehd.c
61+index 07c8099..b550613 100644
62+--- a/src/ehd.c
63++++ b/src/ehd.c
64+@@ -309,7 +309,7 @@ static int ehd_init_volume_luks(struct ehd_mount_request *rq,
65+ lower_dev, strerror(-ret));
66+ goto out;
67+ }
68+- ret = crypt_format(cd, CRYPT_LUKS1, cipher, cipher_mode, NULL, NULL,
69++ ret = crypt_format(cd, crypt_get_type(cd), cipher, cipher_mode, NULL, NULL,
70+ (cont->keybits + CHAR_BIT - 1) / CHAR_BIT, &format_params);
71+ if (ret < 0) {
72+ fprintf(stderr, "crypt_format: %s\n", strerror(-ret));
73diff --git a/debian/patches/series b/debian/patches/series
74index 22fcc9d..8c1dfdc 100644
75--- a/debian/patches/series
76+++ b/debian/patches/series
77@@ -12,3 +12,4 @@
78 013-fix-doc-target
79 014-allow-luserconf-outside-home
80 0014-Don-t-compare-source-when-checking-if-it-s-already-m.patch
81+0015-Use-crypt_get_type-to-get-type-and-support-CRYPT_LUK.patch
82diff --git a/debian/tests/control b/debian/tests/control
83new file mode 100644
84index 0000000..7a02d06
85--- /dev/null
86+++ b/debian/tests/control
87@@ -0,0 +1,3 @@
88+Tests: local-luks
89+Depends: @, cryptsetup, openssl, openssh-client, openssh-server
90+Restrictions: allow-stderr, needs-root, isolation-machine
91diff --git a/debian/tests/local-luks b/debian/tests/local-luks
92new file mode 100644
93index 0000000..9471fd8
94--- /dev/null
95+++ b/debian/tests/local-luks
96@@ -0,0 +1,61 @@
97+#!/bin/bash
98+set -uxe
99+mkdir /etc/pam_mount_keys/
100+echo -n alice > /etc/pam_mount_keys/alice.key
101+echo "libpam-mount is working" > testfile
102+
103+createluks()
104+{
105+ local id=${1}
106+ local type=${2}
107+ local tdir=$(mktemp --directory)
108+
109+ fallocate -l 100M "/tmp/disk-l${id}.img"
110+ cryptsetup luksFormat --batch-mode --verbose --force-password --key-file=/etc/pam_mount_keys/alice.key --type "${type}" "/tmp/disk-l${id}.img"
111+ cryptsetup open --type "${type}" --batch-mode --verbose --key-file=/etc/pam_mount_keys/alice.key "/tmp/disk-l${id}.img" "img-luks${id}"
112+ mkfs.ext4 -L "IMG-LUKS${id}" "/dev/mapper/img-luks${id}"
113+ mount "/dev/mapper/img-luks${id}" "${tdir}"
114+ cp testfile "${tdir}"
115+ umount "/dev/mapper/img-luks${id}"
116+ cryptsetup close "img-luks${id}"
117+}
118+
119+checkluks()
120+{
121+ local id=${1}
122+ local bckp=$(mktemp)
123+
124+ # backup and modify configuration
125+ cp /etc/security/pam_mount.conf.xml "${bckp}"
126+ sed -i -e 's/debug enable="0"/debug enable="1"/' /etc/security/pam_mount.conf.xml
127+ sed -i -e "/<!-- Volume definitions -->/a<volume user='alice' path='/tmp/disk-l${id}.img' mountpoint='~/img-luks${id}' fstype='crypt' fskeycipher='none' fskeyhash='md5' fskeypath='/etc/pam_mount_keys/alice.key' />" /etc/security/pam_mount.conf.xml
128+
129+ # smoke tests the mount and if not working print debug from journal
130+ ssh -o "StrictHostKeyChecking=no" -i test.key alice@localhost "ls -laF ~/img-luks${id}/testfile" || journalctl -xe -u ssh --no-pager
131+
132+ # make sure we are using an actual mount
133+ ssh -o "StrictHostKeyChecking=no" -i test.key alice@localhost "mount" | grep img-luks${id}
134+
135+ # compare data on encrypted disk
136+ ssh -o "StrictHostKeyChecking=no" -i test.key alice@localhost "cat ~/img-luks${id}/testfile" > "luks${id}"
137+ cmp testfile "luks${id}"
138+
139+ # restore configuration
140+ cp "${bckp}" /etc/security/pam_mount.conf.xml
141+}
142+
143+# create user alice with PW alice
144+sudo useradd -G users -m --password "$(openssl passwd -1 alice)" -s /bin/bash alice
145+ssh-keygen -t rsa -N "" -f test.key
146+mkdir -p ~alice/.ssh
147+chown alice:alice ~alice/.ssh
148+cp test.key.pub ~alice/.ssh/authorized_keys
149+mkdir -p /home/alice/img-luks1 /home/alice/img-luks2
150+chown alice:alice ~alice/.ssh/authorized_keys /home/alice/img-luks1 /home/alice/img-luks2
151+
152+# create LUKS devices with known content
153+createluks 1 luks
154+createluks 2 luks2
155+
156+checkluks 1
157+checkluks 2

Subscribers

People subscribed via source and target branches