Skip to content

Commit cd31545

Browse files
kenyonkenyon
committed
(MODULES-10831) key is expired if all subkeys are expired
Previously, subkeys were not considered at all in the determination of whether a key was expired. Now this looks at all of the subkeys, and if they are all expired, considers the whole key expired.
1 parent db8f260 commit cd31545

File tree

2 files changed

+67
-35
lines changed

2 files changed

+67
-35
lines changed

‎lib/puppet/provider/apt_key/apt_key.rb‎

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,50 +8,46 @@
88
end
99
require 'tempfile'
1010

11-
Puppet::Type.type(:apt_key).provide(:apt_key) do
11+
Puppet::Type.type(:apt_key).provide(:apt_key) do # rubocop:disable Metrics/BlockLength
1212
desc 'apt-key provider for apt_key resource'
1313

1414
confine osfamily: :debian
1515
defaultfor osfamily: :debian
1616
commands apt_key: 'apt-key'
1717
commands gpg: '/usr/bin/gpg'
1818

19-
def self.instances
19+
def self.instances # rubocop:disable Metrics/AbcSize
20+
key_array = []
21+
2022
cli_args = ['adv', '--no-tty', '--list-keys', '--with-colons', '--fingerprint', '--fixed-list-mode']
2123

2224
key_output = apt_key(cli_args).encode('UTF-8', 'binary', invalid: :replace, undef: :replace, replace: '')
2325

24-
pub_line, sub_line, fpr_line = nil
25-
26-
key_array = key_output.split("\n").map do |line|
27-
if line.start_with?('pub')
28-
pub_line = line
29-
# reset fpr_line, to skip any previous subkeys which were collected
30-
fpr_line = nil
31-
sub_line = nil
32-
elsif line.start_with?('sub')
33-
sub_line = line
34-
elsif line.start_with?('fpr')
35-
fpr_line = line
36-
end
37-
38-
if sub_line && fpr_line
39-
sub_line, fpr_line = nil
40-
next
26+
pub_line = nil
27+
fpr_lines = []
28+
sub_lines = []
29+
30+
lines = key_output.split("\n")
31+
32+
lines.each_index do |i|
33+
if lines[i].start_with?('pub')
34+
pub_line = lines[i]
35+
# starting a new public key, so reset fpr_lines and sub_lines
36+
fpr_lines = []
37+
sub_lines = []
38+
elsif lines[i].start_with?('fpr')
39+
fpr_lines << lines[i]
40+
elsif lines[i].start_with?('sub')
41+
sub_lines << lines[i]
4142
end
4243

43-
next unless pub_line && fpr_line
44-
45-
line_hash = key_line_hash(pub_line, fpr_line)
46-
47-
# reset everything
48-
pub_line, fpr_line = nil
44+
next unless (pub_line && !fpr_lines.empty?) && (!lines[i + 1] || lines[i + 1].start_with?('pub'))
4945

50-
expired = false
46+
line_hash = key_line_hash(pub_line, fpr_lines)
5147

52-
expired = Time.now >= line_hash[:key_expiry] if line_hash[:key_expiry]
48+
expired = line_hash[:key_expired] || subkeys_all_expired(sub_lines)
5349

