Skip to content

Add self signed - #1474

Open
synfinatic wants to merge 3 commits into
mainfrom
add-self-signed
Open

Add self signed#1474
synfinatic wants to merge 3 commits into
mainfrom
add-self-signed

Conversation

@synfinatic

Copy link
Copy Markdown
Owner

No description provided.

The ECS Server's TLS story was a dead end: docs told users the AWS SDKs
"do NOT support self-signed certificates or private CA's," and
`setup ecs ssl` only accepted a certificate from a public CA -- which no
public CA will issue for localhost/loopback/169.254.170.2.

Tracing the actual SDK sources shows this is stale for every SDK except
Python/botocore: Go SDK v2, Node, Java, and .NET all honor the OS trust
store (or NODE_EXTRA_CA_CERTS for Node), so a private CA trusted once per
machine/runtime makes ECS Server TLS work everywhere except the AWS CLI.
botocore's container-credentials fetcher hardcodes verification against
certifi/its own vendored bundle, ignoring both AWS_CA_BUNDLE and the OS
trust store (tracked upstream: aws/aws-sdk#9016) -- the trust instructions
call this out explicitly with an installation-method-agnostic workaround.

This adds `--self-signed`, which generates a local CA + leaf certificate
via a new internal/certutil package, stores both in the SecureStore
(mirroring the existing leaf-only storage across the keyring, 1Password,
and JSON backends), and prints per-runtime trust instructions. The CA
private key is retained and reused across reruns, so trust only needs to
be established once per machine -- rerunning `--self-signed` rotates only
the short-lived leaf. `--print-ca` re-displays the instructions on a new
machine without regenerating anything, and `--rotate-ca` forces a fresh
CA when actually needed.
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.38756% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.39%. Comparing base (45c0517) to head (f04169f).

Files with missing lines Patch % Lines
internal/certutil/certutil.go 85.33% 8 Missing and 8 partials ⚠️
cmd/aws-sso/setup_cmd.go 95.46% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1474      +/-   ##
==========================================
+ Coverage   86.04%   86.39%   +0.35%     
==========================================
  Files          77       78       +1     
  Lines        4971     5157     +186     
==========================================
+ Hits         4277     4455     +178     
- Misses        470      473       +3     
- Partials      224      229       +5     
Flag Coverage Δ
unittests 86.39% <91.39%> (+0.35%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cmd/aws-sso/main.go 60.59% <ø> (-0.68%) ⬇️
internal/storage/json_store.go 94.00% <100.00%> (+0.82%) ⬆️
internal/storage/keyring.go 94.10% <100.00%> (+0.40%) ⬆️
internal/storage/onepassword.go 96.54% <100.00%> (+0.30%) ⬆️
internal/storage/storage.go 95.00% <100.00%> (+0.27%) ⬆️
cmd/aws-sso/setup_cmd.go 96.83% <95.46%> (+22.47%) ⬆️
internal/certutil/certutil.go 85.33% <85.33%> (ø)

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 45c0517...f04169f. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It introduces security-sensitive private CA key generation/storage across all backends and a breaking CLI change, which warrant final human review despite only minor grammar/spelling issues found.

Pull request overview

This PR replaces the old file-based setup ecs ssl workflow (which required the user to supply an externally-signed certificate/key via --certificate/--private-key) with a self-managed, self-signed private CA. Running aws-sso setup ecs ssl --self-signed now generates a long-lived local CA and a short-lived leaf certificate (covering localhost, 127.0.0.1, ::1, and the ECS credentials IP 169.254.170.2), stores both in the SecureStore, exports the CA cert to ~/.aws-sso/ecs-ca.pem, and prints per-runtime trust instructions. This fits into the existing internal/ecs server TLS story by supplying the leaf cert/key the server already reads from the secure store.

Changes:

  • New internal/certutil package for CA/leaf ECDSA certificate generation and SHA-256 fingerprinting.
  • Extended the SecureStorage interface (and all three backends: keyring, 1Password, JSON) with Save/Get/Delete methods for the CA key pair.
  • Reworked setup ecs ssl into --self-signed/--print-ca/--print/--delete, removed --certificate/--private-key/--force, and added trust-instruction output plus extensive doc/CHANGELOG updates.
File summaries
File Description
internal/certutil/certutil.go New CA/leaf cert generation and fingerprint helpers
internal/certutil/certutil_test.go Tests for CA/leaf generation, SANs, chain-of-trust, fingerprint
internal/storage/secure_store.go Adds CA key-pair methods to the interface
internal/storage/keyring.go Adds CA fields to StorageData and CA methods
internal/storage/onepassword.go Adds CA methods to the 1Password backend
internal/storage/json_store.go Adds CA fields/methods to the JSON backend
internal/storage/*_test.go Backend tests for CA key-pair round-trips
cmd/aws-sso/setup_cmd.go Reworks setup ecs ssl to the self-signed CA flow
cmd/aws-sso/setup_cmd_ecs_ssl_trust.go New per-runtime trust-instruction text
cmd/aws-sso/setup_cmd_test.go / _e2e_test.go Unit/e2e tests for the new flow
cmd/aws-sso/main.go Removes the now-unused add-ssl kong group
docs/*.md Documents the self-signed workflow and Python/AWS CLI caveat
CHANGELOG.md Records the new feature and breaking removal
CLAUDE.md New contributor/architecture guide
Review details
  • Files reviewed: 20/20 changed files
  • Comments generated: 3
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/ecs-server.md Outdated
Comment thread CLAUDE.md Outdated
Comment thread CLAUDE.md Outdated
@synfinatic
synfinatic force-pushed the add-self-signed branch 4 times, most recently from 7a5006c to ae3c4f2 Compare September 1, 2026 04:59
Remove flags which allow user to import a signed cert/key from
another CA.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It introduces security-sensitive CA-generation code intended to be installed into users' OS/runtime trust stores and makes a breaking CLI change, so it warrants human review despite only a minor markdownlint issue being found.

Review details
  • Files reviewed: 21/21 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread docs/ecs-commands.md
Flags:

Flags:
* `--self-signed` -- Generate (or reuse) a local CA and issue a new leaf certificate for the ECS Server (recommended)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants