failed x509 subject_key_identify validation does not seem to be trapped for signed commits
Summary
A customer raised a ticket to report that a git repository was malfunctioning after a developer signed a commit with their x509 smart card. (GitLab team members can read more in the ticket)
Steps to reproduce
To follow.
It looks like the following code is tripping (x509_issuer.rb) - similar validation is done in x509_certificate.rb as well.
# rfc 5280 - 4.2.1.1 Authority Key Identifier
validates :subject_key_identifier, presence: true, format: { with: /\A(\h{2}:){19}\h{2}\z/ }
Suggesting that this field is missing or contains something that does not look like:
X509v3 Subject Key Identifier:
B9:09:CA:9C:1E:DB:D3:6C:3A:6B:AE:ED:54:F1:5B:93:06:35:2E:5E
The customer has attached their root and and an intermediate (signing) cert to the ticket. Additionally, on the call, we looked at one of their client certificates. ALL have a much smaller SKI.
X509v3 Subject Key Identifier:
B9:09:CA:9C:1E:DB:D3:6C
Handling the exception nicely (this issue) and supporting the reason for the exception are two completely different things, so #332503 (closed) is for the SKI problem.
Example Project
What is the current bug behavior?
The back end does not handle the validation failure, so the front end does not get the data it needs to render the
- Part of the project home page does not render:
- 'an error occurred while loading commit signatures' in the commits list (
group/project/-/commits/master
)
- the commit cannot be viewed
What is the expected correct behavior?
Trap the validation failure, and handle it.
If the signed commit cannot be validated, the front end should get the data it needs to
- Render the repository as usual.
- Show problematic commits as unvalidated
- Pass up information to help troubleshoot why not.
Relevant logs and/or screenshots
There's a stack trace on the back end:
ActiveRecord::RecordInvalid (Validation failed: Subject key identifier is invalid):
app/models/application_record.rb:49:in `block in safe_find_or_create_by!'
app/models/application_record.rb:46:in `tap'
app/models/application_record.rb:46:in `safe_find_or_create_by!'
lib/gitlab/metrics/instrumentation.rb:160:in `block in safe_find_or_create_by!'
lib/gitlab/metrics/method_call.rb:27:in `measure'
lib/gitlab/metrics/instrumentation.rb:160:in `safe_find_or_create_by!'
app/models/x509_issuer.rb:16:in `safe_create!'
lib/gitlab/metrics/instrumentation.rb:160:in `block in safe_create!'
lib/gitlab/metrics/method_call.rb:27:in `measure'
lib/gitlab/metrics/instrumentation.rb:160:in `safe_create!'
lib/gitlab/x509/signature.rb:192:in `x509_issuer'
lib/gitlab/x509/signature.rb:196:in `certificate_attributes'
lib/gitlab/x509/signature.rb:20:in `x509_certificate'
lib/gitlab/x509/commit.rb:39:in `attributes'
lib/gitlab/x509/commit.rb:50:in `create_cached_signature!'
lib/gitlab/x509/commit.rb:16:in `signature'
app/models/commit.rb:365:in `block in signature'
lib/gitlab/utils/strong_memoize.rb:30:in `strong_memoize'
app/models/commit.rb:360:in `signature'
app/presenters/commit_presenter.rb:25:in `signature_html'
lib/gitlab/graphql/present/field_extension.rb:17:in `resolve'
lib/gitlab/graphql/generic_tracing.rb:40:in `with_labkit_tracing'
lib/gitlab/graphql/generic_tracing.rb:30:in `platform_trace'
lib/gitlab/graphql/generic_tracing.rb:40:in `with_labkit_tracing'
lib/gitlab/graphql/generic_tracing.rb:30:in `platform_trace'
app/graphql/gitlab_schema.rb:41:in `multiplex'
app/graphql/gitlab_schema.rb:48:in `execute'
app/controllers/graphql_controller.rb:94:in `execute_query'
app/controllers/graphql_controller.rb:37:in `execute'
ee/lib/gitlab/ip_address_state.rb:10:in `with'
ee/app/controllers/ee/application_controller.rb:44:in `set_current_ip_address'
app/controllers/application_controller.rb:485:in `set_current_admin'
lib/gitlab/session.rb:11:in `with_session'
app/controllers/application_controller.rb:476:in `set_session_storage'
lib/gitlab/i18n.rb:73:in `with_locale'
lib/gitlab/i18n.rb:79:in `with_user_locale'
app/controllers/application_controller.rb:470:in `set_locale'
app/controllers/application_controller.rb:463:in `block in set_current_context'
lib/gitlab/application_context.rb:70:in `block in use'
lib/gitlab/application_context.rb:70:in `use'
lib/gitlab/application_context.rb:27:in `with_context'
app/controllers/application_controller.rb:454:in `set_current_context'
lib/gitlab/metrics/elasticsearch_rack_middleware.rb:16:in `call'
lib/gitlab/middleware/rails_queue_duration.rb:33:in `call'
lib/gitlab/metrics/rack_middleware.rb:16:in `block in call'
lib/gitlab/metrics/transaction.rb:56:in `run'
lib/gitlab/metrics/rack_middleware.rb:16:in `call'
lib/gitlab/request_profiler/middleware.rb:17:in `call'
lib/gitlab/jira/middleware.rb:19:in `call'
lib/gitlab/middleware/go.rb:20:in `call'
lib/gitlab/etag_caching/middleware.rb:21:in `call'
lib/gitlab/middleware/multipart.rb:172:in `call'
lib/gitlab/middleware/read_only/controller.rb:50:in `call'
lib/gitlab/middleware/read_only.rb:18:in `call'
lib/gitlab/middleware/same_site_cookies.rb:27:in `call'
lib/gitlab/middleware/handle_malformed_strings.rb:21:in `call'
lib/gitlab/middleware/basic_health_check.rb:25:in `call'
lib/gitlab/middleware/handle_ip_spoof_attack_error.rb:25:in `call'
lib/gitlab/middleware/request_context.rb:21:in `call'
config/initializers/fix_local_cache_middleware.rb:11:in `call'
lib/gitlab/middleware/rack_multipart_tempfile_factory.rb:21:in `call'
lib/gitlab/metrics/requests_rack_middleware.rb:76:in `call'
lib/gitlab/middleware/release_env.rb:12:in `call'
Output of checks
Self Managed 13.11.4