54-
new(
50+
key_array << new(
5551
name: line_hash[:key_fingerprint],
5652
id: line_hash[:key_long],
5753
fingerprint: line_hash[:key_fingerprint],
@@ -65,7 +61,7 @@ def self.instances
6561
created: line_hash[:key_created].strftime('%Y-%m-%d'),
6662
)
6763
end
68-
key_array.compact!
64+
key_array
6965
end
7066

7167
def self.prefetch(resources)
@@ -85,9 +81,18 @@ def self.prefetch(resources)
8581
end
8682
end
8783

88-
def self.key_line_hash(pub_line, fpr_line)
84+
def self.subkeys_all_expired(sub_lines)
85+
return false if sub_lines.empty?
86+
87+
sub_lines.each do |line|
88+
return false if line.split(':')[1] == '-'
89+
end
90+
true
91+
end
92+
93+
def self.key_line_hash(pub_line, fpr_lines)
8994
pub_split = pub_line.split(':')
90-
fpr_split = fpr_line.split(':')
95+
fpr_split = fpr_lines.first.split(':')
9196

9297
fingerprint = fpr_split.last
9398
return_hash = {
@@ -97,6 +102,7 @@ def self.key_line_hash(pub_line, fpr_line)
97102
key_size: pub_split[2],
98103
key_type: nil,
99104
key_created: Time.at(pub_split[5].to_i),
105+
key_expired: pub_split[1] == 'e',
100106
key_expiry: pub_split[6].empty? ? nil : Time.at(pub_split[6].to_i)
101107
}
102108

‎spec/unit/puppet/provider/apt_key_spec.rb‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,34 @@
4949
expect(described_class.instances.size).to eq(2)
5050
expect(described_class.instances[0].name).to eq('630239CC130E1A7FD81A27B140976EAF437D05B5')
5151
expect(described_class.instances[0].id).to eq('40976EAF437D05B5')
52+
expect(described_class.instances[0].expired).to be_falsey
5253
expect(described_class.instances[1].name).to eq('C5986B4F1257FFA86632CBA746181433FBB75451')
5354
expect(described_class.instances[1].id).to eq('46181433FBB75451')
55+
expect(described_class.instances[1].expired).to be_falsey
56+
end
57+
end
58+
59+
context 'with self.instances expired subkeys' do
60+
before :each do
61+
command_output = <<~OUTPUT
62+
Executing: /tmp/apt-key-gpghome.0lru3TZOtF/gpg.1.sh --list-keys --with-colons --fingerprint 0x7721F63BD38B4796
63+
tru:t:1:1682141947:0:3:1:5
64+
pub:-:4096:1:7721F63BD38B4796:1460440275:::-:::scSC::::::23::0:
65+
fpr:::::::::EB4C1BFD4F042F6DDDCCEC917721F63BD38B4796:
66+
uid:-::::1460440275::7830FE2652F718E78EEE5881B7FA2CE8E3533BE4::Google Inc. (Linux Packages Signing Authority) <linux-packages-keymaster@google.com>::::::::::0:
67+
sub:e:4096:1:1397BC53640DB551:1460440520:1555048520:::::s::::::23:
68+
fpr:::::::::3B068FB4789ABE4AEFA3BB491397BC53640DB551:
69+
sub:e:4096:1:6494C6D6997C215E:1485225932:1579833932:::::s::::::23:
70+
fpr:::::::::3E50F6D3EC278FDEB655C8CA6494C6D6997C215E:
71+
OUTPUT
72+
allow(described_class).to receive(:apt_key).with(
73+
['adv', '--no-tty', '--list-keys', '--with-colons', '--fingerprint', '--fixed-list-mode'],
74+
).and_return(command_output)
75+
end
76+
77+
it 'returns 1 expired resource' do
78+
expect(described_class.instances.size).to eq(1)
79+
expect(described_class.instances[0].expired).to be_truthy
5480
end
5581
end
5682

@@ -174,7 +200,7 @@
174200

175201
context 'with key_line_hash function' do
176202
it 'matches rsa' do
177-
expect(described_class.key_line_hash('pub:-:1024:1:40976EAF437D05B5:1095016255:::-:::scESC:', 'fpr:::::::::630239CC130E1A7FD81A27B140976EAF437D05B5:')).to include(
203+
expect(described_class.key_line_hash('pub:-:1024:1:40976EAF437D05B5:1095016255:::-:::scESC:', ['fpr:::::::::630239CC130E1A7FD81A27B140976EAF437D05B5:'])).to include(
178204
key_expiry: nil,
179205
key_fingerprint: '630239CC130E1A7FD81A27B140976EAF437D05B5',
180206
key_long: '40976EAF437D05B5',
@@ -185,7 +211,7 @@
185211
end
186212

187213
it 'matches dsa' do
188-
expect(described_class.key_line_hash('pub:-:1024:17:40976EAF437D05B5:1095016255:::-:::scESC:', 'fpr:::::::::630239CC130E1A7FD81A27B140976EAF437D05B5:')).to include(
214+
expect(described_class.key_line_hash('pub:-:1024:17:40976EAF437D05B5:1095016255:::-:::scESC:', ['fpr:::::::::630239CC130E1A7FD81A27B140976EAF437D05B5:'])).to include(
189215
key_expiry: nil,
190216
key_fingerprint: '630239CC130E1A7FD81A27B140976EAF437D05B5',
191217
key_long: '40976EAF437D05B5',
@@ -196,7 +222,7 @@
196222
end
197223

198224
it 'matches ecc' do
199-
expect(described_class.key_line_hash('pub:-:1024:18:40976EAF437D05B5:1095016255:::-:::scESC:', 'fpr:::::::::630239CC130E1A7FD81A27B140976EAF437D05B5:')).to include(
225+
expect(described_class.key_line_hash('pub:-:1024:18:40976EAF437D05B5:1095016255:::-:::scESC:', ['fpr:::::::::630239CC130E1A7FD81A27B140976EAF437D05B5:'])).to include(
200226
key_expiry: nil,
201227
key_fingerprint: '630239CC130E1A7FD81A27B140976EAF437D05B5',
202228
key_long: '40976EAF437D05B5',
@@ -207,7 +233,7 @@
207233
end
208234

209235
it 'matches ecdsa' do
210-
expect(described_class.key_line_hash('pub:-:1024:19:40976EAF437D05B5:1095016255:::-:::scESC:', 'fpr:::::::::630239CC130E1A7FD81A27B140976EAF437D05B5:')).to include(
236+
expect(described_class.key_line_hash('pub:-:1024:19:40976EAF437D05B5:1095016255:::-:::scESC:', ['fpr:::::::::630239CC130E1A7FD81A27B140976EAF437D05B5:'])).to include(
211237
key_expiry: nil,
212238
key_fingerprint: '630239CC130E1A7FD81A27B140976EAF437D05B5',
213239
key_long: '40976EAF437D05B5',

0 commit comments

Comments
 (0)