Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Next Next commit
Improve dev-certs export error message
During a recent security review of the dev-certs tool, we observed that on export it would create a directory that was potentially world-readable (e.g. based on permissions inherited from the parent directory).  We decided it would be more appropriate to let users make the decision of who should have access to the directory.  Unfortunately, this removal of functionality broke some app authors' workflows.  When dev-certs is run directly, the `--verbose` output makes it clear what went wrong and what needs to happen, but the non-verbose output that appears when another tool does the export is less helpful.  This change introduces a new top-level error state for an export failure caused by a non-existent target directory to make it clearer how to fix broken workflows.

The behavior changed in #57108, which included a backport of #56985, and shipped in 8.0.10.

For #58330
  • Loading branch information
amcasey authored and github-actions committed Oct 16, 2024
commit 4d6a9b241c98b3c65ff96902bb2f0b31a1e43f2c
1 change: 1 addition & 0 deletions src/Shared/CertificateGeneration/CertificateManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ public EnsureCertificateResult EnsureAspNetCoreHttpsDevelopmentCertificate(
var exportDir = Path.GetDirectoryName(path);
if (!string.IsNullOrEmpty(exportDir) && !Directory.Exists(exportDir))
{
result = EnsureCertificateResult.ErrorExportingTheCertificateToNonExistentDirectory;
throw new InvalidOperationException($"The directory '{exportDir}' does not exist. Choose permissions carefully when creating it.");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ internal enum EnsureCertificateResult
ErrorCreatingTheCertificate,
ErrorSavingTheCertificateIntoTheCurrentUserPersonalStore,
ErrorExportingTheCertificate,
ErrorExportingTheCertificateToNonExistentDirectory,
FailedToTrustTheCertificate,
PartiallyFailedToTrustTheCertificate,
UserCancelledTrustStep,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ public void EnsureCreateHttpsCertificate_CannotExportToNonExistentDirectory()
.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), Path.Combine("NoSuchDirectory", CertificateName));

// Assert
Assert.Equal(EnsureCertificateResult.ErrorExportingTheCertificate, result);
Assert.Equal(EnsureCertificateResult.ErrorExportingTheCertificateToNonExistentDirectory, result);
}

[Fact]
Expand Down
4 changes: 4 additions & 0 deletions src/Tools/dotnet-dev-certs/src/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,10 @@ private static int EnsureHttpsCertificate(CommandOption exportPath, CommandOptio
case EnsureCertificateResult.ErrorExportingTheCertificate:
reporter.Warn("There was an error exporting the HTTPS developer certificate to a file.");
return ErrorExportingTheCertificate;
case EnsureCertificateResult.ErrorExportingTheCertificateToNonExistentDirectory:
// A distinct warning is useful, but a distinct error code is probably not.
reporter.Warn("There was an error exporting the HTTPS developer certificate to a file. Please create the target directory before exporting.");
Comment thread
amcasey marked this conversation as resolved.
Outdated
return ErrorExportingTheCertificate;
case EnsureCertificateResult.PartiallyFailedToTrustTheCertificate:
// A distinct warning is useful, but a distinct error code is probably not.
reporter.Warn("There was an error trusting the HTTPS developer certificate. It will be trusted by some clients but not by others.");
Expand Down