Skip to content

Commit 859b21e

Browse files
jmcrawford45jmcrawford45
authored
Merge pull request #4762 from jmcrawford45/PS-5627-vault-dos
Fix various regex security vulns
2 parents caeecb7 + d2e3227 commit 859b21e

File tree

10 files changed

+99
-14
lines changed

10 files changed

+99
-14
lines changed

‎lemur/authorities/schemas.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ class AuthorityUpdateSchema(LemurInputSchema):
9898
description = fields.String()
9999
active = fields.Boolean(missing=True)
100100
roles = fields.Nested(AssociatedRoleSchema(many=True))
101+
options = fields.String()
101102

102103

103104
class RootAuthorityCertificateOutputSchema(LemurOutputSchema):

‎lemur/plugins/lemur_acme/plugin.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class ACMEIssuerPlugin(IssuerPlugin):
4747
"name": "acme_url",
4848
"type": "str",
4949
"required": True,
50-
"validation": check_validation(r"http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\(\),]|(?:%[0-9a-fA-F][0-9a-fA-F]))+"),
50+
"validation": check_validation(r"http[s]?://(?:[a-zA-Z]|[0-9]|[$_@.&+-]|[!*\(\),]|(?:%[0-9a-fA-F][0-9a-fA-F]))+"),
5151
"helpMessage": "ACME resource URI. Must be a valid web url starting with http[s]://",
5252
},
5353
{
@@ -342,7 +342,7 @@ class ACMEHttpIssuerPlugin(IssuerPlugin):
342342
"name": "acme_url",
343343
"type": "str",
344344
"required": True,
345-
"validation": check_validation(r"http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\(\),]|(?:%[0-9a-fA-F][0-9a-fA-F]))+"),
345+
"validation": check_validation(r"http[s]?://(?:[a-zA-Z]|[0-9]|[$_@.&+-]|[!*\(\),]|(?:%[0-9a-fA-F][0-9a-fA-F]))+"),
346346
"helpMessage": "Must be a valid web url starting with http[s]://",
347347
},
348348
{

‎lemur/plugins/lemur_azure_dest/plugin.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ class AzureDestinationPlugin(DestinationPlugin):
122122
"name": "azurePassword",
123123
"type": "str",
124124
"required": True,
125-
"validation": check_validation("[0-9a-zA-Z.:_-~]+"),
125+
"validation": check_validation("[0-9a-zA-Z.:_~-]+"),
126126
"helpMessage": "Tenant password for the Azure Key Vault",
127127
}
128128
]

‎lemur/plugins/lemur_vault_dest/plugin.py‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@ class VaultSourcePlugin(SourcePlugin):
6262
"name": "tokenFileOrVaultRole",
6363
"type": "str",
6464
"required": True,
65-
"validation": check_validation("^([a-zA-Z0-9/._-]+/?)+$"),
66-
"helpMessage": "Must be vaild file path for token based auth and valid role if k8s based auth",
65+
"validation": check_validation("^[a-zA-Z0-9/._-]+/?$"),
66+
"helpMessage": "Must be valid file path for token based auth and valid role if k8s based auth",
6767
},
6868
{
6969
"name": "vaultMount",
@@ -189,7 +189,7 @@ class VaultDestinationPlugin(DestinationPlugin):
189189
"name": "tokenFileOrVaultRole",
190190
"type": "str",
191191
"required": True,
192-
"validation": check_validation("^([a-zA-Z0-9/._-]+/?)+$"),
192+
"validation": check_validation("^[a-zA-Z0-9/._-]+/?$"),
193193
"helpMessage": "Must be vaild file path for token based auth and valid role if k8s based auth",
194194
},
195195
{
@@ -203,15 +203,17 @@ class VaultDestinationPlugin(DestinationPlugin):
203203
"name": "vaultPath",
204204
"type": "str",
205205
"required": True,
206-
"validation": check_validation("^(([a-zA-Z0-9._-]+|{(CN|OU|O|L|S|C)})+/?)+$"),
207-
"helpMessage": "Must be a valid Vault secrets path. Support vars: {CN|OU|O|L|S|C}",
206+
"validation": check_validation(
207+
"^([a-zA-Z0-9._-]+|{CN}|{OU}|{O}|{L}|{S}|{C})(/?([a-zA-Z0-9._-]+|{CN}|{OU}|{O}|{L}|{S}|{C}))*$"),
208+
"helpMessage": "Must be a valid Vault secrets path. Support vars: {CN}|{OU}|{O}|{L}|{S}|{C}",
208209
},
209210
{
210211
"name": "objectName",
211212
"type": "str",
212213
"required": False,
213-
"validation": check_validation("^([0-9a-zA-Z.:_-]+|{(CN|OU|O|L|S|C)})+$"),
214-
"helpMessage": "Name to bundle certs under, if blank use {CN}. Support vars: {CN|OU|O|L|S|C}",
214+
"validation": check_validation(
215+
"^([a-zA-Z0-9:._-]+|{CN}|{OU}|{O}|{L}|{S}|{C})(/?([a-zA-Z0-9._-]+|{CN}|{OU}|{O}|{L}|{S}|{C}))*$"),
216+
"helpMessage": "Name to bundle certs under, if blank use {CN}. Support vars: {CN}|{OU}|{O}|{L}|{S}|{C}",
215217
},
216218
{
217219
"name": "bundleChain",

‎lemur/plugins/lemur_vault_dest/tests/plugin.py‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
from lemur.plugins.bases import SourcePlugin
33
import pytest
44

5+
from lemur.plugins.lemur_vault_dest.plugin import VaultSourcePlugin, VaultDestinationPlugin
6+
57

68
class TestSourcePlugin(SourcePlugin):
79
title = "Test"
@@ -94,3 +96,49 @@ def test_vault_plugin_input_schema(session):
9496
assert not errors
9597
assert data
9698
assert "plugin_object" in data
99+
100+
101+
@pytest.mark.parametrize("option, value, valid", [
102+
("tokenFileOrVaultRole", "", False),
103+
("vaultPath", "", False),
104+
("tokenFileOrVaultRole", "/leading/slash", True),
105+
("vaultPath", "/leading/slash", False),
106+
("tokenFileOrVaultRole", "{CN}/subs", False),
107+
("vaultPath", "{CN}/subs", False),
108+
("tokenFileOrVaultRole", "/leading/slash", True),
109+
("vaultPath", "/leading/slash", False),
110+
("tokenFileOrVaultRole", "noslash", True),
111+
("vaultPath", "noslash", True),
112+
("tokenFileOrVaultRole", "some/random/file.json", True),
113+
("vaultPath", "some/random/file.json", True),
114+
])
115+
def test_source_options(option, value, valid):
116+
plugin = VaultSourcePlugin()
117+
if valid:
118+
plugin.validate_option_value(option, value)
119+
else:
120+
with pytest.raises(ValueError):
121+
plugin.validate_option_value(option, value)
122+
123+
124+
@pytest.mark.parametrize("option, value, valid", [
125+
("tokenFileOrVaultRole", "", False),
126+
("vaultPath", "", False),
127+
("tokenFileOrVaultRole", "/leading/slash", True),
128+
("vaultPath", "/leading/slash", False),
129+
("tokenFileOrVaultRole", "{CN}/subs", False),
130+
("vaultPath", "{CN}/subs", True),
131+
("tokenFileOrVaultRole", "/leading/slash", True),
132+
("vaultPath", "/leading/slash", False),
133+
("tokenFileOrVaultRole", "noslash", True),
134+
("vaultPath", "noslash", True),
135+
("tokenFileOrVaultRole", "some/random/file.json", True),
136+
("vaultPath", "some/random/file.json", True),
137+
])
138+
def test_dest_options(option, value, valid):
139+
plugin = VaultDestinationPlugin()
140+
if valid:
141+
plugin.validate_option_value(option, value)
142+
else:
143+
with pytest.raises(ValueError):
144+
plugin.validate_option_value(option, value)

‎lemur/tests/test_authorities.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,3 +406,37 @@ def test_authority_roles(client, session, issuer_plugin):
406406
)
407407
assert resp.status_code == 200
408408
assert len(resp.json["roles"]) == 0
409+
410+
411+
@pytest.mark.parametrize(
412+
"token,authority_number,status",
413+
[
414+
(VALID_ADMIN_HEADER_TOKEN, 100, 200),
415+
],
416+
)
417+
def test_authorities_put_update_options(client, authority_number, token, status):
418+
"""
419+
This test relies on the configuration option ADMIN_ONLY_AUTHORITY_CREATION = True, set in conf.py
420+
"""
421+
data = {'name': f'testauthority{authority_number}', 'owner': 'test@example.com',
422+
'common_name': 'testauthority1.example.com', "serial_number": 1,
423+
"validityStart": "2023-07-12T07:00:00.000Z",
424+
"validityEnd": "2050-07-13T07:00:00.000Z",
425+
'plugin': {'slug': 'cryptography-issuer'}}
426+
response = client.post(
427+
api.url_for(AuthoritiesList),
428+
data=json.dumps(data),
429+
headers=token
430+
)
431+
assert response.status_code == status, f"expected code {status}, but actual code was {response.status_code}; error: {response.json}"
432+
response = json.loads(
433+
client.put(
434+
api.url_for(Authorities, authority_id=1), data=json.dumps(
435+
{'owner': 'updated@example.com',
436+
'description': 'updated',
437+
'roles': [],
438+
'options': json.dumps([{'updated': 'bar'}])}), headers=token
439+
).text
440+
)
441+
for field in ['owner', 'description', 'options']:
442+
assert 'updated' in json.dumps(response[field])

‎requirements-dev.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ jeepney==0.8.0
298298
# via
299299
# keyring
300300
# secretstorage
301-
jinja2==3.1.2
301+
jinja2==3.1.3
302302
# via
303303
# -r requirements-tests.txt
304304
# flask

‎requirements-docs.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ javaobj-py3==0.4.3
327327
# via
328328
# -r requirements-tests.txt
329329
# pyjks
330-
jinja2==3.1.2
330+
jinja2==3.1.3
331331
# via
332332
# -r requirements-tests.txt
333333
# flask

‎requirements-tests.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ javaobj-py3==0.4.3
255255
# via
256256
# -r requirements.txt
257257
# pyjks
258-
jinja2==3.1.2
258+
jinja2==3.1.3
259259
# via
260260
# -r requirements.txt
261261
# flask

‎requirements.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ itsdangerous==2.1.2
151151
# flask
152152
javaobj-py3==0.4.3
153153
# via pyjks
154-
jinja2==3.1.2
154+
jinja2==3.1.3
155155
# via
156156
# -r requirements.in
157157
# flask

0 commit comments

Comments
 (0)