deis / controller

Deis Workflow Controller (API)
https://deis.com
MIT License
41 stars 53 forks source link

v2.4.1: Adding a SSL cert with blank Common Name fails with DB non-null constraint error #1000

Closed seanknox closed 8 years ago

seanknox commented 8 years ago

Hi fine Deis folks,

I'm using v2.4.1. I generated self-signed SSL certs as part of the Workflow guide to start testing. Appears that a cert with with a blank CN causes an error when the controller tries to save to the DB as the column has a not-null constraint. Generating another cert with CN worked fine. Logs are below, let me know if you need any additional information.

cheers, Sean

10.244.94.9 "GET /v2/apps/yonder-yardbird/domains/?limit=100 HTTP/1.1" 200 312 "Deis Client v2.4.0-82b6368"
INFO [yonder-yardbird]: domain monkey.foo.com added
10.244.94.9 "POST /v2/apps/yonder-yardbird/domains/ HTTP/1.1" 201 133 "Deis Client v2.4.0-82b6368"
ERROR:root:Uncaught Exception
Traceback (most recent call last):
  File "/usr/local/lib/python3.5/dist-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
psycopg2.IntegrityError: null value in column "common_name" violates not-null constraint
DETAIL:  Failing row contains (4, 2016-08-22 06:19:19.467755+00, 2016-08-22 06:19:19.467779+00, -----BEGIN CERTIFICATE-----
MIIDBjCCAe4CCQD4mvp8h8UAOzANBgkqhkiG..., -----BEGIN RSA PRIVATE KEY-----
MIIEowIBAAKCAQEAroMrryY7ZbxCsvyW..., null, 2017-08-21 01:55:22+00, 2, 91:5F:B7:52:1E:F5:AD:5F:73:F6:48:BE:FB:F7:EA:98:DB:C1:16:70:25:8..., monkey-foo-com, {}, /C=AU/ST=Some-State/O=Internet Widgits Pty Ltd, 2016-08-21 01:55:22+00, /C=AU/ST=Some-State/O=Internet Widgits Pty Ltd).

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/local/lib/python3.5/dist-packages/rest_framework/views.py", line 471, in dispatch
    response = handler(request, *args, **kwargs)
  File "/usr/local/lib/python3.5/dist-packages/rest_framework/mixins.py", line 21, in create
    self.perform_create(serializer)
  File "/app/api/viewsets.py", line 20, in perform_create
    obj = serializer.save(owner=self.request.user)
  File "/usr/local/lib/python3.5/dist-packages/rest_framework/serializers.py", line 191, in save
    self.instance = self.create(validated_data)
  File "/usr/local/lib/python3.5/dist-packages/rest_framework/serializers.py", line 872, in create
    instance = ModelClass.objects.create(**validated_data)
  File "/usr/local/lib/python3.5/dist-packages/django/db/models/manager.py", line 122, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/usr/local/lib/python3.5/dist-packages/django/db/models/query.py", line 401, in create
    obj.save(force_insert=True, using=self.db)
  File "/app/api/models/certificate.py", line 149, in save
    return super(Certificate, self).save(*args, **kwargs)
  File "/usr/local/lib/python3.5/dist-packages/django/db/models/base.py", line 708, in save
    force_update=force_update, update_fields=update_fields)
  File "/usr/local/lib/python3.5/dist-packages/django/db/models/base.py", line 736, in save_base
    updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields)
  File "/usr/local/lib/python3.5/dist-packages/django/db/models/base.py", line 820, in _save_table
    result = self._do_insert(cls._base_manager, using, fields, update_pk, raw)
  File "/usr/local/lib/python3.5/dist-packages/django/db/models/base.py", line 859, in _do_insert
    using=using, raw=raw)
  File "/usr/local/lib/python3.5/dist-packages/django/db/models/manager.py", line 122, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/usr/local/lib/python3.5/dist-packages/django/db/models/query.py", line 1039, in _insert
    return query.get_compiler(using=using).execute_sql(return_id)
  File "/usr/local/lib/python3.5/dist-packages/django/db/models/sql/compiler.py", line 1060, in execute_sql
    cursor.execute(sql, params)
  File "/usr/local/lib/python3.5/dist-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/usr/local/lib/python3.5/dist-packages/django/db/utils.py", line 95, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/usr/local/lib/python3.5/dist-packages/django/utils/six.py", line 685, in reraise
    raise value.with_traceback(tb)
  File "/usr/local/lib/python3.5/dist-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
django.db.utils.IntegrityError: null value in column "common_name" violates not-null constraint
DETAIL:  Failing row contains (4, 2016-08-22 06:19:19.467755+00, 2016-08-22 06:19:19.467779+00, -----BEGIN CERTIFICATE-----
MIIDBjCCAe4CCQD4mvp8h8UAOzANBgkqhkiG..., -----BEGIN RSA PRIVATE KEY-----
MIIEowIBAAKCAQEAroMrryY7ZbxCsvyW..., null, 2017-08-21 01:55:22+00, 2, 91:5F:B7:52:1E:F5:AD:5F:73:F6:48:BE:FB:F7:EA:98:DB:C1:16:70:25:8..., monkey-foo-com, {}, /C=AU/ST=Some-State/O=Internet Widgits Pty Ltd, 2016-08-21 01:55:22+00, /C=AU/ST=Some-State/O=Internet Widgits Pty Ltd).

10.244.94.9 "POST /v2/certs/ HTTP/1.1" 500 25 "Deis Client v2.4.0-82b6368"
bacongobbler commented 8 years ago

Thanks for the report @seanknox. Looks like we want it to fail validation and return a 400 rather than throw an exception and a 500 server error.

helgi commented 8 years ago

Other option is to (maybe?) have it turn into an empty string in the database, but that's just off hand thought... Often when doing self signed certs I skip the CN but it isn't the worst requirement for us to keep either

seanknox commented 8 years ago

@helgi @bacongobbler another thought: does the CN column really need the not-null constraint? if there's a app dependency somewhere on CN then perhaps the controller can reject SSL certs with blank CNs; otherwise let it be null.

bacongobbler commented 8 years ago

I'm fairly certain that the router relies on the common name, so I'm not too sure it's a field that we can leave blank.

https://github.com/deis/controller/blob/780eb920c025e3726b8e2424845c51a810f51e9e/rootfs/api/tests/test_certificate_use_case_4.py#L50