-
Notifications
You must be signed in to change notification settings - Fork 22
Validate signature hashes #1474
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.
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
Merged
Merged
Changes from 36 commits
Commits
Show all changes
49 commits
Select commit
Hold shift + click to select a range
d6e3573
Add signature hash validator.
bjorntore eb88444
Use service owner auth if the data type is restricted.
bjorntore 9bd750d
Add claudes tests.
bjorntore a07d326
Test tweaks.
bjorntore 56f7d1d
Nitpick
bjorntore bbe8741
Remove settings.local.json.
bjorntore 82c037e
Actually stream the data.
bjorntore de6ff8a
Dispose stream.
bjorntore 94b2139
Adjust test.
bjorntore 7f6779c
Some cleanup. Extract method for validation logic. Never return null …
bjorntore eab540d
Add remark that the sha digest formatting needs to match Storage.
bjorntore 26ae23e
Test that exception from underlying code in ShouldRunForTask bubbles up.
bjorntore 967d18b
Formatting that got past rider.
bjorntore 0e30e33
Add tests for GetBinaryDataStream.
bjorntore 933a50a
Rename GetUnbufferedAsync to GetStreamingAsync to avoid confusion. Ad…
bjorntore 5be7cb8
Adjust cancellation token param.
bjorntore a91dc2f
Add correct verified.txt.
bjorntore 2f69711
Merge branch 'main' into feat/signature-validation
bjorntore 0b829ab
Add ResponseWrapperStream to ensure that the http response is dispose…
bjorntore 7db9c25
Add async overrides to ResponseWrapperStream.
bjorntore a72fe6e
Create platform exception before disposing response.
bjorntore c1c510c
Null checks in ResponseWrapperStream.
bjorntore 24bebc9
PlatformHttpResponseSnapshotException first edition.
bjorntore 7ac2d12
PlatformHttpResponseSnapshotException that inherits from PlatformHttp…
bjorntore 9f2b786
Tweaks to PlatformHttpResponseSnapshowException.
bjorntore 94199df
Use a separate streaming http client with longer timeout when streami…
bjorntore 98e62ea
Make PlatformHttpResponseSnapshotException internal.
bjorntore 8413f59
Don't default to any content type in PlatformHttpResponseSnapshotExce…
bjorntore c8d6d21
Redact sensitive headers from PlatformHttpResponseSnapshotException.
bjorntore 5f17178
PlatformHttpResponseSnapshotException: Snapshot content with bounded …
bjorntore cb044ed
Various ai suggestions.
bjorntore 5239d47
More ai suggested disposing in DataClienTests.cs.
bjorntore ab6aed7
Merge branch 'main' into feat/signature-validation
bjorntore cba78ae
Remove this unnecessary check for null.
bjorntore c2530c7
Merge branch 'main' into feat/signature-validation
bjorntore 841451f
Base timeout in DataClient on cancellation token instead of HttpClien…
bjorntore c15edb5
Remove unused streaming http client.
bjorntore 412ad8e
remove more streaming client stuff
bjorntore 6370f20
Set hard coded max of 30 minute timeout on streaming.
bjorntore 7018d57
Add cancellation token to methods in HttpClientExtension.
bjorntore 84b31ae
Merge branch 'main' into feat/signature-validation
bjorntore ce1bceb
Use the http context cancellation token in SignatureHashValidator.
bjorntore 9fa8624
Remove unused org and app params from IDataCient.GetBinaryDataStream.
bjorntore 9fa048d
Make cancellation token pattern in DataClient slightly less noisy by …
bjorntore dbc85c4
Revert to having a dedicated streaming http client to avoid merge con…
bjorntore 8c2aa6c
Re-add named param for cancellation token.
bjorntore c32f408
Formatting.
bjorntore 8320792
Merge branch 'main' into feat/signature-validation
bjorntore 37c6082
Attempt to use CustomTextKey.
bjorntore File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
178 changes: 178 additions & 0 deletions
178
src/Altinn.App.Core/Features/Validation/Default/SignatureHashValidator.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,178 @@ | ||
| using System.Diagnostics; | ||
| using System.Security.Cryptography; | ||
| using Altinn.App.Core.Features.Signing.Models; | ||
| using Altinn.App.Core.Features.Signing.Services; | ||
| using Altinn.App.Core.Internal.App; | ||
| using Altinn.App.Core.Internal.Data; | ||
| using Altinn.App.Core.Internal.Process; | ||
| using Altinn.App.Core.Internal.Process.Elements.AltinnExtensionProperties; | ||
| using Altinn.App.Core.Models; | ||
| using Altinn.App.Core.Models.Validation; | ||
| using Altinn.Platform.Storage.Interface.Models; | ||
| using Microsoft.Extensions.Logging; | ||
|
|
||
| namespace Altinn.App.Core.Features.Validation.Default; | ||
|
|
||
| /// <summary> | ||
| /// Validates that signature hashes are still valid. | ||
| /// </summary> | ||
| internal sealed class SignatureHashValidator( | ||
| ISigningService signingService, | ||
| IProcessReader processReader, | ||
| IDataClient dataClient, | ||
| IAppMetadata appMetadata, | ||
| ILogger<SignatureHashValidator> logger | ||
| ) : IValidator | ||
| { | ||
| private const string SigningTaskType = "signing"; | ||
|
|
||
| /// <summary> | ||
| /// We implement <see cref="ShouldRunForTask"/> instead. | ||
| /// </summary> | ||
| public string TaskId => "*"; | ||
|
|
||
| /// <summary> | ||
| /// Only runs for tasks that are of type "signing". | ||
| /// </summary> | ||
| public bool ShouldRunForTask(string taskId) | ||
| { | ||
| AltinnTaskExtension? taskConfig = processReader.GetAltinnTaskExtension(taskId); | ||
| return taskConfig?.TaskType is SigningTaskType; | ||
| } | ||
|
|
||
| public bool NoIncrementalValidation => true; | ||
|
|
||
| /// <inheritdoc /> | ||
| public Task<bool> HasRelevantChanges(IInstanceDataAccessor dataAccessor, string taskId, DataElementChanges changes) | ||
| { | ||
| throw new UnreachableException( | ||
| "HasRelevantChanges should not be called because NoIncrementalValidation is true." | ||
| ); | ||
| } | ||
|
|
||
| public async Task<List<ValidationIssue>> Validate( | ||
| IInstanceDataAccessor dataAccessor, | ||
| string taskId, | ||
| string? language | ||
| ) | ||
| { | ||
| Instance instance = dataAccessor.Instance; | ||
|
|
||
| AltinnSignatureConfiguration signingConfiguration = | ||
| processReader.GetAltinnTaskExtension(taskId)?.SignatureConfiguration | ||
| ?? throw new ApplicationConfigException("Signing configuration not found in AltinnTaskExtension"); | ||
|
|
||
| ApplicationMetadata applicationMetadata = await appMetadata.GetApplicationMetadata(); | ||
|
|
||
| List<SigneeContext> signeeContextsResults = await signingService.GetSigneeContexts( | ||
| dataAccessor, | ||
| signingConfiguration, | ||
| CancellationToken.None | ||
| ); | ||
|
|
||
| foreach (SigneeContext signeeContext in signeeContextsResults) | ||
| { | ||
| List<SignDocument.DataElementSignature> dataElementSignatures = | ||
| signeeContext.SignDocument?.DataElementSignatures ?? []; | ||
|
|
||
| foreach (SignDocument.DataElementSignature dataElementSignature in dataElementSignatures) | ||
| { | ||
| ValidationIssue? validationIssue = await ValidateDataElementSignature( | ||
| dataElementSignature, | ||
| instance, | ||
| applicationMetadata | ||
| ); | ||
|
|
||
| if (validationIssue != null) | ||
| { | ||
| return [validationIssue]; | ||
| } | ||
| } | ||
bjorntore marked this conversation as resolved.
Dismissed
Show dismissed
Hide dismissed
|
||
| } | ||
|
|
||
| logger.LogInformation("All signature hashes are valid for instance {InstanceId}", instance.Id); | ||
|
|
||
| return []; | ||
| } | ||
|
|
||
| private async Task<ValidationIssue?> ValidateDataElementSignature( | ||
| SignDocument.DataElementSignature dataElementSignature, | ||
| Instance instance, | ||
| ApplicationMetadata applicationMetadata | ||
| ) | ||
| { | ||
| var instanceIdentifier = new InstanceIdentifier(instance); | ||
|
|
||
| await using Stream dataStream = await dataClient.GetBinaryDataStream( | ||
| instance.Org, | ||
| instance.AppId, | ||
| instanceIdentifier.InstanceOwnerPartyId, | ||
| instanceIdentifier.InstanceGuid, | ||
| Guid.Parse(dataElementSignature.DataElementId), | ||
| HasRestrictedRead(applicationMetadata, instance, dataElementSignature.DataElementId) | ||
| ? StorageAuthenticationMethod.ServiceOwner() | ||
| : null | ||
| ); | ||
|
|
||
| string sha256Hash = await GenerateSha256Hash(dataStream); | ||
|
|
||
| if (sha256Hash != dataElementSignature.Sha256Hash) | ||
| { | ||
| logger.LogError( | ||
| "Found an invalid signature for data element {DataElementId} on instance {InstanceId}. Expected hash {ExpectedHash}, calculated hash {CalculatedHash}", | ||
| dataElementSignature.DataElementId, | ||
| instance.Id, | ||
| dataElementSignature.Sha256Hash, | ||
| sha256Hash | ||
| ); | ||
|
|
||
| return new ValidationIssue | ||
| { | ||
| Code = ValidationIssueCodes.DataElementCodes.InvalidSignatureHash, | ||
| Severity = ValidationIssueSeverity.Error, | ||
| Description = ValidationIssueCodes.DataElementCodes.InvalidSignatureHash, | ||
bjorntore marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| }; | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| private static bool HasRestrictedRead( | ||
| ApplicationMetadata applicationMetadata, | ||
| Instance instance, | ||
| string dataElementId | ||
| ) | ||
| { | ||
| DataElement? dataElement = instance.Data.FirstOrDefault(de => de.Id == dataElementId); | ||
| string? dataTypeId = dataElement?.DataType; | ||
| DataType? dataType = applicationMetadata.DataTypes.FirstOrDefault(dt => dt.Id == dataTypeId); | ||
|
|
||
| if (dataType == null) | ||
| { | ||
| throw new ApplicationConfigException( | ||
| $"Unable to find data type {dataTypeId} for data element {dataElementId} in applicationmetadata.json." | ||
| ); | ||
| } | ||
|
|
||
| return !string.IsNullOrEmpty(dataType.ActionRequiredToRead); | ||
| } | ||
|
|
||
| private static async Task<string> GenerateSha256Hash(Stream stream) | ||
| { | ||
| using var sha256 = SHA256.Create(); | ||
| byte[] digest = await sha256.ComputeHashAsync(stream); | ||
| return FormatShaDigest(digest); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Formats a SHA digest with common best practice:<br/> | ||
| /// Lowercase hexadecimal representation without delimiters | ||
| /// </summary> | ||
| /// <param name="digest">The hash code (digest) to format</param> | ||
| /// <returns>String representation of the digest</returns> | ||
| /// <remarks>This mirrors how the altinn-storage formats the Sha digest when creating the signature document, and it must stay in sync.</remarks> | ||
| private static string FormatShaDigest(byte[] digest) | ||
| { | ||
| return Convert.ToHexString(digest).ToLowerInvariant(); | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.