Skip to content
This repository has been archived by the owner on Jul 30, 2024. It is now read-only.
/ NuGet.Jobs Public archive

Add cache control on blobs in public containers #663

Merged
merged 10 commits into from
Nov 9, 2018
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,14 @@ Task CopyValidationSetPackageToPackageFileAsync(
/// successive read and write operations.</exception>
Task<PackageStreamMetadata> UpdatePackageBlobMetadataAsync(PackageValidationSet validationSet);

/// <summary>
/// Updates package blob properties.
/// </summary>
/// <param name="validationSet">A validationSet that will identify the package that will have its blob metadata updated.</param>
/// <exception cref="Microsoft.WindowsAzure.Storage.StorageException">Thrown if the blob has changed between
/// successive read and write operations.</exception>
Task UpdatePackageBlobPropertiesAsync(PackageValidationSet validationSet);

/// <summary>
/// Reads the ETag for the package in the public container.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,16 @@ protected virtual async Task MakePackageAvailableAsync(IValidatingEntity<T> vali
// 1) Operate on blob storage.
var copied = await UpdatePublicPackageAsync(validationSet);

// 2) Update the package's blob metadata in the packages blob storage container.
// 2) Update the package's blob metadata in the public blob storage container.
var metadata = await _packageFileService.UpdatePackageBlobMetadataAsync(validationSet);

// 3) Operate on the database.
// 3) Update the package's blob properties in the public blob storage container.
await _packageFileService.UpdatePackageBlobPropertiesAsync(validationSet);

// 4) Operate on the database.
var fromStatus = await MarkPackageAsAvailableAsync(validationSet, validatingEntity, metadata, copied);

// 4) Emit telemetry and clean up.
// 5) Emit telemetry and clean up.
if (fromStatus != PackageStatus.Available)
{
_telemetryService.TrackPackageStatusChange(fromStatus, PackageStatus.Available);
Expand All @@ -113,7 +116,7 @@ protected virtual async Task MakePackageAvailableAsync(IValidatingEntity<T> vali
await _packageFileService.DeleteValidationPackageFileAsync(validationSet);
}

// 4) Verify the package still exists (we've had bugs here before).
// 5) Verify the package still exists (we've had bugs here before).
if (validatingEntity.Status == PackageStatus.Available
&& !await _packageFileService.DoesPackageFileExistAsync(validationSet))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,30 @@ public Task CopyPackageUrlForValidationSetAsync(PackageValidationSet validationS
AccessConditionWrapper.GenerateEmptyCondition());
}

public async Task UpdatePackageBlobPropertiesAsync(PackageValidationSet validationSet)
{
var fileName = BuildFileName(
validationSet,
_fileMetadataService.FileSavePathTemplate,
_fileMetadataService.FileExtension);

// This will throw if the ETag changes between read and write operations.
await _fileStorageService.SetPropertiesAsync(
_fileMetadataService.FileFolderName,
fileName,
async (lazyStream, blobProperties) =>
{
// Update the cache control only if the cache control is not the same as the default value.
if (!string.Equals(blobProperties.CacheControl, CoreConstants.DefaultCacheControl, StringComparison.OrdinalIgnoreCase))
{
blobProperties.CacheControl = CoreConstants.DefaultCacheControl;
return await Task.FromResult(true);
Copy link
Member

Choose a reason for hiding this comment

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

can't we just return true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The contract expects the delegate to return a task, it can be run synchronously but I don't like to have intellisense warnings.

}

return await Task.FromResult(false);
});
}

public async Task<PackageStreamMetadata> UpdatePackageBlobMetadataAsync(PackageValidationSet validationSet)
{
var fileName = BuildFileName(
Expand Down
2 changes: 1 addition & 1 deletion src/Validation.Common.Job/Validation.Common.Job.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@
<Version>2.33.0</Version>
</PackageReference>
<PackageReference Include="NuGetGallery.Core">
<Version>4.4.5-dev-2170472</Version>
<Version>4.4.5-dev-2193892</Version>
</PackageReference>
<PackageReference Include="Serilog">
<Version>2.5.0</Version>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,10 @@ public class SignatureFormatValidator : ISignatureFormatValidator
allowNoTimestamp: true,
allowUnknownRevocation: true,
reportUnknownRevocation: false,
allowNoRepositoryCertificateList: true,
allowNoClientCertificateList: true,
verificationTarget: VerificationTarget.All,
signaturePlacement: SignaturePlacement.PrimarySignature,
repositoryCountersignatureVerificationBehavior: SignatureVerificationBehavior.Never,
repoAllowListEntries: null,
clientAllowListEntries: null);
revocationMode: RevocationMode.Online);

private static readonly PackageSignatureVerifier _minimalVerifier = new PackageSignatureVerifier(new[]
{
Expand All @@ -40,7 +37,7 @@ public class SignatureFormatValidator : ISignatureFormatValidator
{
new IntegrityVerificationProvider(),
new SignatureTrustAndValidityVerificationProvider(),
new AllowListVerificationProvider(),
new AllowListVerificationProvider(allowList: null),
});

private readonly IOptionsSnapshot<ProcessSignatureConfiguration> _config;
Expand All @@ -61,13 +58,10 @@ public SignatureFormatValidator(IOptionsSnapshot<ProcessSignatureConfiguration>
allowNoTimestamp: false,
allowUnknownRevocation: true,
reportUnknownRevocation: true,
allowNoRepositoryCertificateList: true,
allowNoClientCertificateList: true,
verificationTarget: VerificationTarget.Author,
signaturePlacement: SignaturePlacement.PrimarySignature,
repositoryCountersignatureVerificationBehavior: SignatureVerificationBehavior.Never,
repoAllowListEntries: null,
clientAllowListEntries: null);
revocationMode: RevocationMode.Online);

var repoAllowListEntries = _config
.Value
Expand All @@ -90,13 +84,10 @@ public SignatureFormatValidator(IOptionsSnapshot<ProcessSignatureConfiguration>
allowNoTimestamp: _authorSignatureSettings.AllowNoTimestamp,
allowUnknownRevocation: _authorSignatureSettings.AllowUnknownRevocation,
reportUnknownRevocation: _authorSignatureSettings.ReportUnknownRevocation,
allowNoRepositoryCertificateList: false,
allowNoClientCertificateList: _authorSignatureSettings.AllowNoClientCertificateList,
verificationTarget: VerificationTarget.Repository,
signaturePlacement: SignaturePlacement.Any,
repositoryCountersignatureVerificationBehavior: SignatureVerificationBehavior.IfExists,
repoAllowListEntries: repoAllowListEntries,
clientAllowListEntries: _authorSignatureSettings.ClientCertificateList);
revocationMode: _authorSignatureSettings.RevocationMode);

_authorOrRepositorySignatureSettings = new SignedPackageVerifierSettings(
allowUnsigned: _authorSignatureSettings.AllowUnsigned,
Expand All @@ -107,13 +98,10 @@ public SignatureFormatValidator(IOptionsSnapshot<ProcessSignatureConfiguration>
allowNoTimestamp: _authorSignatureSettings.AllowNoTimestamp,
allowUnknownRevocation: _authorSignatureSettings.AllowUnknownRevocation,
reportUnknownRevocation: _authorSignatureSettings.ReportUnknownRevocation,
allowNoRepositoryCertificateList: false,
allowNoClientCertificateList: _authorSignatureSettings.AllowNoClientCertificateList,
verificationTarget: VerificationTarget.All,
signaturePlacement: SignaturePlacement.Any,
repositoryCountersignatureVerificationBehavior: SignatureVerificationBehavior.IfExists,
repoAllowListEntries: repoAllowListEntries,
clientAllowListEntries: _authorSignatureSettings.ClientCertificateList);
revocationMode: _authorSignatureSettings.RevocationMode);
}

public async Task<VerifySignaturesResult> ValidateMinimalAsync(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ private async Task<bool> IsValidRepositorySignatureAsync<T>(Context context, T s
// Strip repository signatures that do not pass verification.
var verifyResult = await _formatValidator.ValidateRepositorySignatureAsync(context.PackageReader, context.CancellationToken);

if (!verifyResult.Valid)
if (!verifyResult.IsValid)
{
var warningsForLogs = verifyResult
.Results
Expand Down Expand Up @@ -654,7 +654,7 @@ private async Task<SignatureValidatorResult> GetVerifyResult(
.Select(x => $"{x.Code}: {x.Message}")
.ToList();

if (!verifyResult.Valid)
if (!verifyResult.IsValid)
{
_logger.LogInformation(
"Signed package {PackageId} {PackageVersion} is blocked during {VerificationName} for validation " +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,12 @@ public ValidateAsync(ITestOutputHelper output)
_packageSigningStateService = new Mock<IPackageSigningStateService>();
_formatValidator = new Mock<ISignatureFormatValidator>();

_minimalVerifyResult = new VerifySignaturesResult(valid: true, signed: true);
_minimalVerifyResult = new VerifySignaturesResult(isValid: true, isSigned: true);
_formatValidator
.Setup(x => x.ValidateMinimalAsync(It.IsAny<ISignedPackageReader>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(() => _minimalVerifyResult);

_fullVerifyResult = new VerifySignaturesResult(valid: true, signed: true);
_fullVerifyResult = new VerifySignaturesResult(isValid: true, isSigned: true);
_formatValidator
.Setup(x => x.ValidateAllSignaturesAsync(It.IsAny<ISignedPackageReader>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(() => _fullVerifyResult);
Expand Down Expand Up @@ -276,7 +276,7 @@ public async Task RejectsSignedPackagesWithFailedMinimalVerifyResult()
{
// Arrange
_packageStream = TestResources.GetResourceStream(TestResources.SignedPackageLeaf1);
_minimalVerifyResult = new VerifySignaturesResult(valid: false, signed: true);
_minimalVerifyResult = new VerifySignaturesResult(isValid: false, isSigned: true);
_message = new SignatureValidationMessage(
TestResources.SignedPackageLeafId,
TestResources.SignedPackageLeaf1Version,
Expand Down Expand Up @@ -304,8 +304,8 @@ public async Task RejectsPackagesWithMimimalVerificationErrors()
// Arrange
_packageStream = TestResources.GetResourceStream(TestResources.SignedPackageLeaf1);
_minimalVerifyResult = new VerifySignaturesResult(
valid: false,
signed: true,
isValid: false,
isSigned: true,
results: new[]
{
new InvalidSignaturePackageVerificationResult(
Expand Down Expand Up @@ -349,7 +349,7 @@ public async Task RejectsSignedPackagesWithKnownCertificatesButFailedFullVerifyR
TestResources.SignedPackageLeafId,
TestResources.SignedPackageLeaf1Version,
TestResources.Leaf1Thumbprint);
_fullVerifyResult = new VerifySignaturesResult(valid: false, signed: true);
_fullVerifyResult = new VerifySignaturesResult(isValid: false, isSigned: true);
_message = new SignatureValidationMessage(
TestResources.SignedPackageLeafId,
TestResources.SignedPackageLeaf1Version,
Expand Down Expand Up @@ -379,8 +379,8 @@ public async Task RejectsPackagesWithFullVerificationErrors()
TestResources.SignedPackageLeaf1Version,
TestResources.Leaf1Thumbprint);
_fullVerifyResult = new VerifySignaturesResult(
valid: false,
signed: true,
isValid: false,
isSigned: true,
results: new[]
{
new InvalidSignaturePackageVerificationResult(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,21 @@ public class SignatureValidatorIntegrationTests : IDisposable
private const string AuthorPrimaryCertificateUntrustedMessage = "The author primary signature found a chain building issue: " +
"A certificate chain processed, but terminated in a root certificate which is not trusted by the trust provider.";
private const string AuthorPrimaryCertificateRevocationOfflineMessage = "NU3018: The author primary signature found a chain building issue: " +
"The revocation function was unable to check revocation because the revocation server was offline.";
"The revocation function was unable to check revocation because the revocation server could not be reached.";
private const string AuthorPrimaryCertificateRevocationUnknownMessage = "NU3018: The author primary signature found a chain building issue: " +
"The revocation function was unable to check revocation for the certificate.";
private const string RepositoryCounterCertificateRevocationOfflineMessage = "NU3018: The repository countersignature found a chain building issue: " +
"The revocation function was unable to check revocation because the revocation server was offline.";
"The revocation function was unable to check revocation because the revocation server could not be reached.";
private const string RepositoryCounterCertificateRevocationUnknownMessage = "NU3018: The repository countersignature found a chain building issue: " +
"The revocation function was unable to check revocation for the certificate.";

// NU3028
// Should be NU3028 => TODO: https://github.com/NuGet/Engineering/issues/1891
private const string AuthorPrimaryTimestampCertificateUntrustedMessage = "The author primary signature's timestamp found a chain building issue: " +
"A certificate chain processed, but terminated in a root certificate which is not trusted by the trust provider.";
private const string AuthorPrimaryTimestampCertificateRevocationUnknownMessage = "NU3028: The author primary signature's timestamp found a chain building issue: " +
private const string AuthorPrimaryTimestampCertificateRevocationUnknownMessage = "NU3018: The author primary signature's timestamp found a chain building issue: " +
"The revocation function was unable to check revocation for the certificate.";
private const string AuthorPrimaryTimestampCertificateRevocationOfflineMessage = "NU3028: The author primary signature's timestamp found a chain building issue: " +
"The revocation function was unable to check revocation because the revocation server was offline.";
private const string AuthorPrimaryTimestampCertificateRevocationOfflineMessage = "NU3018: The author primary signature's timestamp found a chain building issue: " +
"The revocation function was unable to check revocation because the revocation server could not be reached.";

private readonly CertificateIntegrationTestFixture _fixture;
private readonly ITestOutputHelper _output;
Expand Down Expand Up @@ -307,6 +307,7 @@ await _fixture.GetSigningCertificateAsync(),
}

[Fact]
// TODO: https://github.com/NuGet/Engineering/issues/1891
public async Task AcceptsTrustedTimestampingCertificateWithUnavailableRevocation()
{
// Arrange
Expand Down Expand Up @@ -360,6 +361,7 @@ await _fixture.GetSigningCertificateAsync(),
}

[Fact]
// TODO: https://github.com/NuGet/Engineering/issues/1891
public async Task AcceptsTrustedSigningCertificateWithUnavailableRevocation()
{
// Arrange
Expand Down Expand Up @@ -1449,6 +1451,7 @@ await _fixture.GetSigningCertificateAsync(),
}

[Fact]
// TODO: https://github.com/NuGet/Engineering/issues/1891
public async Task WhenRepositoryCounterSigned_AcceptsTrustedTimestampingCertificateWithUnavailableRevocation()
{
// Arrange
Expand Down Expand Up @@ -1508,6 +1511,7 @@ await _fixture.GetSigningCertificateAsync(),
}

[Fact]
// TODO: https://github.com/NuGet/Engineering/issues/1891
public async Task WhenRepositoryCounterSigned_AcceptsTrustedSigningCertificateWithUnavailableRevocation()
{
// Arrange
Expand Down