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

trusted_ca_certificates puts certificate in 'conventional location' which is not used #378

Closed
sharms opened this issue Jan 16, 2018 · 8 comments
Labels

Comments

@sharms
Copy link

sharms commented Jan 16, 2018

Summary

trusted_certs has been depreciated in favor of trusted_ca_certificates, however the certificates are now put in /var/vcap/data/rep/trusted_certs and are ignored by buildpacks and openssl. This can be configured on the rootfs options, but trusted_ca_certificates functionally isn't of value.

Expected Result

Certificates specified in trusted_ca_certificates are used as trusted ca certificates

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/154364290

The labels on this github issue will be updated when the story is started.

@emalm
Copy link
Member

emalm commented Jan 25, 2018

Hi, @sharms,

That /var/vcap/data/rep/trusted_certs location is bind-mounted to the app instance containers on the cell at the location specified by the TrustedSystemCertificatesPath field, which in a CF context is always /etc/cf-system-certificates. Recent versions of Diego also export this location into the container environment in the $CF_SYSTEM_CERT_PATH variable.

I agree that this level of installation is suboptimal for buildpack-app containers based on cflinuxfs2, since in that case the Linux distribution is known in advance to be Ubuntu and so the rootfs setup job can install extra certs, but for supplying these same CA certs to Docker-image-based apps with filesystems based on arbitrary Linux distributions (CentOS, SUSE, or even just an /etc/passwd file and a /root directory) or to Windows app instances it has seemed like the only reasonable option.

Thanks,
Eric, CF Diego PM

@emalm
Copy link
Member

emalm commented Feb 13, 2018

Updated https://docs.cloudfoundry.org/running/trusted-system-certificates.html with information about configuring containers.trusted_ca_certificates in cloudfoundry/docs-running-cf#51, so closing this out this issue.

@qwertycody
Copy link

emalm, I've tested this and it would seem that this property is ignored on my application.yml file during the cf-push.

@emalm
Copy link
Member

emalm commented Jun 26, 2018

@qwertycody This is a setting relevant for the BOSH manifest that deploys the Diego cell rep as a CF components, not a setting for per-app configuration via its CF manifest. If you need to supply additional CA certificates for your application to trust, maybe it would work for you to do so via other configuration mechanisms for CF apps, such as environment variables or a user-provided service instance? If that's not sufficient, I'd be interested to hear why.

Thanks,
Eric

@qwertycody
Copy link

qwertycody commented Jun 26, 2018

Well, to be quite honest, I'm not entirely sure that .NET Core would support something that based on my initial research. I definitely could have missed something.

`applications:

  • name: Best App Ever
    memory: 1G
    instances: 1
    buildpack: dotnet_core_buildpack
    stack: cflinuxfs2
    env:
    CACHE_NUGET_PACKAGES: false
    ASPNETCORE_ENVIRONMENT: Development
    path: ViewController/bin/Debug/netcoreapp2.0/publish
    services:
  • service-registry
  • config-server
  • sso
    properties:
    cflinuxfs2-rootfs:
    trusted_certs:
  • |
    -----BEGIN CERTIFICATE-----
    MIIGlzCCBX+gAwIBAgIKUP73nAAAAAHEyjANBgkqhkiG9w0BAQUFADBnMRMwEQYK
    CZImiZPyLGQBGRYDY29tMRIwEAYKCZImiZPyLGQBGRYCZ20xFDASBgoJkiaJk/Is
    ZAEZFgRjb3JwMSYwJAYDVQQDEx1HTSBJbmZyYXN0cnVjdHVyZSBJc3N1aW5nIENB
    MjAeFw0xNTEwMjMxNjA5MDFaFw0xODEwMjIxNjA5MDFaMAAwggEiMA0GCSqGSIb3
    DQEBAQUAA4IBDwAwggEKAoIB
    -----END CERTIFICATE--------- `

@emalm
Copy link
Member

emalm commented Jun 26, 2018

@qwertycody Would you mind reformatting that app manifest with a triple-backtick fenced code block? It's difficult to tell the YAML structure as is.

@qwertycody
Copy link

