Skip to content

Conversation

@wfurt
Copy link
Member

@wfurt wfurt commented Jul 31, 2020

This regression caused by #32008. The old security buffer had dedicated size property. With the PR, we switched to using Span and Span.Length. However that is not correct for TokenBinding. For that the span is empty e.g. cbBuffer becomes 0 and OS will reject ChannelBinding buffer.
Alternatively we can convert the UnmanagedToken to Span as well. But since currently doing not have any good test infrastructure for this, I would prefer to limit the changes.

This effectively solves #39011 but I will leave it open until we have tests to prevent regression like this.

contributes to #39011.

@wfurt wfurt added this to the 5.0.0 milestone Jul 31, 2020
@wfurt wfurt requested a review from a team July 31, 2020 23:43
@wfurt wfurt self-assigned this Jul 31, 2020
@ghost
Copy link

ghost commented Jul 31, 2020

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@geoffkizer
Copy link
Contributor

Aside from that, LGTM

@SteveL-MSFT
Copy link
Contributor

@wfurt can you confirm if this change is already in .NET 5 preview 8?

@wfurt
Copy link
Member Author

wfurt commented Aug 28, 2020

It does not seems to @SteveL-MSFT. Looks like the fix missed the train and I don't see it in preview8 branch.

@SteveL-MSFT
Copy link
Contributor

@wfurt thanks for the confirmation. The reported issues in the PowerShell web cmdlets repro'd with a build with .NET 5 preview 8 and was concerned that this PR didn't fix it. Looks like I'll have to see if it's fixed with RC1.

@wfurt
Copy link
Member Author

wfurt commented Aug 28, 2020

unfortunately, yes. You can always grab daily build if you have local repro outside of production.

@karelz karelz modified the milestones: 5.0.0, 6.0.0 Aug 28, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants