Skip to content

Fix local certificate issuance on LibreSSL #113

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.

Sign up for GitHub

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

Open
dajiaohuang wants to merge 1 commit into
base: main
Choose a base branch
from
+50 −8
Open

Fix local certificate issuance on LibreSSL #113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions Dory/Net/LocalCA.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ nonisolated enum ShellError: Error, Sendable {
case launchFailed(String)
case nonZeroExit(Int32, String)
case toolNotFound(String)
case invalidArgument(String)
}

nonisolated enum Shell {
Expand Down Expand Up @@ -112,11 +113,18 @@ nonisolated struct LocalCA: Sendable {
@discardableResult
func issue(domain: String, extraSANs: [String] = []) throws -> CertificatePair {
guard let openssl = opensslPath else { throw ShellError.toolNotFound("openssl") }
try Self.validateCertificateName(domain)
for name in extraSANs where !name.isEmpty { try Self.validateCertificateName(name) }
try ensureCA()
let certificate = directory.appendingPathComponent("\(domain).crt")
let key = directory.appendingPathComponent("\(domain).key")
let csr = directory.appendingPathComponent("\(domain).csr")
defer { try? FileManager.default.removeItem(at: csr) }
let safeName = domain.replacingOccurrences(of: "/", with: "_")
let certificate = directory.appendingPathComponent("\(safeName).crt")
let key = directory.appendingPathComponent("\(safeName).key")
let csr = directory.appendingPathComponent("\(safeName).csr")
let extensionsFile = directory.appendingPathComponent("\(safeName).extensions-\(UUID().uuidString).cnf")
defer {
try? FileManager.default.removeItem(at: csr)
try? FileManager.default.removeItem(at: extensionsFile)
}

// TLS wildcards match a single label, so a `*.dory.local` cert does NOT cover multi-level
// names like `web.default.k8s.dory.local`. Callers pass those explicitly as extra SANs.
Expand All @@ -127,9 +135,11 @@ nonisolated struct LocalCA: Sendable {
"-keyout", key.path, "-out", csr.path, "-subj", "/CN=\(domain)",
"-addext", san,
])
try Data("[dory_leaf]\n\(san)\n".utf8).write(to: extensionsFile)
try Shell.run(openssl, [
"x509", "-req", "-in", csr.path, "-CA", caCertificate.path, "-CAkey", caKey.path,
"-CAcreateserial", "-out", certificate.path, "-days", "825", "-copy_extensions", "copyall",
"-CAcreateserial", "-out", certificate.path, "-days", "825",
"-extfile", extensionsFile.path, "-extensions", "dory_leaf",
])
return CertificatePair(certificate: certificate, privateKey: key)
}
Expand Down Expand Up @@ -160,4 +170,17 @@ nonisolated struct LocalCA: Sendable {
return try Shell.run(openssl, ["x509", "-in", certificate.path, "-noout", "-text"])
}

private static func validateCertificateName(_ name: String) throws {
var value = name
if value.hasPrefix("*.") { value = String(value.dropFirst(2)) }
let labels = value.split(separator: ".", omittingEmptySubsequences: false)
guard !labels.isEmpty else { throw ShellError.invalidArgument("certificate name: \(name)") }
for label in labels {
guard !label.isEmpty,
label.allSatisfy({ $0.isASCII && ($0.isLetter || $0.isNumber || $0 == "-") }) else {
throw ShellError.invalidArgument("certificate name: \(name)")
}
}
}

}
15 changes: 14 additions & 1 deletion DoryTests/LocalCATests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ struct LocalCATests {
#expect(ca.caExists)
#expect(FileManager.default.fileExists(atPath: ca.caCertificate.path))

let pair = try ca.issue(domain: "web.dory.local")
let pair = try ca.issue(domain: "web.dory.local", extraSANs: ["api.dory.local"])
#expect(FileManager.default.fileExists(atPath: pair.certificate.path))
#expect(FileManager.default.fileExists(atPath: pair.privateKey.path))

Expand All @@ -25,9 +25,22 @@ struct LocalCATests {
// The SAN must include the requested domain.
let text = try ca.certificateText(pair.certificate)
#expect(text.contains("web.dory.local"))
#expect(text.contains("api.dory.local"))
#expect(text.contains("Dory Local CA"))
}

@Test func rejectsCertificateNamesThatCouldInjectOpenSSLConfiguration() throws {
let directory = URL(fileURLWithPath: NSTemporaryDirectory())
.appendingPathComponent("dory-ca-invalid-name-\(UUID().uuidString)")
defer { try? FileManager.default.removeItem(at: directory) }

let ca = LocalCA(directory: directory)
guard ca.opensslPath != nil else { return }
#expect(throws: ShellError.self) {
try ca.issue(domain: "web.dory.local\n[attacker]")
}
}

@Test func localTrustParserAcceptsOnlyAValidDoryCA() throws {
let directory = URL(fileURLWithPath: NSTemporaryDirectory())
.appendingPathComponent("dory-trust-parser-\(UUID().uuidString)")
Expand Down
10 changes: 8 additions & 2 deletions dory-core-swift/Sources/DorydKit/LocalCA.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,11 @@ public struct DoryLocalCA {
let certificate = directory.appendingPathComponent("\(safeName).crt")
let key = directory.appendingPathComponent("\(safeName).key")
let csr = directory.appendingPathComponent("\(safeName).csr")
defer { try? fileManager.removeItem(at: csr) }
let extensionsFile = directory.appendingPathComponent("\(safeName).extensions-\(UUID().uuidString).cnf")
defer {
try? fileManager.removeItem(at: csr)
try? fileManager.removeItem(at: extensionsFile)
}

var san = "subjectAltName=DNS:\(domain),DNS:*.\(domain)"
for name in extraSANs where !name.isEmpty {
Expand All @@ -147,9 +151,11 @@ public struct DoryLocalCA {
"-keyout", key.path, "-out", csr.path, "-subj", "/CN=\(domain)",
"-addext", san,
])
try Data("[dory_leaf]\n\(san)\n".utf8).write(to: extensionsFile)
try DoryShell.run(openssl, [
"x509", "-req", "-in", csr.path, "-CA", caCertificate.path, "-CAkey", caKey.path,
"-CAcreateserial", "-out", certificate.path, "-days", "825", "-copy_extensions", "copyall",
"-CAcreateserial", "-out", certificate.path, "-days", "825",
"-extfile", extensionsFile.path, "-extensions", "dory_leaf",
])
try fileManager.setAttributes([.posixPermissions: 0o600], ofItemAtPath: key.path)
try fileManager.setAttributes([.posixPermissions: 0o644], ofItemAtPath: certificate.path)
Expand Down