Skip to content
This repository was archived by the owner on Oct 5, 2021. It is now read-only.

remove windows cgo #20

Closed
wants to merge 12 commits into from
Closed

remove windows cgo #20

wants to merge 12 commits into from

Conversation

tg123
Copy link

@tg123 tg123 commented Nov 15, 2020

  • replace cgo with sys call. gcc is not widely used on windows
  • force use cng. cng is recommend after vista
  • PSS supported. this is for go app to use keys in cert store to do tls1.3 (https)
  • add OpenStoreWindows which takes store name and store location.

@tg123
Copy link
Author

tg123 commented Nov 15, 2020

seems golang.org/x/sys/windows drop support of 1.8

// +build go1.9

package windows

import "syscall"

type Errno = syscall.Errno

@ptoomey3 ptoomey3 requested a review from a team November 20, 2020 23:14
@ptoomey3
Copy link
Member

/cc @github/prodsec-reviewers

Copy link
Member

@vcsjones vcsjones left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request! This is a fairly large change and wanted to share some thoughts before it gets reviewed in detail.

replace cgo with sys call. gcc is not widely used on windows

This results in a fairly substantial change. Though I can understand that GCC and such tooling are not widely distributed on Windows, they are available. I also personally tend to find the syscall code more difficult to read. Is there a benefit to the syscall move aside from not having a dependency on GCC?

force use cng. cng is recommend after vista

I would have to go back and see why CAPI support was added initially, but if I had a hunch it is to be compatible with some PIV card handlers that do not support CNG. That might be less of a concern now, but given the choice of removing support for something and continuing to support it, my preference would be to continue to support it. How difficult would it be to continue to support CAPI?

@tg123
Copy link
Author

tg123 commented Dec 1, 2020

CGO

I also trying to move all syscall (or dll call) to window pkg in order to make code more readable https://go-review.googlesource.com/c/sys/+/273907

honestly, no benefits other than removing the dependency on gcc. but still, having a gcc made this pkg not easy to use on windows.
our projects are all built by MSVC, it is hard to introduce another compiler on windows.

CAPI

To support CAPI, just a convert those window API into go syscall pattern. not a big deal.
but i do not think we need to support those legacy API or ask them to use older version.

Thanks for the pull request! This is a fairly large change and wanted to share some thoughts before it gets reviewed in detail.

replace cgo with sys call. gcc is not widely used on windows

This results in a fairly substantial change. Though I can understand that GCC and such tooling are not widely distributed on Windows, they are available. I also personally tend to find the syscall code more difficult to read. Is there a benefit to the syscall move aside from not having a dependency on GCC?

force use cng. cng is recommend after vista

I would have to go back and see why CAPI support was added initially, but if I had a hunch it is to be compatible with some PIV card handlers that do not support CNG. That might be less of a concern now, but given the choice of removing support for something and continuing to support it, my preference would be to continue to support it. How difficult would it be to continue to support CAPI?

@tg123
Copy link
Author

tg123 commented Dec 1, 2020

GO 1.13 is EOL you want want to remove those unsupported versions

@lgarron
Copy link
Contributor

lgarron commented Oct 4, 2021

This project is now part of smimesign.
Please send this PR to https://github.com/github/smimesign if it's still relevant!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants