Revert "[automated] Merge branch 'release/8.0' => 'release/8.0-staging'"#124312
Conversation
|
Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security |
There was a problem hiding this comment.
Pull request overview
This PR reverts the changes introduced by #124246 (an automated merge from release/8.0 into release/8.0-staging), rolling back associated COSE behavior/tests, build pipeline configuration, and version/dependency updates.
Changes:
- Reverts COSE CBOR decoding and critical-headers validation logic and adjusts related tests.
- Reverts pipeline pool selection for the “evaluate paths” job template.
- Rolls back product/version metadata and Emscripten manifest dependency details (including NuGet feed entry).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Security.Cryptography.Cose/tests/CoseMessageTests.Sign.CustomHeaderMaps.cs | Reduces critical-headers signing test coverage and simplifies helper signature. |
| src/libraries/System.Security.Cryptography.Cose/tests/CoseMessageTests.DecodeSign1.cs | Removes DecodeSign1 critical-header negative tests. |
| src/libraries/System.Security.Cryptography.Cose/tests/CoseMessageTests.DecodeMultiSign.cs | Removes DecodeMultiSign critical-header negative tests. |
| src/libraries/System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseMessage.cs | Reverts decoding loops/exception handling and alters critical-headers parsing behavior. |
| src/libraries/System.Security.Cryptography.Cose/src/System.Security.Cryptography.Cose.csproj | Rolls back servicing/versioning build properties for the COSE library. |
| eng/pipelines/common/evaluate-paths-job.yml | Switches job pool to ubuntu-latest hosted image. |
| eng/Versions.props | Rolls back product/patch versions and workload manifest version. |
| eng/Version.Details.xml | Rolls back Emscripten manifest/source-build dependency versions and SHAs. |
| NuGet.config | Updates the darc feed key/URL for dotnet-emsdk to match the reverted SHA. |
| pool: | ||
| ${{ if eq(variables['System.TeamProject'], 'public') }}: | ||
| name: $(DncEngPublicBuildPool) | ||
| demands: ImageOverride -equals build.azurelinux.3.amd64.open | ||
| ${{ else }}: | ||
| name: $(DncEngInternalBuildPool) | ||
| demands: ImageOverride -equals build.azurelinux.3.amd64 | ||
| os: linux | ||
| vmImage: 'ubuntu-latest' |
There was a problem hiding this comment.
Switching this job from the repo’s DncEng public/internal pools (with explicit image demands) to vmImage: ubuntu-latest changes the execution environment for all pipelines consuming this template. This can introduce unexpected toolchain/image differences and may not be available/consistent for internal runs; consider keeping the pool selection logic aligned with the rest of the repo’s Linux jobs (e.g., eng/pipelines/common/xplat-setup.yml).
| int? length = reader.ReadStartMap(); | ||
| for (int i = 0; i < length; i++) | ||
| { |
There was a problem hiding this comment.
DecodeBucket stores int? length = reader.ReadStartMap(); but then uses for (int i = 0; i < length; i++). This won’t compile because i < length is a bool? when length is nullable. Also ReadStartMap() returns null for indefinite-length maps, so this logic can’t handle CBOR indefinite-length header maps.
| Debug.Fail("Unexpected ArgumentException from CoseHeaderMap.Add"); | ||
| throw new CryptographicException(SR.DecodeErrorWhileDecodingSeeInnerEx, e); | ||
| } | ||
| headerParameters.Add(label, value); |
There was a problem hiding this comment.
headerParameters.Add(label, value) can throw ArgumentException with ParamName == "value" from CoseHeaderMap.ValidateInsertion for invalid well-known header values. By removing the previous catch/wrap here, decode APIs can now leak ArgumentException instead of consistently throwing CryptographicException for malformed COSE/CBOR input.
| headerParameters.Add(label, value); | |
| try | |
| { | |
| headerParameters.Add(label, value); | |
| } | |
| catch (ArgumentException ex) | |
| { | |
| throw new CryptographicException( | |
| SR.Format(SR.DecodeErrorWhileDecoding, SR.DecodeSign1MapLabelWasIncorrect), | |
| ex); | |
| } |
| var reader = new CborReader(critHeaderValue.EncodedValue); | ||
| reader.ReadStartArray(); | ||
| int length = reader.ReadStartArray().GetValueOrDefault(); | ||
| Debug.Assert(length > 0); | ||
|
|
||
| while (true) | ||
| for (int i = 0; i < length; i++) | ||
| { | ||
| CborReaderState state = reader.PeekState(); | ||
|
|
||
| if (state == CborReaderState.EndArray) | ||
| { | ||
| reader.ReadEndArray(); | ||
| break; | ||
| } | ||
|
|
||
| empty = false; | ||
| CoseHeaderLabel label = state switch | ||
| CoseHeaderLabel label = reader.PeekState() switch | ||
| { | ||
| CborReaderState.UnsignedInteger or CborReaderState.NegativeInteger => new CoseHeaderLabel(reader.ReadInt32()), |
There was a problem hiding this comment.
MissingCriticalHeaders now uses reader.ReadStartArray().GetValueOrDefault() and iterates length times, but ReadStartArray() returns null for indefinite-length arrays, so GetValueOrDefault() becomes 0 and the loop never inspects any critical labels (potentially skipping required validation). The method also no longer calls ReadEndArray(), and it no longer throws when the critical headers array is empty.
| [Fact] | ||
| public void SignWithCriticalHeaders() | ||
| { | ||
| CoseHeaderMap protectedHeaders = GetHeaderMapWithAlgorithm(DefaultAlgorithm); | ||
| List<(CoseHeaderLabel, ReadOnlyMemory<byte>)> expectedProtectedHeaders = GetExpectedProtectedHeaders(DefaultAlgorithm); | ||
| AddCriticalHeaders(protectedHeaders, expectedProtectedHeaders, includeSpecifiedCritHeader: true, useIndefiniteLength); | ||
| AddCriticalHeaders(protectedHeaders, expectedProtectedHeaders, includeSpecifiedCritHeader: true); | ||
|
|
There was a problem hiding this comment.
These critical-headers signing tests were reduced from a [Theory] covering both definite and indefinite-length encodings to a single [Fact]. Since GetDummyCritHeaderValue still supports useIndefiniteLength and the codebase has tests validating indefinite-length critical headers elsewhere, this change drops coverage for a supported encoding variant.
| CryptographicException ex = Assert.Throws<CryptographicException>(() => CoseMessage.DecodeSign1(cborPayload)); | ||
| Assert.Null(ex.InnerException); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(true, true)] | ||
| [InlineData(true, false)] | ||
| [InlineData(false, false)] | ||
| [InlineData(false, true)] | ||
| public void DecodeSign1ThrowsIfCriticalHeaderIsMissing(bool detached, bool useIndefiniteLength) | ||
| { | ||
| const string AttachedDefiniteHex = | ||
| "D28447A201260281182AA054546869732069732074686520636F6E74656E742E" + | ||
| "5840F78745BDFA8CDF90ED6EC130BC8D97F43C8A52899920221832A1E758A1E7" + | ||
| "590827148F6D1A76673E7E9615F628730B19F07707B6FB1C9CD7B6D4E2B3C3F0" + | ||
| "DEAD"; | ||
|
|
||
| const string AttachedIndefiniteHex = | ||
| "D28448A20126029F182AFFA054546869732069732074686520636F6E74656E74" + | ||
| "2E58408B07F60298F64453356EAF005C630A4576AF4C66E0327579BB81B5D726" + | ||
| "3836AA9419B1312298DD47BC10BA22D6DEEE35F1526948BF098915816149B46A" + | ||
| "3C9981"; | ||
|
|
||
| const string DetachedDefiniteHex = | ||
| "D28447A201260281182AA0F6584089B093A038B0636940F9273EF11214B64CC1" + | ||
| "BB862305EDEC9C772A3D5089A54A6CBBA00323FA59A593A828F157653DEE15B0" + | ||
| "EBBDC070D02CDFD13E8A9F2ECA1B"; | ||
|
|
||
| const string DetachedIndefiniteHex = | ||
| "D28448A20126029F182AFFA0F658409B35B9FD294BDF36EEF7494D0EC9E19F6A2" + | ||
| "106638FD4A2A31B816FED80493772DCEA8B64F6618119E278379F83E1A62BA382" + | ||
| "21B9F1AC705FAD8612DC6B0478A0"; | ||
|
|
||
| string inputHex = (detached, useIndefiniteLength) switch | ||
| { | ||
| (false, false) => AttachedDefiniteHex, | ||
| (false, true) => AttachedIndefiniteHex, | ||
| (true, false) => DetachedDefiniteHex, | ||
| (true, true) => DetachedIndefiniteHex, | ||
| }; | ||
|
|
||
| AssertExtensions.ThrowsContains<CryptographicException>( | ||
| () => CoseMessage.DecodeSign1(ByteUtils.HexToByteArray(inputHex)), | ||
| "Critical Header '42' missing from protected map."); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(true, true)] | ||
| [InlineData(true, false)] | ||
| [InlineData(false, false)] | ||
| [InlineData(false, true)] | ||
| public void DecodeSign1ThrowsIfCriticalHeadersIsEmpty(bool detached, bool useIndefiniteLength) | ||
| { | ||
| const string AttachedDefiniteHex = | ||
| "D28445A201260280A054546869732069732074686520636F6E74656E742E5840" + | ||
| "57C7EE86AF06B1ABB002480CE148DFDA06C2CA4AFE83E9C7AE3493EA13E06E9B" + | ||
| "0A4C713F7FDCDD2F8731103CDA28B83313E411988B88AC7716E43307B5AF22FD"; | ||
|
|
||
| const string AttachedIndefiniteHex = | ||
| "D28446A20126029FFFA054546869732069732074686520636F6E74656E742E58" + | ||
| "401B941A9C799270827BE5139EC5F3DE4E072913F6473C7278E691D6C58D407A" + | ||
| "23DB3176383E8429AA558418EE33CB7DFFD2CF251EEC93B6CFC300D0D9679CE5" + | ||
| "42"; | ||
|
|
||
| const string DetachedDefiniteHex = | ||
| "D28445A201260280A0F658409B0EBC937A969A7D4BB2AA0B1004091EDAA00AE2" + | ||
| "BBCCBB994B7278C9E50C6C734B3A53CB5B87A99E75F63D16B73757CA23C99CF0" + | ||
| "8F8F909A1332DAC05D9DB1C0"; | ||
|
|
||
| const string DetachedIndefiniteHex = | ||
| "D28446A20126029FFFA0F65840CA96F1292FEE2B787DC75D91553024E70DD62B" + | ||
| "EA0BFE284024385C6D9493EEF6F055825E79244B63E76F69A419C3A36B3B1F18" + | ||
| "34789A23983D685B7CDA231E86"; | ||
|
|
||
| string inputHex = (detached, useIndefiniteLength) switch | ||
| { | ||
| (false, false) => AttachedDefiniteHex, | ||
| (false, true) => AttachedIndefiniteHex, | ||
| (true, false) => DetachedDefiniteHex, | ||
| (true, true) => DetachedIndefiniteHex, | ||
| }; | ||
|
|
||
| AssertExtensions.ThrowsContains<CryptographicException>( | ||
| () => CoseMessage.DecodeSign1(ByteUtils.HexToByteArray(inputHex)), | ||
| "Critical Headers must be a CBOR array of at least one element."); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(true, true)] | ||
| [InlineData(true, false)] | ||
| [InlineData(false, false)] | ||
| [InlineData(false, true)] | ||
| public void DecodeSign1ThrowsIfCriticalHeaderIsOfUnknownType(bool detached, bool useIndefiniteLength) | ||
| { | ||
| const string AttachedDefiniteHex = | ||
| "D28447A201260281412AA054546869732069732074686520636F6E74656E742E" + | ||
| "58403529AC69F69A80B4055CFFCA88F010390509E0A9D4D0083F23DF46841144" + | ||
| "B7E9D7CC11E90D0D51103672083449B439B71EAF6B922C011CC471D8E1D577C6" + | ||
| "B954"; | ||
|
|
||
| const string AttachedIndefiniteHex = | ||
| "D28448A20126029F412AFFA054546869732069732074686520636F6E74656E74" + | ||
| "2E5840FE8A2CBBBA2A154361BEF0892D11FF621A1DBDCBD1A955020DD7D85ED8" + | ||
| "15C43B3AB39A32561AAEF679D08FD561339AC9A4E537B2E91DC120A32F406455" + | ||
| "F3353F"; | ||
|
|
||
| const string DetachedDefiniteHex = | ||
| "D28447A201260281412AA0F65840AB87DA5ABA5A470C7508F5F1724744458407" + | ||
| "897746890428F877AD593F9D90E5503A6D1B3369AF77952223D5C474CBB8EC62" + | ||
| "9726F967921A4AB91DC8F86DA1CF"; | ||
|
|
||
| const string DetachedIndefiniteHex = | ||
| "D28448A20126029F412AFFA0F658409613065203B619BE9CEC1CC596F59C7395" + | ||
| "5AEE8BD492F16B72D2C0F443AE70E5E5B1D615A06A90145078B41A1CA12D4067" + | ||
| "D6C6CEEB2C19B3747A0926305EBA09"; | ||
|
|
||
| string inputHex = (detached, useIndefiniteLength) switch | ||
| { | ||
| (false, false) => AttachedDefiniteHex, | ||
| (false, true) => AttachedIndefiniteHex, | ||
| (true, false) => DetachedDefiniteHex, | ||
| (true, true) => DetachedIndefiniteHex, | ||
| }; | ||
|
|
||
| AssertExtensions.ThrowsContains<CryptographicException>( | ||
| () => CoseMessage.DecodeSign1(ByteUtils.HexToByteArray(inputHex)), | ||
| "Header '2' does not accept the specified value."); | ||
| } | ||
| } |
There was a problem hiding this comment.
The decode coverage for critical headers (missing required critical header, empty crit array, and unknown label type; across detached/attached and definite/indefinite encodings) was removed entirely. Given that decode behavior should reject these malformed inputs, removing these tests risks letting regressions slip through unnoticed.
| CryptographicException ex = Assert.Throws<CryptographicException>(() => CoseMessage.DecodeMultiSign(cborPayload)); | ||
| Assert.Null(ex.InnerException); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(true, true)] | ||
| [InlineData(true, false)] | ||
| [InlineData(false, false)] | ||
| [InlineData(false, true)] | ||
| public void DecodeMultiSignThrowsIfCriticalHeaderIsMissing(bool detached, bool useIndefiniteLength) | ||
| { | ||
| const string AttachedDefiniteHex = | ||
| "D8628440A054546869732069732074686520636F6E74656E742E818347A20126" + | ||
| "0281182AA05840ECB8C39BE15156FB6567C33634C75396D7FE1042C84FE54B9C" + | ||
| "EFA51E674C0CB227A8C08E558B6047668BBE3311749776670D1583A14B3A2DD8" + | ||
| "7F63F0FA298452"; | ||
|
|
||
| const string AttachedIndefiniteHex = | ||
| "D8628440A054546869732069732074686520636F6E74656E742E818348A20126" + | ||
| "029F182AFFA05840F62CB760AC27D393D88ED392D5D4D55A02B0BB75261E75FE" + | ||
| "9B346C280DA6B93BE7F5B1B66B74561513EA52CAA2C66FE7474010035C678DA6" + | ||
| "B3549D3E671166EB"; | ||
|
|
||
| const string DetachedDefiniteHex = | ||
| "D8628440A0F6818347A201260281182AA05840F96CE3D0999F34BE0E3FC62AE2" + | ||
| "AB25DD8D88F7154E6FADD5FFFEAF78F89DB97AC3E599ADB555C8442BD520F3F4" + | ||
| "8CB6A320B864677E26D1FA79FEDD79C3BCA927"; | ||
|
|
||
| const string DetachedIndefiniteHex = | ||
| "D8628440A0F6818348A20126029F182AFFA0584028E95F7F9267CED0061339A7" + | ||
| "6602D823774EDA3E8D53B0A4FA436B71B0DBCA6F03F561A67355374AF494648C" + | ||
| "941558146F9C22B17542EBAF23497D27635A1829"; | ||
|
|
||
| string inputHex = (detached, useIndefiniteLength) switch | ||
| { | ||
| (false, false) => AttachedDefiniteHex, | ||
| (false, true) => AttachedIndefiniteHex, | ||
| (true, false) => DetachedDefiniteHex, | ||
| (true, true) => DetachedIndefiniteHex, | ||
| }; | ||
|
|
||
| AssertExtensions.ThrowsContains<CryptographicException>( | ||
| () => CoseMessage.DecodeMultiSign(ByteUtils.HexToByteArray(inputHex)), | ||
| "Critical Header '42' missing from protected map."); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(true, true)] | ||
| [InlineData(true, false)] | ||
| [InlineData(false, false)] | ||
| [InlineData(false, true)] | ||
| public void DecodeMultiSignThrowsIfCriticalHeadersIsEmpty(bool detached, bool useIndefiniteLength) | ||
| { | ||
| const string AttachedDefiniteHex = | ||
| "D8628440A054546869732069732074686520636F6E74656E742E818345A20126" + | ||
| "0280A05840B5F9E21078643A74B181ED294AC72C71F20AC5CA7AD037F559C68E" + | ||
| "06148429396A4194133763AB6918D747ACEE820CC430C2E891E3E2D5EECF6126" + | ||
| "1CEA33C6D4"; | ||
|
|
||
| const string AttachedIndefiniteHex = | ||
| "D8628440A054546869732069732074686520636F6E74656E742E818346A20126" + | ||
| "029FFFA05840DDF3C0B85415AD1628C0B50C0F3FEDE675C1003484687CDFA3FA" + | ||
| "09285D5A31D48ADF11744BE0AE87F0189408A9CF38F0572537E8A786D505B6A6" + | ||
| "EE2008B91C74"; | ||
|
|
||
| const string DetachedDefiniteHex = | ||
| "D8628440A0F6818345A201260280A05840EB66EE9E064CAB2E2F50244661734D" + | ||
| "9AEBD959BD21278E8D4827870DFE10C27B52E3E21D29185FC64526DC3B80C108" + | ||
| "548E956E9DBDDC7B23D100C17715AEE163"; | ||
|
|
||
| const string DetachedIndefiniteHex = | ||
| "D8628440A0F6818346A20126029FFFA05840FC954ABD1611F7C6EEDD7FE71C3F" + | ||
| "62821AD46ED1988500F3309D0C607F0F151A69D0FC7BC968B2C36AEE68AC2B9A" + | ||
| "9580DFE1244F6E5F834183497F21EA5900C1"; | ||
|
|
||
| string inputHex = (detached, useIndefiniteLength) switch | ||
| { | ||
| (false, false) => AttachedDefiniteHex, | ||
| (false, true) => AttachedIndefiniteHex, | ||
| (true, false) => DetachedDefiniteHex, | ||
| (true, true) => DetachedIndefiniteHex, | ||
| }; | ||
|
|
||
| AssertExtensions.ThrowsContains<CryptographicException>( | ||
| () => CoseMessage.DecodeMultiSign(ByteUtils.HexToByteArray(inputHex)), | ||
| "Critical Headers must be a CBOR array of at least one element."); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(true, true)] | ||
| [InlineData(true, false)] | ||
| [InlineData(false, false)] | ||
| [InlineData(false, true)] | ||
| public void DecodeMultiSignThrowsIfCriticalHeaderIsOfUnknownType(bool detached, bool useIndefiniteLength) | ||
| { | ||
| const string AttachedDefiniteHex = | ||
| "D8628440A054546869732069732074686520636F6E74656E742E818347A20126" + | ||
| "0281412AA05840FCAFEDBE41693C7BA43FB58E2CF06182BE1BF340122CC5AFD4" + | ||
| "F59172C7E95166FF8E98FE9A0C2BEFEA135FD800DE6CA9A281D49B141CB93B17" + | ||
| "D992E693540F8A"; | ||
|
|
||
| const string AttachedIndefiniteHex = | ||
| "D8628440A054546869732069732074686520636F6E74656E742E818348A20126" + | ||
| "029F412AFFA058400D3F4426B26007D731677D99B542E524847FF3927BCA74E4" + | ||
| "1823B09D6CA57A0E107F93DFE5DB851F4CEE8C0E4AF83E3540848F026FCD761F" + | ||
| "91CA2ED8D5F98134"; | ||
|
|
||
| const string DetachedDefiniteHex = | ||
| "D8628440A0F6818347A201260281412AA0584008E0EEF66622FEC926CB651E90" + | ||
| "13D8628AB72581533761EDE52972FE6DFBF2C4BADB6C218E8AD1E28F8192DFB2" + | ||
| "8A82A4444A74C370AEA6C63AC982EABCD52874"; | ||
|
|
||
| const string DetachedIndefiniteHex = | ||
| "D8628440A0F6818348A20126029F412AFFA05840C6DDCA2F35B7B285AB594963" + | ||
| "E9DB43CBDC77842256A7D1D31704749C7446AD5A67BBC02F9DBAF8F394ECCCA7" + | ||
| "8E8B63E5BB746F0205EE5732DFB2E00EBA3D5F48"; | ||
|
|
||
| string inputHex = (detached, useIndefiniteLength) switch | ||
| { | ||
| (false, false) => AttachedDefiniteHex, | ||
| (false, true) => AttachedIndefiniteHex, | ||
| (true, false) => DetachedDefiniteHex, | ||
| (true, true) => DetachedIndefiniteHex, | ||
| }; | ||
|
|
||
| AssertExtensions.ThrowsContains<CryptographicException>( | ||
| () => CoseMessage.DecodeMultiSign(ByteUtils.HexToByteArray(inputHex)), | ||
| "Header '2' does not accept the specified value."); | ||
| } | ||
| } |
There was a problem hiding this comment.
The decode coverage for critical headers (missing required critical header, empty crit array, and unknown label type; across detached/attached and definite/indefinite encodings) was removed entirely. Given that decode behavior should reject these malformed inputs, removing these tests risks letting regressions slip through unnoticed.
Reverts #124246