-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
First stab at a re-write. #1
base: main
Are you sure you want to change the base?
Changes from all commits
2008f84
1d40207
1bd42ea
94d1afe
e859bf7
1193197
5355a4d
4cc6880
cfa115f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,190 @@ | ||||||||||||||||||||||
#!/usr/bin/env python3 | ||||||||||||||||||||||
import argparse | ||||||||||||||||||||||
import fnmatch | ||||||||||||||||||||||
import re | ||||||||||||||||||||||
import os | ||||||||||||||||||||||
import subprocess | ||||||||||||||||||||||
import readline | ||||||||||||||||||||||
from jinja2 import Template, Environment, FileSystemLoader | ||||||||||||||||||||||
|
||||||||||||||||||||||
file_loader = FileSystemLoader('.') | ||||||||||||||||||||||
env = Environment(loader=file_loader) | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
description = """ | ||||||||||||||||||||||
Simple script to handle generating Certificate Signing Requests (CSR) | ||||||||||||||||||||||
This has the ability to generate CSRs with and without a provided key. | ||||||||||||||||||||||
Also can handle multiple domains as Subject Alternative Name (SAN) records. | ||||||||||||||||||||||
""" | ||||||||||||||||||||||
config_template = """[ req ] | ||||||||||||||||||||||
default_bits = 2048 | ||||||||||||||||||||||
distinguished_name = req_distinguished_name | ||||||||||||||||||||||
prompt = no | ||||||||||||||||||||||
req_extensions = v3_req | ||||||||||||||||||||||
[ req_distinguished_name ] | ||||||||||||||||||||||
{% if c is not none -%} | ||||||||||||||||||||||
C = {{s}} | ||||||||||||||||||||||
{%- endif %} | ||||||||||||||||||||||
{%- if l is not none -%} | ||||||||||||||||||||||
L = {{l}} | ||||||||||||||||||||||
{%- endif %} | ||||||||||||||||||||||
{%- if s is not none -%} | ||||||||||||||||||||||
ST = {{s}} | ||||||||||||||||||||||
{%- endif %} | ||||||||||||||||||||||
{%- if o is not none -%} | ||||||||||||||||||||||
O = {{o}} | ||||||||||||||||||||||
{%- endif %} | ||||||||||||||||||||||
{{' CN = {{ cn }} | ||||||||||||||||||||||
[ v3_req ] | ||||||||||||||||||||||
subjectAltName = @alt_names | ||||||||||||||||||||||
|
||||||||||||||||||||||
[alt_names] | ||||||||||||||||||||||
DNS.1 = {{ cn }} | ||||||||||||||||||||||
{%- for domain in sans %} | ||||||||||||||||||||||
{%- if domain %} | ||||||||||||||||||||||
DNS.{{ loop.index +1 }} = {{ domain }} | ||||||||||||||||||||||
{%- endif %} | ||||||||||||||||||||||
{%- endfor %} | ||||||||||||||||||||||
'}} | ||||||||||||||||||||||
""" | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
def check_file(file): | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The name There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aaaand I just realized this is already a library function: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also the Can document in code by checking your parameter first thing in the code: In general, good practice to check your parameters before using them, to make debugging easier. |
||||||||||||||||||||||
try: | ||||||||||||||||||||||
os.stat(file).st_size > 1 | ||||||||||||||||||||||
except: | ||||||||||||||||||||||
return False | ||||||||||||||||||||||
else: | ||||||||||||||||||||||
return file | ||||||||||||||||||||||
Comment on lines
+53
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually that may not be right. A docstring would be helpful to define the desired behavior. I think I misinterpreted it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is suppoed to check that the file exists and is not empty. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see two different types being returned by this function: bool Choose one and stick with it. Looks like you want bool here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it returns true how does that affect argparse value? |
||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
def check_domain(input): | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comments as earlier:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two things. Check if it is a domain or if it is a file with domains. Maybe I should create 2 flags rather then combining them in one? |
||||||||||||||||||||||
if check_file(input): | ||||||||||||||||||||||
return input, True | ||||||||||||||||||||||
else: | ||||||||||||||||||||||
domain_template = re.compile( | ||||||||||||||||||||||
"^(?=.{1,255}$)(?!-)[A-Za-z0-9\-]{1,63}(\.[A-Za-z0-9\-]{1,63})*\.?(?<!-)$") | ||||||||||||||||||||||
if domain_template.match(input): | ||||||||||||||||||||||
return input.lower() | ||||||||||||||||||||||
else: | ||||||||||||||||||||||
raise argparse.ArgumentError( | ||||||||||||||||||||||
"Unable to determine if this is a domain or a file. Please check your input.") | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
def parse_arguments(): | ||||||||||||||||||||||
parser = argparse.ArgumentParser(description=description, | ||||||||||||||||||||||
formatter_class=argparse.RawTextHelpFormatter) | ||||||||||||||||||||||
subparser = parser.add_subparsers(dest="command") | ||||||||||||||||||||||
|
||||||||||||||||||||||
csr = subparser.add_parser("csr", help="Create CSR", | ||||||||||||||||||||||
formatter_class=argparse.ArgumentDefaultsHelpFormatter) | ||||||||||||||||||||||
csr.add_argument('-k', "--key", type=check_file, help='Specify a key') | ||||||||||||||||||||||
csr.add_argument('-d', "--domain", type=check_domain, required=True, | ||||||||||||||||||||||
help="Specify a domain or a file with list of domains") | ||||||||||||||||||||||
csr.add_argument("-o", "--org", help="Organization Name") | ||||||||||||||||||||||
|
||||||||||||||||||||||
config = subparser.add_parser("config", help="Create config file") | ||||||||||||||||||||||
config.add_argument("-c", "--country", help="Country short code") | ||||||||||||||||||||||
config.add_argument("-l", "--locality", help="Locality/City") | ||||||||||||||||||||||
config.add_argument("-s", "--state", help="State Name") | ||||||||||||||||||||||
config.add_argument("-o", "--org", help="Organization Name", required=True) | ||||||||||||||||||||||
|
||||||||||||||||||||||
if parser.parse_args().command: | ||||||||||||||||||||||
return parser.parse_args() | ||||||||||||||||||||||
else: | ||||||||||||||||||||||
parser.print_help() | ||||||||||||||||||||||
exit() | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
def gen_csr_config(args, config_file): | ||||||||||||||||||||||
tm = env.get_template(config_file) | ||||||||||||||||||||||
print(config_file) | ||||||||||||||||||||||
SANS = list() | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All-caps variable names are generally used for constants. Suggest |
||||||||||||||||||||||
print("Generating CSR configuration..") | ||||||||||||||||||||||
if type(args.domain) is tuple: | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||||||||||||
domain_file, is_file = args.domain | ||||||||||||||||||||||
with open(domain_file) as fp: | ||||||||||||||||||||||
domain = fp.readline() | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggest using |
||||||||||||||||||||||
count = 0 | ||||||||||||||||||||||
while domain: | ||||||||||||||||||||||
print(domain) | ||||||||||||||||||||||
if count == 0: | ||||||||||||||||||||||
CN = domain.strip().lower() | ||||||||||||||||||||||
domain = fp.readline() | ||||||||||||||||||||||
count += 1 | ||||||||||||||||||||||
elif 1 <= count <= 100: | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggest creating a named constant at the top of the file instead of burying |
||||||||||||||||||||||
SANS.append(domain.strip().lower()) | ||||||||||||||||||||||
domain = fp.readline() | ||||||||||||||||||||||
count += 1 | ||||||||||||||||||||||
else: | ||||||||||||||||||||||
print("Too many domains! Please limit to 100") | ||||||||||||||||||||||
exit(1) | ||||||||||||||||||||||
print(CN) | ||||||||||||||||||||||
else: | ||||||||||||||||||||||
domain = args.domain | ||||||||||||||||||||||
CN = domain.strip().lower() | ||||||||||||||||||||||
|
||||||||||||||||||||||
generated_csr_config = open(args.domain + '_csr_config', 'w') | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer |
||||||||||||||||||||||
generated_csr_config.write(tm.render(cn=CN, sans=SANS)) | ||||||||||||||||||||||
generated_csr_config.close() | ||||||||||||||||||||||
|
||||||||||||||||||||||
return str(args.domain + '_csr_config') | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
def main(args): | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems to me like there's too much implementation logic in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's an example of how it could be implemented https://github.com/pre-commit/pre-commit/blob/main/pre_commit/main.py#L349-L355 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 to keeping There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also consider trapping the case where |
||||||||||||||||||||||
if args.command == 'csr': | ||||||||||||||||||||||
# Look for csrgen config files | ||||||||||||||||||||||
config_file_search = fnmatch.filter(os.listdir(), "*_gen_config") | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see |
||||||||||||||||||||||
if len(config_file_search) == 0: | ||||||||||||||||||||||
print("No config files found please run with 'config -h' to create one") | ||||||||||||||||||||||
exit() | ||||||||||||||||||||||
elif len(config_file_search) > 1: # Check if there is more then one config file | ||||||||||||||||||||||
if args.org: | ||||||||||||||||||||||
# Check if any of the config files match the specified Org | ||||||||||||||||||||||
if check_file({args.org + "_gen_config"}): | ||||||||||||||||||||||
csr_config_out = gen_csr_config( | ||||||||||||||||||||||
args, {args.org + "_gen_config"}) | ||||||||||||||||||||||
else: | ||||||||||||||||||||||
print("Please specify Organization name with -o <Org>") | ||||||||||||||||||||||
exit() | ||||||||||||||||||||||
else: # There is only one found so assume its the right one\ | ||||||||||||||||||||||
csr_config_out = gen_csr_config(args, config_file_search[0]) | ||||||||||||||||||||||
|
||||||||||||||||||||||
# Validate key argument and file | ||||||||||||||||||||||
if args.key: | ||||||||||||||||||||||
if check_file(args.key): | ||||||||||||||||||||||
try: | ||||||||||||||||||||||
subprocess.call(['openssl', 'rsa', '-in', | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here and elsewhere: suggest checking & printing |
||||||||||||||||||||||
args.key, '-check']) | ||||||||||||||||||||||
except: | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be, but I'm not 100% sure.
Suggested change
|
||||||||||||||||||||||
print("Key specified is not a valid rsa key.") | ||||||||||||||||||||||
else: | ||||||||||||||||||||||
print("Key file specified doesn't exist.") | ||||||||||||||||||||||
else: | ||||||||||||||||||||||
print("No key specified. Creating a new one.") | ||||||||||||||||||||||
subprocess.call(['openssl', 'genrsa', '-out', | ||||||||||||||||||||||
args.domain + '.key', '2048']) | ||||||||||||||||||||||
print("Key Generated") | ||||||||||||||||||||||
|
||||||||||||||||||||||
if args.key: | ||||||||||||||||||||||
print("Generating CSR") | ||||||||||||||||||||||
subprocess.call(['openssl', 'req', '-new', '-config', csr_config_out, | ||||||||||||||||||||||
'-key', + args.key, '-out', args.domain + '.csr']) | ||||||||||||||||||||||
else: | ||||||||||||||||||||||
print("Generating CSR") | ||||||||||||||||||||||
subprocess.call(['openssl', 'req', '-new', '-config', csr_config_out, | ||||||||||||||||||||||
'-key', args.domain + '.key', '-out', args.domain + '.csr']) | ||||||||||||||||||||||
|
||||||||||||||||||||||
if args.command == 'config': | ||||||||||||||||||||||
tm = Template(config_template) | ||||||||||||||||||||||
config_file_name = args.org + "_gen_config" | ||||||||||||||||||||||
print("Creating config template file: {filename}".format( | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT use an f-string instead of ex. |
||||||||||||||||||||||
filename=config_file_name)) | ||||||||||||||||||||||
config_file = open(config_file_name, "w") | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer |
||||||||||||||||||||||
config_file.write(tm.render(c=args.country, l=args.locality, | ||||||||||||||||||||||
s=args.state, o=args.org)) | ||||||||||||||||||||||
config_file.close() | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
if __name__ == '__main__': | ||||||||||||||||||||||
main(parse_arguments()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PEP8 says constants should be named in all caps.
https://peps.python.org/pep-0008/#constants
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: mixed use of single-quote and double-quotes throughout this file.