From b677e6e3253b81fc8467803f2697ac99b35fe3e2 Mon Sep 17 00:00:00 2001 From: sayali Date: Tue, 13 Oct 2020 19:40:01 -0700 Subject: [PATCH 01/14] Copy subject details for non-CAB-compliant authorities --- lemur/authorities/models.py | 17 +++++++++++++++++ lemur/authorities/schemas.py | 1 + lemur/certificates/schemas.py | 23 ++++++++++++++++++++++- 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/lemur/authorities/models.py b/lemur/authorities/models.py index ccd1fab8..e8c0e03a 100644 --- a/lemur/authorities/models.py +++ b/lemur/authorities/models.py @@ -6,6 +6,8 @@ :license: Apache, see LICENSE for more details. .. moduleauthor:: Kevin Glisson """ +import json + from sqlalchemy.orm import relationship from sqlalchemy import ( Column, @@ -80,5 +82,20 @@ class Authority(db.Model): def plugin(self): return plugins.get(self.plugin_name) + @property + def is_cab_compliant(self): + """ + Parse the options to find whether authority is CAB Compliant. Returns None if + option is not available + """ + if not self.options: + return None + + for option in json.loads(self.options): + if option["name"] == 'cab_compliant': + return option["value"] + + return None + def __repr__(self): return "Authority(name={name})".format(name=self.name) diff --git a/lemur/authorities/schemas.py b/lemur/authorities/schemas.py index f80d1581..6c48a183 100644 --- a/lemur/authorities/schemas.py +++ b/lemur/authorities/schemas.py @@ -139,6 +139,7 @@ class AuthorityNestedOutputSchema(LemurOutputSchema): plugin = fields.Nested(PluginOutputSchema) active = fields.Boolean() authority_certificate = fields.Nested(RootAuthorityCertificateOutputSchema, only=["max_issuance_days", "default_validity_days"]) + is_cab_compliant = fields.Boolean() authority_update_schema = AuthorityUpdateSchema() diff --git a/lemur/certificates/schemas.py b/lemur/certificates/schemas.py index 688d6ba4..21abd214 100644 --- a/lemur/certificates/schemas.py +++ b/lemur/certificates/schemas.py @@ -8,7 +8,7 @@ from flask import current_app from flask_restful import inputs from flask_restful.reqparse import RequestParser -from marshmallow import fields, validate, validates_schema, post_load, pre_load +from marshmallow import fields, validate, validates_schema, post_load, pre_load, post_dump from marshmallow.exceptions import ValidationError from lemur.authorities.schemas import AuthorityNestedOutputSchema @@ -332,6 +332,27 @@ class CertificateOutputSchema(LemurOutputSchema): ) rotation_policy = fields.Nested(RotationPolicyNestedOutputSchema) + country = fields.String() + location = fields.String() + state = fields.String() + organization = fields.String() + organizational_unit = fields.String() + + @post_dump + def handle_subject_details(self, data): + # Remove subject details if authority is CAB compliant. The code will use default set of values in that case. + # If CAB compliance of an authority is unknown (None), it is safe to fallback to default values. Thus below + # condition checks for 'not False' ==> 'True or None' + if data.get("authority"): + is_cab_compliant = data.get("authority").get("isCabCompliant") + + if is_cab_compliant is not False: + data.pop("country", None) + data.pop("state", None) + data.pop("location", None) + data.pop("organization", None) + data.pop("organizational_unit", None) + class CertificateShortOutputSchema(LemurOutputSchema): id = fields.Integer() From 28381737dc8401d683063a47e9abc60831039ffb Mon Sep 17 00:00:00 2001 From: sayali Date: Tue, 13 Oct 2020 19:19:12 -0700 Subject: [PATCH 02/14] Removed OU from digicert plugin --- lemur/plugins/lemur_digicert/plugin.py | 1 - lemur/plugins/lemur_digicert/tests/test_digicert.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lemur/plugins/lemur_digicert/plugin.py b/lemur/plugins/lemur_digicert/plugin.py index cf01c9d1..f28279a6 100644 --- a/lemur/plugins/lemur_digicert/plugin.py +++ b/lemur/plugins/lemur_digicert/plugin.py @@ -175,7 +175,6 @@ def map_cis_fields(options, csr): }, "organization": { "name": options["organization"], - "units": [options["organizational_unit"]], }, } # possibility to default to a SIGNING_ALGORITHM for a given profile diff --git a/lemur/plugins/lemur_digicert/tests/test_digicert.py b/lemur/plugins/lemur_digicert/tests/test_digicert.py index 4abfcf54..34dcef71 100644 --- a/lemur/plugins/lemur_digicert/tests/test_digicert.py +++ b/lemur/plugins/lemur_digicert/tests/test_digicert.py @@ -121,7 +121,7 @@ def test_map_cis_fields_with_validity_years(mock_current_app, authority): "csr": CSR_STR, "additional_dns_names": names, "signature_hash": "sha256", - "organization": {"name": "Example, Inc.", "units": ["Example Org"]}, + "organization": {"name": "Example, Inc."}, "validity": { "valid_to": arrow.get(2018, 11, 3).format("YYYY-MM-DDTHH:MM") + "Z" }, @@ -157,7 +157,7 @@ def test_map_cis_fields_with_validity_end_and_start(mock_current_app, app, autho "csr": CSR_STR, "additional_dns_names": names, "signature_hash": "sha256", - "organization": {"name": "Example, Inc.", "units": ["Example Org"]}, + "organization": {"name": "Example, Inc."}, "validity": { "valid_to": arrow.get(2017, 5, 7).format("YYYY-MM-DDTHH:MM") + "Z" }, From 82dd6639424865cb5324db199783f7c8ef931917 Mon Sep 17 00:00:00 2001 From: sayali Date: Tue, 13 Oct 2020 19:20:08 -0700 Subject: [PATCH 03/14] Moving default key_type to getDefaults --- .../app/angular/certificates/certificate/certificate.js | 3 --- lemur/static/app/angular/certificates/services.js | 5 +++++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lemur/static/app/angular/certificates/certificate/certificate.js b/lemur/static/app/angular/certificates/certificate/certificate.js index 9fadb655..4bdbf60e 100644 --- a/lemur/static/app/angular/certificates/certificate/certificate.js +++ b/lemur/static/app/angular/certificates/certificate/certificate.js @@ -255,9 +255,6 @@ angular.module('lemur') $scope.certificate.replacedBy = []; // should not clone 'replaced by' info $scope.certificate.removeReplaces(); // should not clone 'replacement cert' info - if(!$scope.certificate.keyType) { - $scope.certificate.keyType = 'RSA2048'; // default algo to select during clone if backend did not return algo - } CertificateService.getDefaults($scope.certificate); }); diff --git a/lemur/static/app/angular/certificates/services.js b/lemur/static/app/angular/certificates/services.js index ce88ccb3..280d6078 100644 --- a/lemur/static/app/angular/certificates/services.js +++ b/lemur/static/app/angular/certificates/services.js @@ -289,6 +289,11 @@ angular.module('lemur') if (certificate.dnsProviderId) { certificate.dnsProvider = {id: certificate.dnsProviderId}; } + + if(!certificate.keyType) { + certificate.keyType = 'RSA2048'; // default algo to select during clone if backend did not return algo + } + }); }; From 97cf54433b5cc0d00819735ff60a87f0b711eb87 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Wed, 14 Oct 2020 09:45:13 -0700 Subject: [PATCH 04/14] Update models.py language --- lemur/authorities/models.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lemur/authorities/models.py b/lemur/authorities/models.py index e8c0e03a..ba0516fd 100644 --- a/lemur/authorities/models.py +++ b/lemur/authorities/models.py @@ -85,8 +85,9 @@ class Authority(db.Model): @property def is_cab_compliant(self): """ - Parse the options to find whether authority is CAB Compliant. Returns None if - option is not available + Parse the options to find whether authority is CAB Forum Compliant + i.e., adhering to the CA/Browser Forum Baseline Requirements. + Returns None if option is not available """ if not self.options: return None From 894e35b4e2a8f310a024bdf7da89009bc47b1f29 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Wed, 14 Oct 2020 09:48:40 -0700 Subject: [PATCH 05/14] Update schemas.py minor language --- lemur/certificates/schemas.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lemur/certificates/schemas.py b/lemur/certificates/schemas.py index 21abd214..cc0a607e 100644 --- a/lemur/certificates/schemas.py +++ b/lemur/certificates/schemas.py @@ -340,8 +340,8 @@ class CertificateOutputSchema(LemurOutputSchema): @post_dump def handle_subject_details(self, data): - # Remove subject details if authority is CAB compliant. The code will use default set of values in that case. - # If CAB compliance of an authority is unknown (None), it is safe to fallback to default values. Thus below + # Remove subject details if authority is CA/Browser Forum compliant. The code will use default set of values in that case. + # If CA/Browser Forum compliance of an authority is unknown (None), it is safe to fallback to default values. Thus below # condition checks for 'not False' ==> 'True or None' if data.get("authority"): is_cab_compliant = data.get("authority").get("isCabCompliant") From 409e12a9d60901ef35449058774bb610c6ddbdeb Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Wed, 14 Oct 2020 10:03:44 -0700 Subject: [PATCH 06/14] Update models.py lint --- lemur/authorities/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemur/authorities/models.py b/lemur/authorities/models.py index ba0516fd..cced3ed1 100644 --- a/lemur/authorities/models.py +++ b/lemur/authorities/models.py @@ -85,7 +85,7 @@ class Authority(db.Model): @property def is_cab_compliant(self): """ - Parse the options to find whether authority is CAB Forum Compliant + Parse the options to find whether authority is CAB Forum Compliant, i.e., adhering to the CA/Browser Forum Baseline Requirements. Returns None if option is not available """ From f08b50a952b5988b1939ba35f392321f10f64d17 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Wed, 14 Oct 2020 17:45:18 +0000 Subject: [PATCH 07/14] Bump boto3 from 1.15.12 to 1.15.16 Bumps [boto3](https://github.com/boto/boto3) from 1.15.12 to 1.15.16. - [Release notes](https://github.com/boto/boto3/releases) - [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst) - [Commits](https://github.com/boto/boto3/compare/1.15.12...1.15.16) Signed-off-by: dependabot-preview[bot] --- requirements-docs.txt | 2 +- requirements-tests.txt | 2 +- requirements.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements-docs.txt b/requirements-docs.txt index e35ead88..38b62198 100644 --- a/requirements-docs.txt +++ b/requirements-docs.txt @@ -17,7 +17,7 @@ bcrypt==3.1.7 # via -r requirements.txt, flask-bcrypt, paramiko beautifulsoup4==4.9.1 # via -r requirements.txt, cloudflare billiard==3.6.3.0 # via -r requirements.txt, celery blinker==1.4 # via -r requirements.txt, flask-mail, flask-principal, raven -boto3==1.15.12 # via -r requirements.txt +boto3==1.15.16 # via -r requirements.txt botocore==1.18.16 # via -r requirements.txt, boto3, s3transfer celery[redis]==4.4.2 # via -r requirements.txt certifi==2020.6.20 # via -r requirements.txt, requests diff --git a/requirements-tests.txt b/requirements-tests.txt index 07bd2c0b..39ef0e66 100644 --- a/requirements-tests.txt +++ b/requirements-tests.txt @@ -10,7 +10,7 @@ aws-sam-translator==1.22.0 # via cfn-lint aws-xray-sdk==2.5.0 # via moto bandit==1.6.2 # via -r requirements-tests.in black==20.8b1 # via -r requirements-tests.in -boto3==1.15.12 # via aws-sam-translator, moto +boto3==1.15.16 # via aws-sam-translator, moto boto==2.49.0 # via moto botocore==1.18.16 # via aws-xray-sdk, boto3, moto, s3transfer certifi==2020.6.20 # via requests diff --git a/requirements.txt b/requirements.txt index fb43b5a3..d323b40f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -15,7 +15,7 @@ bcrypt==3.1.7 # via flask-bcrypt, paramiko beautifulsoup4==4.9.1 # via cloudflare billiard==3.6.3.0 # via celery blinker==1.4 # via flask-mail, flask-principal, raven -boto3==1.15.12 # via -r requirements.in +boto3==1.15.16 # via -r requirements.in botocore==1.18.16 # via -r requirements.in, boto3, s3transfer celery[redis]==4.4.2 # via -r requirements.in certifi==2020.6.20 # via -r requirements.in, requests From ddf94e04da75dabf599e7e846f78f0fe40e251f1 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Wed, 14 Oct 2020 18:09:48 +0000 Subject: [PATCH 08/14] Bump faker from 4.4.0 to 4.14.0 Bumps [faker](https://github.com/joke2k/faker) from 4.4.0 to 4.14.0. - [Release notes](https://github.com/joke2k/faker/releases) - [Changelog](https://github.com/joke2k/faker/blob/master/CHANGELOG.rst) - [Commits](https://github.com/joke2k/faker/compare/v4.4.0...v4.14.0) Signed-off-by: dependabot-preview[bot] --- requirements-tests.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements-tests.txt b/requirements-tests.txt index 39ef0e66..8df4f5d1 100644 --- a/requirements-tests.txt +++ b/requirements-tests.txt @@ -24,7 +24,7 @@ decorator==4.4.2 # via networkx docker==4.2.0 # via moto ecdsa==0.14.1 # via moto, python-jose, sshpubkeys factory-boy==3.1.0 # via -r requirements-tests.in -faker==4.4.0 # via -r requirements-tests.in, factory-boy +faker==4.14.0 # via -r requirements-tests.in, factory-boy fakeredis==1.4.3 # via -r requirements-tests.in flask==1.1.2 # via pytest-flask freezegun==1.0.0 # via -r requirements-tests.in From 62d099b50073d8723a2c465c43e7a5f60c2fc37a Mon Sep 17 00:00:00 2001 From: sayali Date: Wed, 14 Oct 2020 12:41:49 -0700 Subject: [PATCH 09/14] Unit tests to check cab_compliant option --- lemur/tests/test_certificates.py | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/lemur/tests/test_certificates.py b/lemur/tests/test_certificates.py index c4140f03..c0f59f2f 100644 --- a/lemur/tests/test_certificates.py +++ b/lemur/tests/test_certificates.py @@ -9,7 +9,6 @@ from cryptography import x509 from cryptography.hazmat.backends import default_backend from marshmallow import ValidationError from freezegun import freeze_time -# from mock import patch from unittest.mock import patch from lemur.certificates.service import create_csr @@ -171,10 +170,33 @@ def test_certificate_output_schema(session, certificate, issuer_plugin): ) as wrapper: data, errors = CertificateOutputSchema().dump(certificate) assert data["issuer"] == "LemurTrustUnittestsClass1CA2018" + assert data["distinguishedName"] == "L=Earth,ST=N/A,C=EE,OU=Karate Lessons,O=Daniel San & co,CN=san.example.org" + # Authority does not have 'cab_compliant', thus subject details should not be returned + assert "organization" not in data assert wrapper.call_count == 1 +def test_certificate_output_schema_subject_details(session, certificate, issuer_plugin): + from lemur.certificates.schemas import CertificateOutputSchema + from lemur.authorities.service import update_options + + # Mark authority as non-cab-compliant + update_options(certificate.authority.id, '[{"name": "cab_compliant","value":false}]') + + data, errors = CertificateOutputSchema().dump(certificate) + assert not errors + assert data["issuer"] == "LemurTrustUnittestsClass1CA2018" + assert data["distinguishedName"] == "L=Earth,ST=N/A,C=EE,OU=Karate Lessons,O=Daniel San & co,CN=san.example.org" + + # Original subject details should be returned because of cab_compliant option update above + assert data["country"] == "EE" + assert data["state"] == "N/A" + assert data["location"] == "Earth" + assert data["organization"] == "Daniel San & co" + assert data["organizationalUnit"] == "Karate Lessons" + + def test_certificate_edit_schema(session): from lemur.certificates.schemas import CertificateEditInputSchema @@ -921,6 +943,14 @@ def test_certificate_get_body(client): "CN=LemurTrust Unittests Class 1 CA 2018" ) + # No authority details are provided in this test, no information about being cab_compliant is available. + # Thus original subject details should be returned. + assert response_body["country"] == "EE" + assert response_body["state"] == "N/A" + assert response_body["location"] == "Earth" + assert response_body["organization"] == "LemurTrust Enterprises Ltd" + assert response_body["organizationalUnit"] == "Unittesting Operations Center" + @pytest.mark.parametrize( "token,status", From 90839b4d4b1b467409fb84827ab957bd039578e4 Mon Sep 17 00:00:00 2001 From: sayali Date: Wed, 14 Oct 2020 14:48:54 -0700 Subject: [PATCH 10/14] Unit test for cab_compliant = true --- lemur/tests/test_certificates.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lemur/tests/test_certificates.py b/lemur/tests/test_certificates.py index c0f59f2f..fbe24e26 100644 --- a/lemur/tests/test_certificates.py +++ b/lemur/tests/test_certificates.py @@ -196,6 +196,16 @@ def test_certificate_output_schema_subject_details(session, certificate, issuer_ assert data["organization"] == "Daniel San & co" assert data["organizationalUnit"] == "Karate Lessons" + # Mark authority as cab-compliant + update_options(certificate.authority.id, '[{"name": "cab_compliant","value":true}]') + data, errors = CertificateOutputSchema().dump(certificate) + assert not errors + assert "country" not in data + assert "state" not in data + assert "location" not in data + assert "organization" not in data + assert "organizationalUnit" not in data + def test_certificate_edit_schema(session): from lemur.certificates.schemas import CertificateEditInputSchema From ee1d07000a4c827a061979d49d2b2369110e20f5 Mon Sep 17 00:00:00 2001 From: sayali Date: Wed, 14 Oct 2020 14:49:46 -0700 Subject: [PATCH 11/14] Test subject details in reissue with cab_compliant option --- lemur/tests/test_certificates.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/lemur/tests/test_certificates.py b/lemur/tests/test_certificates.py index fbe24e26..9c50c438 100644 --- a/lemur/tests/test_certificates.py +++ b/lemur/tests/test_certificates.py @@ -791,12 +791,25 @@ def test_reissue_certificate( issuer_plugin, crypto_authority, certificate, logged_in_user ): from lemur.certificates.service import reissue_certificate + from lemur.authorities.service import update_options + from lemur.tests.conf import LEMUR_DEFAULT_ORGANIZATION # test-authority would return a mismatching private key, so use 'cryptography-issuer' plugin instead. certificate.authority = crypto_authority new_cert = reissue_certificate(certificate) assert new_cert - assert (new_cert.key_type == "RSA2048") + assert new_cert.key_type == "RSA2048" + assert new_cert.organization != certificate.organization + # Check for default value since authority does not have cab_compliant option set + assert new_cert.organization == LEMUR_DEFAULT_ORGANIZATION + + # update cab_compliant option to false for crypto_authority to maintain subject details + update_options(crypto_authority.id, '[{"name": "cab_compliant","value":false}]') + new_cert = reissue_certificate(certificate) + assert new_cert.organization == certificate.organization + + # reset options + update_options(crypto_authority.id, None) def test_create_csr(): From 4d5e712e854367c8d42d0e02ac407bd07c7f4e8c Mon Sep 17 00:00:00 2001 From: sayali Date: Wed, 14 Oct 2020 15:40:23 -0700 Subject: [PATCH 12/14] Remove option reset from test --- lemur/tests/test_certificates.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/lemur/tests/test_certificates.py b/lemur/tests/test_certificates.py index 9c50c438..c271a97e 100644 --- a/lemur/tests/test_certificates.py +++ b/lemur/tests/test_certificates.py @@ -808,9 +808,6 @@ def test_reissue_certificate( new_cert = reissue_certificate(certificate) assert new_cert.organization == certificate.organization - # reset options - update_options(crypto_authority.id, None) - def test_create_csr(): csr, private_key = create_csr( From f38380d156a29d5403ba43c2ef7e07b0d1b57c40 Mon Sep 17 00:00:00 2001 From: sayali Date: Wed, 14 Oct 2020 17:38:32 -0700 Subject: [PATCH 13/14] Check if option is present --- lemur/authorities/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemur/authorities/models.py b/lemur/authorities/models.py index cced3ed1..d1b41a21 100644 --- a/lemur/authorities/models.py +++ b/lemur/authorities/models.py @@ -93,7 +93,7 @@ class Authority(db.Model): return None for option in json.loads(self.options): - if option["name"] == 'cab_compliant': + if "name" in option and option["name"] == 'cab_compliant': return option["value"] return None From 55658c5f23870c44f01754bfc0b8df8dffc7f361 Mon Sep 17 00:00:00 2001 From: Mathias Petermann Date: Fri, 16 Oct 2020 10:43:52 +0200 Subject: [PATCH 14/14] Add double % for escaped SQLALCHEMY_DATABASE_URI --- lemur/migrations/env.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lemur/migrations/env.py b/lemur/migrations/env.py index 3acefc3a..91fa5fcb 100644 --- a/lemur/migrations/env.py +++ b/lemur/migrations/env.py @@ -20,8 +20,9 @@ fileConfig(config.config_file_name) # target_metadata = mymodel.Base.metadata from flask import current_app +db_url_escaped = current_app.config.get('SQLALCHEMY_DATABASE_URI').replace('%', '%%') config.set_main_option( - "sqlalchemy.url", current_app.config.get("SQLALCHEMY_DATABASE_URI") + "sqlalchemy.url", db_url_escaped ) target_metadata = current_app.extensions["migrate"].db.metadata