Skip to content
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

SignValidationExclusionList does not work #2888

Closed
Tracked by #1270
natemcmaster opened this issue May 24, 2019 · 12 comments
Closed
Tracked by #1270

SignValidationExclusionList does not work #2888

natemcmaster opened this issue May 24, 2019 · 12 comments

Comments

@natemcmaster
Copy link
Contributor

natemcmaster commented May 24, 2019

I had to disable the async pipelines publishing for aspnet/Blazor because I cannot get SignValidationExclusionList to work.

I have specified, by file name, the files in Microsoft.AspNetCore.Blazor.Mono which are not supposed to be code-signed. (See https://github.com/aspnet/Blazor/blob/ff7b7c94be74f39d99043a3f5374960d78b76813/eng/Signing.props#L196-L197). Despite this, the signing validation step still fails the build . See https://dev.azure.com/dnceng/internal/_releaseProgress?_a=release-environment-logs&releaseId=8007&environmentId=22188

I suspect the problem is the generation of the signcheck exclusion file:

<WriteLinesToFile
File="$(SignCheckExclusionsFile)"
Lines="@(SignValidationExclusionList)"
Condition="'@(SignValidationExclusionList)' != ''"
Overwrite="true"
Encoding="Unicode"/>

I think this maybe producing a file that is not in the format signcheck expects.

@JohnTortugo @dougbu @JunTaoLuo

@natemcmaster
Copy link
Contributor Author

Would it be possible to specify the exclusion list directly, instead of generating it? I'm working on converting aspnet/AspNetCore to use Arcade. We already have a sign exclusion file in the right format (see https://github.com/aspnet/AspNetCore/blob/master/eng/signcheck.exclusions.txt). It seems unnecessary to invent an MSBuild API to generate this file when we can just create the file manually.

Also, I couldn't find anyone else using the SignValidationExclusionList item group, so it might be safe to remove this altogether.

@JohnTortugo
Copy link
Contributor

I think this maybe producing a file that is not in the format signcheck expects.

Did you try adding the separators that SignCheck expect?

@markwilkie
Copy link
Member

@natemcmaster ?

@natemcmaster
Copy link
Contributor Author

Sorry, missed the reply. I didn't try adding the separators yet. I haven't figured out a good way to test signcheck exclusions without pushing to master and hoping for the best.

@dagood
Copy link
Member

dagood commented Aug 26, 2019

I think this maybe producing a file that is not in the format signcheck expects.

Did you try adding the separators that SignCheck expect?

I found this in the code, but is there a more accessible doc on the separators that SignCheck expects?

/// <summary>
/// Creates a new <see cref="Exclusion"/>.
/// </summary>
/// <param name="exclusion">A string representation of a file exclusion. An exclusion contains a number of fields, separated by
/// a ';'. The entry is formated as FILE_PATTERNS;PARENT_FILES;COMMENT. Additional fields are ignored and fields may be left
/// empty, e.g. ";B.txt" indicates an exclusion with no file patterns and one parent file.
///
/// The FILE_PATTERNS and PARENT_FILES fields may contain multiple values separated by a '|'.
///
/// For example: "A.txt|C:\Dir1\B.txt;C.zip;" indicates an exclusion with two file patterns ("A.txt" and "C:\Dir1\B.txt") and one
/// parent file ("C.zip").
/// </param>
public Exclusion(string exclusion)

(Haven't seen whether it works yet. I expect it will given this recent fix: #3572.)

/cc @joeloff

@dagood
Copy link
Member

dagood commented Aug 26, 2019

Oh, weird, I guess SignValidationExclusionList truly doesn't work. The eng/common project has an implementation:
https://github.com/dotnet/arcade/blob/master/eng/common/SigningValidation.proj

but the SDK has a copy that doesn't implement it:
https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.Arcade.Sdk/tools/SdkTasks/SigningValidation.proj

I guess stage-based publish only supports the raw txt file, and the PR I pointed to is only about that. 😕

@joeloff
Copy link
Member

joeloff commented Aug 26, 2019

Looks like Arcade might be generating a file based on an item group.

@dagood
Copy link
Member

dagood commented Aug 26, 2019

It does in the eng/common one, but not the SDK duplicate. Stages-based publishing/validation seems to use the SDK duplicate.

@JohnTortugo
Copy link
Contributor

For the record, this is the exclusion file that stages-based signing validation uses:

https://github.com/dotnet/arcade/blob/master/eng/common/templates/post-build/post-build.yml#L62

@dagood
Copy link
Member

dagood commented Aug 26, 2019

Yeah, I figured out how to get exclusions to work like that without SignValidationExclusionList from this PR: dotnet/coreclr#26020.

It seems to work fine, I am not blocked on SignValidationExclusionList. However, maintaining two separate files (Signing.props and SignCheckExclusionsFile.txt) is risky, so I would still appreciate something like SignValidationExclusionList.

@dougbu
Copy link
Member

dougbu commented Nov 18, 2019

I don't believe this problem remains. For example, I was able to exclude some sign checks in dotnet/aspnetcore#13899. But, I'm leaving this open in case I'm missing some aspect of the problem.

@Chrisboh
Copy link
Member

Chrisboh commented Dec 2, 2022

Based on Doug's last comment I am assuming this is fixed.

@Chrisboh Chrisboh closed this as completed Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants