Skip to content

Commit

Permalink
Improve dev-certs export error message
Browse files Browse the repository at this point in the history
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 dotnet#57108, which included a backport of dotnet#56985, and shipped in 8.0.10.

For dotnet#58330
  • Loading branch information
amcasey committed Oct 16, 2024
1 parent d624755 commit b77ca75
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 1 deletion.
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.");
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

0 comments on commit b77ca75

Please sign in to comment.