@emalm, wanted to follow up and let you guys know I ended up finding the solution for this.

As you mentioned, the above solution is not for an APP but for a cell. The below code is how I solved the problem. Turns out the LDAP Library I was using implements a common SSLValidationHandler and I wrote a method to do this validation in the app.

public static bool ManualSslVerification(object sender, X509Certificate certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors)
        {
            try
            {
                //Testing to see if the Certificate and Chain build properly, aka no forgery.
                chain.ChainPolicy.VerificationFlags = X509VerificationFlags.NoFlag;
                chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
                chain.Build(new X509Certificate2(certificate));

                //Looking to see if there are no errors in the build that we don't like
                foreach (X509ChainStatus status in chain.ChainStatus)
                {
                    if (status.Status == X509ChainStatusFlags.NoError || status.Status == X509ChainStatusFlags.UntrustedRoot)
                    {
                        //Acceptable Status, We want to know if it builds properly.
                    }
                    else
                    {
                        return false;
                    }
                }

                X509Certificate2 trustedRootCertificateAuthority = new X509Certificate2(ViewController.Properties.Resources.My_Root_CA);

                //Now that we have tested to see if the cert builds properly, we now will check if the thumbprint of the root ca matches our trusted one
                if(chain.ChainElements[chain.ChainElements.Count - 1].Certificate.Thumbprint != trustedRootCertificateAuthority.Thumbprint)
                {
                    return false;
                }

                //Once we have verified the thumbprint the last fun check we can do is to build the chain and then see if the remote cert builds properly with it
                //Testing to see if the Certificate and Chain build properly, aka no forgery.
                X509Chain trustedChain = new X509Chain();
                trustedChain.ChainPolicy.ExtraStore.Add(trustedRootCertificateAuthority);
                trustedChain.ChainPolicy.VerificationFlags = X509VerificationFlags.NoFlag;
                trustedChain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
                trustedChain.Build(new X509Certificate2(certificate));

                //Looking to see if there are no errors in the build that we don't like
                foreach (X509ChainStatus status in trustedChain.ChainStatus)
                {
                    if(status.Status == X509ChainStatusFlags.NoError || status.Status == X509ChainStatusFlags.UntrustedRoot)
                    {
                        //Acceptable Status, We want to know if it builds properly.
                    }
                    else
                    {
                        return false;
                    }
                }
            }
            catch (Exception ex)
            {
                Console.WriteLine(ex);
                return false;
            }

            return true;
        }

