credentials/insecure: Implement insecure credentials.

[easwars]

No description provided.

[dfawley]

How does this impact call creds? Will call creds not aware of security level be allowed? Should they be? What do the other languages do? @yihuazhang do you know offhand?

[yihuazhang]

The security level of a channel is orthogonal to the security level of a call credential. A security level is an inherent property of both channel and call credential. For a call credential, its security level dictates the minimum security level a channel's security level should be in order to transfer the call credential. So in the case of insecure credentials, it can only transfer the call credentials having the security level - NoSecurity. The above behavior is consistent across all languages (C-core, Java, and Go). PLMK if you have more questions.

[dfawley]

We have a bunch of call credentials that were written before we had the "security levels" concept.

Are those supposed to work with insecure creds? I would assume anything that doesn't support security levels, because it is too old, should NOT be used with insecure creds. At least, if they have RequireTransportSecurity() = true.

Did we add a check somewhere:

if perRPCCreds.RequireTransportSecurity() && transportCreds.SecurityLevel() == INSECURE {
  // FAIL
}

I don't think we did. Here are the places I can see where we check RequireTransportSecurity:

grpc-go/clientconn.go

Lines 175 to 182 in 9519eff

	if cc.dopts.copts.TransportCredentials != nil || cc.dopts.copts.CredsBundle != nil {
	return nil, errCredentialsConflict
	}
	for _, cd := range cc.dopts.copts.PerRPCCredentials {
	if cd.RequireTransportSecurity() {
	return nil, errTransportCredentialsMissing
	}
	}

and

grpc-go/internal/transport/http2_client.go

Lines 559 to 562 in 9519eff

	if callCreds := callHdr.Creds; callCreds != nil {
	if !t.isSecure && callCreds.RequireTransportSecurity() {
	return nil, status.Error(codes.Unauthenticated, "transport: cannot send secure credentials on an insecure connection")
	}

+

grpc-go/internal/transport/http2_client.go

Lines 226 to 237 in 9519eff

	if transportCreds != nil {
	// gRPC, resolver, balancer etc. can specify arbitrary data in the
	// Attributes field of resolver.Address, which is shoved into connectCtx
	// and passed to the credential handshaker. This makes it possible for
	// address specific arbitrary data to reach the credential handshaker.
	contextWithHandshakeInfo := internal.NewClientHandshakeInfoContext.(func(context.Context, credentials.ClientHandshakeInfo) context.Context)
	connectCtx = contextWithHandshakeInfo(connectCtx, credentials.ClientHandshakeInfo{Attributes: addr.Attributes})
	conn, authInfo, err = transportCreds.ClientHandshake(connectCtx, addr.ServerName, conn)
	if err != nil {
	return nil, connectionErrorf(isTemporary(err), err, "transport: authentication handshake failed: %v", err)
	}
	isSecure = true

In both cases, the presence of TransportCredentials is enough to make the PerRPCCredentials happy. But that seems too weak now that we will have credentials that aren't actually providing security at all. Or does "insecure" credentials mean "also fake out call credentials so they think the connection is secure"? And if that is the case, these creds should return the max security level, shouldn't they?

[yihuazhang]

Good point. I agree the current check (if we leave as it is now) is too weak because as you said, all PerRPCCredentials having either security level > NoSecurity or RequireTransportSecurity() = true can be communicated over channels created with insecure credentials. I think we should add the check you suggested above, and probably in http2_client.go where GetRequestMetadata() is called?

[yihuazhang]

I think we should also consider a channel's Invalid security level for the purpose of backward compatibility in the above check. Similar to CheckSecurityLevel() API in credentials.go

[dfawley]

Yes, that all sounds right to me. But note that the check in clientconn.go can't be improved since the TransportCredentials don't report their security level until after the connection is established. So the only place we can do this is in http2_client.go, and it will have to be a connection failure and not a channel-creation failure.

Also, do we require !=NoSecurity or ==PrivacyAndIntegrity if PerRPCCredentials.RequireTransportSecurity()==true? Let's move this conversation to #3974.

[dfawley]

We should have a test that covers the interaction between insecure creds and call creds.

[easwars]

Could we add the test as part of the other PR where we add the missing checks mentioned in the other comment thread?

[dfawley]

Sure, since the interaction we want (reject PerRPCCredentials that RequireTransportSecurity) isn't happening now.

[yihuazhang]

QQ: Are we going to deprecate withinsecure() API once this insecure credential API gets stabilized? Anyway, we need to control the use of new API in prod by implementing a "stop-bleeding" mechanism (similar to the one we built for WithInsecure) @dfawley, @jiangtaoli2016 F.Y.I.

[dfawley]

We can label it as deprecated, but it needs to remain throughout v1.x of gRPC-Go since it was not experimental upon introduction.

Can we use the same whitelist for the insecure creds that we have for the WithInsecure flag?

[yihuazhang]

I see, thanks for the information.

Yes, I believe we can reuse the existing mechanism (nogo check) to control the new API.