Browse Source

Fix entropy problems with OS::Random::String

When generating a random string, once we had selected from the various
required pools, we continued by selecting a pool at random and then
selecting a character from that pool at random. This did not take into
account the differing sizes of the available pools, nor the fact that the
same character could appear in multiple pools, which resulted in a
non-uniform probability distribution of characters. Since users mostly make
use of this feature to generate default passwords for services they are
deploying, this would result in the generated passwords having slightly
less entropy than expected (and pathological cases were possible).

Rectify this by always selecting non-constrained characters from a single
combined pool, and by ensuring that each character appears only once in any
pool we're selecting from.

Since we also want to use this method to generate passwords for OpenStack
Users, the new implementation is in a separate module in heat.common rather
than mixed in with the resource's logic. Also, use a StringIO object to
collect the characters rather than repeatedly appending to a string.

Change-Id: Ia7b63e72c1e3c0649290caf4fea8a32f7f89560b
Closes-Bug: #1757300
Related-Bug: #1666129
Related-Bug: #1444429
(cherry picked from commit 6e16c051ba)
tags/10.0.1
Zane Bitter 1 year ago
parent
commit
8437ec3fce

+ 109
- 0
heat/common/password_gen.py View File

@@ -0,0 +1,109 @@
1
+#
2
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
3
+#    not use this file except in compliance with the License. You may obtain
4
+#    a copy of the License at
5
+#
6
+#         http://www.apache.org/licenses/LICENSE-2.0
7
+#
8
+#    Unless required by applicable law or agreed to in writing, software
9
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
10
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
11
+#    License for the specific language governing permissions and limitations
12
+#    under the License.
13
+
14
+import collections
15
+import random as random_module
16
+import string
17
+
18
+import six
19
+
20
+
21
+# NOTE(pas-ha) Heat officially supports only POSIX::Linux platform
22
+# where os.urandom() and random.SystemRandom() are available
23
+random = random_module.SystemRandom()
24
+
25
+
26
+CHARACTER_CLASSES = (
27
+    LETTERS_DIGITS, LETTERS, LOWERCASE, UPPERCASE,
28
+    DIGITS, HEXDIGITS, OCTDIGITS,
29
+) = (
30
+    'lettersdigits', 'letters', 'lowercase', 'uppercase',
31
+    'digits', 'hexdigits', 'octdigits',
32
+)
33
+
34
+_char_class_members = {
35
+    LETTERS_DIGITS: string.ascii_letters + string.digits,
36
+    LETTERS: string.ascii_letters,
37
+    LOWERCASE: string.ascii_lowercase,
38
+    UPPERCASE: string.ascii_uppercase,
39
+    DIGITS: string.digits,
40
+    HEXDIGITS: string.digits + 'ABCDEF',
41
+    OCTDIGITS: string.octdigits,
42
+}
43
+
44
+
45
+CharClass = collections.namedtuple('CharClass',
46
+                                   ('allowed_chars', 'min_count'))
47
+
48
+
49
+def named_char_class(char_class, min_count=0):
50
+    """Return a predefined character class.
51
+
52
+    The result of this function can be passed to :func:`generate_password` as
53
+    one of the character classes to use in generating a password.
54
+
55
+    :param char_class: Any of the character classes named in
56
+                       :const:`CHARACTER_CLASSES`
57
+    :param min_count: The minimum number of members of this class to appear in
58
+                      a generated password
59
+    """
60
+    assert char_class in CHARACTER_CLASSES
61
+    return CharClass(frozenset(_char_class_members[char_class]), min_count)
62
+
63
+
64
+def special_char_class(allowed_chars, min_count=0):
65
+    """Return a character class containing custom characters.
66
+
67
+    The result of this function can be passed to :func:`generate_password` as
68
+    one of the character classes to use in generating a password.
69
+
70
+    :param allowed_chars: Iterable of the characters in the character class
71
+    :param min_count: The minimum number of members of this class to appear in
72
+                      a generated password
73
+    """
74
+    return CharClass(frozenset(allowed_chars), min_count)
75
+
76
+
77
+def generate_password(length, char_classes):
78
+    """Generate a random password.
79
+
80
+    The password will be of the specified length, and comprised of characters
81
+    from the specified character classes, which can be generated using the
82
+    :func:`named_char_class` and :func:`special_char_class` functions. Where
83
+    a minimum count is specified in the character class, at least that number
84
+    of characters in the resulting password are guaranteed to be from that
85
+    character class.
86
+
87
+    :param length: The length of the password to generate, in characters
88
+    :param char_classes: Iterable over classes of characters from which to
89
+                         generate a password
90
+    """
91
+    char_buffer = six.StringIO()
92
+    all_allowed_chars = set()
93
+
94
+    # Add the minimum number of chars from each char class
95
+    for char_class in char_classes:
96
+        all_allowed_chars |= char_class.allowed_chars
97
+        allowed_chars = tuple(char_class.allowed_chars)
98
+        for i in six.moves.xrange(char_class.min_count):
99
+            char_buffer.write(random.choice(allowed_chars))
100
+
101
+    # Fill up rest with random chars from provided classes
102
+    combined_chars = tuple(all_allowed_chars)
103
+    for i in six.moves.xrange(max(0, length - char_buffer.tell())):
104
+        char_buffer.write(random.choice(combined_chars))
105
+
106
+    # Shuffle string
107
+    selected_chars = char_buffer.getvalue()
108
+    char_buffer.close()
109
+    return ''.join(random.sample(selected_chars, length))

+ 20
- 79
heat/engine/resources/openstack/heat/random_string.py View File

@@ -11,13 +11,11 @@
11 11
 #    License for the specific language governing permissions and limitations
12 12
 #    under the License.
13 13
 
14
-import random as random_module
15
-import string
16
-
17 14
 import six
18 15
 
19 16
 from heat.common import exception
20 17
 from heat.common.i18n import _
18
+from heat.common import password_gen
21 19
 from heat.engine import attributes
22 20
 from heat.engine import constraints
23 21
 from heat.engine import properties
@@ -25,10 +23,6 @@ from heat.engine import resource
25 23
 from heat.engine import support
26 24
 from heat.engine import translation
27 25
 
28
-# NOTE(pas-ha) Heat officially supports only POSIX::Linux platform
29
-# where os.urandom() and random.SystemRandom() are available
30
-random = random_module.SystemRandom()
31
-
32 26
 
33 27
 class RandomString(resource.Resource):
34 28
     """A resource which generates a random string.
@@ -83,10 +77,7 @@ class RandomString(resource.Resource):
83 77
             properties.Schema.STRING,
84 78
             _('Sequence of characters to build the random string from.'),
85 79
             constraints=[
86
-                constraints.AllowedValues(['lettersdigits', 'letters',
87
-                                           'lowercase', 'uppercase',
88
-                                           'digits', 'hexdigits',
89
-                                           'octdigits']),
80
+                constraints.AllowedValues(password_gen.CHARACTER_CLASSES),
90 81
             ],
91 82
             support_status=support.SupportStatus(
92 83
                 status=support.HIDDEN,
@@ -112,11 +103,9 @@ class RandomString(resource.Resource):
112 103
                          % {'min': CHARACTER_CLASSES_MIN}),
113 104
                         constraints=[
114 105
                             constraints.AllowedValues(
115
-                                ['lettersdigits', 'letters', 'lowercase',
116
-                                 'uppercase', 'digits', 'hexdigits',
117
-                                 'octdigits']),
106
+                                password_gen.CHARACTER_CLASSES),
118 107
                         ],
119
-                        default='lettersdigits'),
108
+                        default=password_gen.LETTERS_DIGITS),
120 109
                     CHARACTER_CLASSES_MIN: properties.Schema(
121 110
                         properties.Schema.INTEGER,
122 111
                         _('The minimum number of characters from this '
@@ -130,7 +119,7 @@ class RandomString(resource.Resource):
130 119
                 }
131 120
             ),
132 121
             # add defaults for backward compatibility
133
-            default=[{CHARACTER_CLASSES_CLASS: 'lettersdigits',
122
+            default=[{CHARACTER_CLASSES_CLASS: password_gen.LETTERS_DIGITS,
134 123
                       CHARACTER_CLASSES_MIN: 1}]
135 124
 
136 125
         ),
@@ -177,16 +166,6 @@ class RandomString(resource.Resource):
177 166
         ),
178 167
     }
179 168
 
180
-    _sequences = {
181
-        'lettersdigits': string.ascii_letters + string.digits,
182
-        'letters': string.ascii_letters,
183
-        'lowercase': string.ascii_lowercase,
184
-        'uppercase': string.ascii_uppercase,
185
-        'digits': string.digits,
186
-        'hexdigits': string.digits + 'ABCDEF',
187
-        'octdigits': string.octdigits
188
-    }
189
-
190 169
     def translation_rules(self, props):
191 170
         if props.get(self.SEQUENCE):
192 171
             return [
@@ -205,57 +184,19 @@ class RandomString(resource.Resource):
205 184
             ]
206 185
 
207 186
     def _generate_random_string(self, char_sequences, char_classes, length):
208
-        random_string = ""
209
-
210
-        # Add the minimum number of chars from each char sequence & char class
211
-        if char_sequences:
212
-            for char_seq in char_sequences:
213
-                seq = char_seq[self.CHARACTER_SEQUENCES_SEQUENCE]
214
-                seq_min = char_seq[self.CHARACTER_SEQUENCES_MIN]
215
-                for i in six.moves.xrange(seq_min):
216
-                    random_string += random.choice(seq)
217
-
218
-        if char_classes:
219
-            for char_class in char_classes:
220
-                cclass_class = char_class[self.CHARACTER_CLASSES_CLASS]
221
-                cclass_seq = self._sequences[cclass_class]
222
-                cclass_min = char_class[self.CHARACTER_CLASSES_MIN]
223
-                for i in six.moves.xrange(cclass_min):
224
-                    random_string += random.choice(cclass_seq)
225
-
226
-        def random_class_char():
227
-            cclass_dict = random.choice(char_classes)
228
-            cclass_class = cclass_dict[self.CHARACTER_CLASSES_CLASS]
229
-            cclass_seq = self._sequences[cclass_class]
230
-            return random.choice(cclass_seq)
231
-
232
-        def random_seq_char():
233
-            seq_dict = random.choice(char_sequences)
234
-            seq = seq_dict[self.CHARACTER_SEQUENCES_SEQUENCE]
235
-            return random.choice(seq)
236
-
237
-        # Fill up rest with random chars from provided sequences & classes
238
-        if char_sequences and char_classes:
239
-            weighted_choices = ([True] * len(char_classes) +
240
-                                [False] * len(char_sequences))
241
-            while len(random_string) < length:
242
-                if random.choice(weighted_choices):
243
-                    random_string += random_class_char()
244
-                else:
245
-                    random_string += random_seq_char()
246
-
247
-        elif char_sequences:
248
-            while len(random_string) < length:
249
-                random_string += random_seq_char()
250
-
251
-        else:
252
-            while len(random_string) < length:
253
-                random_string += random_class_char()
254
-
255
-        # Randomize string
256
-        random_string = ''.join(random.sample(random_string,
257
-                                              len(random_string)))
258
-        return random_string
187
+        seq_mins = [
188
+            password_gen.special_char_class(
189
+                char_seq[self.CHARACTER_SEQUENCES_SEQUENCE],
190
+                char_seq[self.CHARACTER_SEQUENCES_MIN])
191
+            for char_seq in char_sequences]
192
+        char_class_mins = [
193
+            password_gen.named_char_class(
194
+                char_class[self.CHARACTER_CLASSES_CLASS],
195
+                char_class[self.CHARACTER_CLASSES_MIN])
196
+            for char_class in char_classes]
197
+
198
+        return password_gen.generate_password(length,
199
+                                              seq_mins + char_class_mins)
259 200
 
260 201
     def validate(self):
261 202
         super(RandomString, self).validate()
@@ -276,8 +217,8 @@ class RandomString(resource.Resource):
276 217
             raise exception.StackValidationFailed(message=msg)
277 218
 
278 219
     def handle_create(self):
279
-        char_sequences = self.properties[self.CHARACTER_SEQUENCES]
280
-        char_classes = self.properties[self.CHARACTER_CLASSES]
220
+        char_sequences = self.properties[self.CHARACTER_SEQUENCES] or []
221
+        char_classes = self.properties[self.CHARACTER_CLASSES] or []
281 222
         length = self.properties[self.LENGTH]
282 223
 
283 224
         random_string = self._generate_random_string(char_sequences,

+ 119
- 2
heat/tests/openstack/heat/test_random_string.py View File

@@ -259,7 +259,7 @@ Resources:
259 259
         return stack
260 260
 
261 261
     # test was saved to test backward compatibility with old behavior
262
-    def test_generate_random_string_backward_compatiable(self):
262
+    def test_generate_random_string_backward_compatible(self):
263 263
         stack = self.parse_stack(template_format.parse(self.template_rs))
264 264
         secret = stack['secret']
265 265
         char_classes = secret.properties['character_classes']
@@ -268,8 +268,125 @@ Resources:
268 268
         # run each test multiple times to confirm random generator
269 269
         # doesn't generate a matching pattern by chance
270 270
         for i in range(1, 32):
271
-            r = secret._generate_random_string(None, char_classes, self.length)
271
+            r = secret._generate_random_string([], char_classes, self.length)
272 272
 
273 273
             self.assertThat(r, matchers.HasLength(self.length))
274 274
             regex = '%s{%s}' % (self.pattern, self.length)
275 275
             self.assertThat(r, matchers.MatchesRegex(regex))
276
+
277
+
278
+class TestGenerateRandomStringDistribution(common.HeatTestCase):
279
+    def run_test(self, tmpl, iterations=5):
280
+        stack = utils.parse_stack(template_format.parse(tmpl))
281
+        secret = stack['secret']
282
+        secret.data_set = mock.Mock()
283
+
284
+        for i in range(iterations):
285
+            secret.handle_create()
286
+
287
+        return [call[1][1] for call in secret.data_set.mock_calls]
288
+
289
+    def char_counts(self, random_strings, char):
290
+        return [rs.count(char) for rs in random_strings]
291
+
292
+    def check_stats(self, char_counts, expected_mean, allowed_variance,
293
+                    expected_minimum=0):
294
+        mean = float(sum(char_counts)) / len(char_counts)
295
+        self.assertLess(mean, expected_mean + allowed_variance)
296
+        self.assertGreater(mean, max(0, expected_mean - allowed_variance))
297
+        if expected_minimum:
298
+            self.assertGreaterEqual(min(char_counts), expected_minimum)
299
+
300
+    def test_class_uniformity(self):
301
+        template_rs = '''
302
+HeatTemplateFormatVersion: '2012-12-12'
303
+Resources:
304
+  secret:
305
+    Type: OS::Heat::RandomString
306
+    Properties:
307
+      length: 66
308
+      character_classes:
309
+        - class: lettersdigits
310
+      character_sequences:
311
+        - sequence: "*$"
312
+'''
313
+
314
+        results = self.run_test(template_rs, 10)
315
+        for char in '$*':
316
+            self.check_stats(self.char_counts(results, char), 1.5, 2)
317
+
318
+    def test_repeated_sequence(self):
319
+        template_rs = '''
320
+HeatTemplateFormatVersion: '2012-12-12'
321
+Resources:
322
+  secret:
323
+    Type: OS::Heat::RandomString
324
+    Properties:
325
+      length: 40
326
+      character_classes: []
327
+      character_sequences:
328
+        - sequence: "**********$*****************************"
329
+'''
330
+
331
+        results = self.run_test(template_rs)
332
+        for char in '$*':
333
+            self.check_stats(self.char_counts(results, char), 20, 6)
334
+
335
+    def test_overlapping_classes(self):
336
+        template_rs = '''
337
+HeatTemplateFormatVersion: '2012-12-12'
338
+Resources:
339
+  secret:
340
+    Type: OS::Heat::RandomString
341
+    Properties:
342
+      length: 624
343
+      character_classes:
344
+        - class: lettersdigits
345
+        - class: digits
346
+        - class: octdigits
347
+        - class: hexdigits
348
+'''
349
+
350
+        results = self.run_test(template_rs, 20)
351
+        self.check_stats(self.char_counts(results, '0'), 10.3, 2.5)
352
+
353
+    def test_overlapping_sequences(self):
354
+        template_rs = '''
355
+HeatTemplateFormatVersion: '2012-12-12'
356
+Resources:
357
+  secret:
358
+    Type: OS::Heat::RandomString
359
+    Properties:
360
+      length: 60
361
+      character_classes: []
362
+      character_sequences:
363
+        - sequence: "01"
364
+        - sequence: "02"
365
+        - sequence: "03"
366
+        - sequence: "04"
367
+        - sequence: "05"
368
+        - sequence: "06"
369
+        - sequence: "07"
370
+        - sequence: "08"
371
+        - sequence: "09"
372
+'''
373
+
374
+        results = self.run_test(template_rs)
375
+        self.check_stats(self.char_counts(results, '0'), 10, 5)
376
+
377
+    def test_overlapping_class_sequence(self):
378
+        template_rs = '''
379
+HeatTemplateFormatVersion: '2012-12-12'
380
+Resources:
381
+  secret:
382
+    Type: OS::Heat::RandomString
383
+    Properties:
384
+      length: 402
385
+      character_classes:
386
+        - class: octdigits
387
+      character_sequences:
388
+        - sequence: "0"
389
+'''
390
+
391
+        results = self.run_test(template_rs, 10)
392
+        self.check_stats(self.char_counts(results, '0'), 51.125, 8, 1)

+ 9
- 0
releasenotes/notes/random-string-entropy-9b8e23874cd79b8f.yaml View File

@@ -0,0 +1,9 @@
1
+---
2
+security:
3
+  - |
4
+    Passwords generated by the OS::Heat::RandomString resource may have had
5
+    less entropy than expected, depending on what is specified in the
6
+    ``character_class`` and ``character_sequence`` properties. This has been
7
+    corrected so that each character present in any of the specified classes
8
+    or sequences now has an equal probability of appearing at each point in
9
+    the generated random string.

Loading…
Cancel
Save