n4wei pushed a commit that referenced this issue Jan 11, 2019
[finishes #163037208](https://www.pivotaltracker.com/story/show/163037208)

Submodule src/code.cloudfoundry.org/executor 325be3662..b67be5e6e:
  > Merge branch 'pr/41'
Submodule src/github.com/envoyproxy/go-control-plane 000000000...8a91fb26f (new submodule)
Submodule src/github.com/gogo/googleapis 000000000...8558fb44d (new submodule)
Submodule src/github.com/gogo/protobuf f9114dace..4cbf7e384:
  > Merge pull request #528 from jmarais/mergeaa810b6
  > merged in golang/protobuf commit 7d1b268556d691919f2262240737157830eab632 - jsonpb: avoid unexported fields in hand-crafted message (#526)
  > merged in golang/protobuf commit f5983d50c82d70eaa88c17080245cc871558081f - proto: make invalid UTF-8 errors non-fatal (#525)
  > merged in golang/protobuf commit 560bdb64431cc123098c2db67f16053a923a0688 - jsonpb: strictly document JSONPBMarshaler and JSONPBUnmarshaler behavior (#524)
  > merged in golang/protobuf commit 93b26e6a70e37abb14f2f88194949312b0592a84 - protoc-gen-go: refactor generator by splitting up generateMessage
  > Merge pull request #522 from jmarais/somemerges
  > use only one write in the varint writer when possible (#504)
  > fix typo independant to independent (#512)
  > Add godocs link to Readme.md (#506)
  > Fix text unmarshal for (u)int(8/16) fields (#498)
  > Codegen for well-known types (#489)
  > reorder some of the protoc paths in order to prefer our protobuf/google/protobuf/*.proto files. This is just to avoid using the wrong protos if you have the same protos in you gopath/src dir. (#502)
  > fix error: bad Go source code was generated, illegal hexadecimal number (#488)
  > Jsonpb custom type - #411 (#491)
  > Customtype  Warnings and issues  update (#479)
  > Exact slice allocation for repeated packed varints (#480)
  > Adding missing func to CustomType documentation (#483)
  > bumped the go version (#475)
  > added nil check in Proto/Size methods fix #444 (#451)
  > fix for letmegrpc (#474)
  > options to not generate xxx fields (#467)
  > updated to go1.11 and removed go1.9 (#473)
  > merged in golang/protobuf commit 70c277a8a150a8e069492e6600926300405c2884 - Fix unmarshaling JSON object with escaped string into Struct type. (#464)
  > merge in golang/protobuf commit 3a3da3a4e26776cc22a79ef46d5d58477532dede - proto: mention field name in error message (#616) (#465)
  > added license details to Readme.md (#469)
  > Update Readme.md (#468)
  > Set defaults on nonnullable fields (#435) (#459)
  > removed the GOPATH env dep in the makefile protopath (#461)
  > Merge pull request #455 from donaldgraham/master
  > Fix nullable extension issues for non generated code (#453)
  > Fix wrong build tags (#445)
  > merged commit 32a84b27e28ab9f681f0df16160c4ef1f6587094 from golang/pr… (#446)
  >  Added ProtoSize wrapper functions for the well known types  #438 (#443)
  > exact slice allocation for fixed size packed fields (#437)
  > Added gRPC Course on Udemy (#434)
  > Update Readme.md
  > Fix typo (#441)
  > fix #427 consistent import naming between the import declaration and the vars in grpc
  > fix build by regenerating everything
  > fix git diff for travis
  > merged 7c4add53b497798e7fd7b204f28e41ab409bdbb7 from golang/protobuf. protoc-gen-go: remove deprecated function in grpc (#426)
  > merged 3fac2a27c94f99f4379551928df388fcb0ad37ce from golang/protobuf. ptypes: optimize Is to avoid prefix scan (#425)
  > Merge pull request #424 from gogo/dev
  > gofmt
  > travis: opt into apt get
  > messagename
  > grpc error usage article
  > add mentions from Johan Brandhorst
  > merge bbd03ef6da3a115852eaf24c8a1c46aeb39aa175 from golang/protobuf
  > upgrade to go1.10
  > fix build for gopherjs that now requires go 1.10
  > fix build
  > fix for issue 389: importing of customtypes that are messages should not cause another import for the original message
  > Update Readme.md
  > add new user : go-spacemesh
  > protoc-min-version don't suppress protoc stdout and stderr out on success. (#381)
  > Update Readme.md
  > More well known types (#378)
  > Added link to new blog post in README (#375)
  > merge 925541529c1fa6821df4e44ce2723319eb2be768 from golang/protobuf
  > Added instructions for using proto files from google/protobuf (#371)
  > another user: zero stor
  > Update to protobuf 3.5.1 and minor cleanups for Golang 1.10 (#363)
  > Test with latest Go and protobuf patch versions. (#352)
  > Merge pull request #361 from temoto/lint-equal-return
  > Merge pull request #357 from benesch/populate-recursion
  > example
  > Merge pull request #355 from agnivade/agnivade-protopath
  > Merge pull request #341 from meling/master
  > fix for testdata/my_test
  > Merge pull request #350 from tomwilkie/go_package
  > Merge pull request #345 from Starnop/master
  > min version 3 for my_test/test.proto
  > Update extensions.md
  > merged 130e6b02ab059e7b717a096f397c5b60111cae74 from golang/protobuf
  > updated descriptor
  > merged 11b8df160996e00fd4b55cbaafb3d84ec6d50fa8 from golang/protobuf
  > merged 17ce1425424ab154092bbb43af630bd647f3bb0d from golang/protobuf
  > merged 5afd06f9d81a86d6e3bb7dc702d6bd148ea3ff23 from golang/protobuf
  > Merge pull request #343 from gogo/nounsafe
  > Merge branch 'master' of https://github.com/gogo/protobuf
  > fix mixbench
  > fix for issue 330
  > remove unset GOROOT
  > different unset method
  > unset GOROOT
  > update gopherjs
  > go1.9 for gopherjs
  > upgrade descriptor and other includes
  > protoc3.4
  > Merge branch 'master' of https://github.com/gogo/protobuf
  > less go versions
  > upgrade to go1.9
  > nullable = false for defaults as well
  > merged 1909bc2f63dc92bb931deace8b8312c4db72d12f from golang/protobuf
  > merged 748d386b5c1ea99658fd69fe9f03991ce86a90c1 from golang/protobuf
  > merged 0a4f71a498b7c4812f64969510bcb4eca251e33a from golang/protobuf
  > merged 4f95b0d3eab845462f9b0ca5ad21dea8093be211 from golang/protobuf
  > merged 5a0f697c9ed9d68fef0116532c6e05cfeae00e55 from golang/protobuf
  > merged 6e4cc92cc905d5f4a73041c1b8228ea08f4c6147 from golang/protobuf
  > merged 9f174c986221c608fb5143bd623b6076a71feae3 from golang/protobuf
  > merge 7b8002443fd4a3ce5f25ef93087c524546799a56 from golang/protobuf
  > merge 63bfc7087234061252151bf2276a9db446645bb9 from golang/protobuf
  > Fix unmarshal to correctly handle default value in maps (#318)
  > Fix a race when using TextMarshal on a StdDuration (#290)
  > another gostring fix for non nullable repeated types
  > fix for issue 312
  > hacks to reproduce a failing GoString test
  > trying to reproduce issue 312
  > Update Readme.md
  > more consistent marshal/unmarshal behavior for types that implement json Marshaler and Unmarshaler interfaces (#308)
  > Update Readme.md
  > Update Readme.md
  > Update Readme.md
  > remove tags of typedecl=false and add JsonPBMarshalers and Unmarshaler implementations
  > fix:305 only serialize protofields (#307)
  > Update Readme.md
  > Update Readme.md
  > Merge branch 'master' of https://github.com/gogo/protobuf
  > doc.go file in sizerconflict
  > merged 7a211bcf3bce0e3f1d74f9894916e6f116ae83b4 from golang/protobuf
  > merged a4e8f93323fcf2bc1ae5c036dc4d973872057936 from golang/protobuf
  > upgrade to proto3.3 and merge 157d9c53be5810dd5a0fac4a467f7d5f400042ea and fec3b39b059c0f88fa6b20f5ed012b1aa203a8b4 from golang/protobuf
  > merged 47eb67eaf5cab63c58956d4d4ce86b03ad5eaa03 from golang/protobuf follow the new code generation comment convention
  > merged b50ceb1fa9818fa4d78b016c2d4ae025593a7ce3 from golang/protobuf
  > disable Any with Message for go1.7
  > merged 18c9bb3261723cd5401db4d0c9fbc5c3b6c70fe8 from golang/protobuf
  > Update extensions.md
  > removed go1.6.3 from continious testing
  > Merge branch 'master' of https://github.com/gogo/protobuf
  > install protoc script modified to rather download precompiled protoc for versions which has this available
  > protoc 3.2.0 (#280)
  > skip unsafe tests for bigendian architectures. (#272)
  > Compile time warning when both Sizer and Protosizer are used (#271)
  > Add dual registration plugin (#270)
  > better mesos-go link
  > fixed mesos-go sample link
  > a real fix for issue261
  > Merge branch 'master' of https://github.com/gogo/protobuf
  > fixes issue 261
  > fix broken build
  > fix for issue 262
  > Remove unnecessary statement (#263)
  > Add references to some custom imports (#258)
  > new user protoactor-go
  > Unrendered markdown (#256)
  > Drop types declaration option (#250)
  > fix for issue 253 gostring for stdtime and stdduration when nullable=false
Submodule src/github.com/lyft/protoc-gen-validate 000000000...a11cd25ca (new submodule)

Co-authored-by: Sunjay Bhatia <sbhatia@pivotal.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants