[Q] ServiceAccount credentials from JSON and private_key_id field in it

[spnshah]

Hello,

We are currently using google-cloud-cpp version v0.20.x for our integration with Google storage.

It was noticed that when trying to establish a service account credentials through JSON contents, private_id field is made mandatory to establish credentials, but we can pass dummy contents to it and it is able to connect to the service. The JSON is having all other fields correct except the private_id field.

json credJson;
credJson["client_email"] = <client_email>;            // Valid value in RHS.
credJson["private_key_id"] = "some dummy string";   // Invalid value in RHS.
credJson["private_key"] = <private_keyValue>;         // Valid value in RHS.
credJson["client_x509_cert_url"] = <x509>;            // Valid value in RHS.
credJson["type"] = "service_account";                 // Valid value in RHS.
credJson["auth_uri"] = "https://accounts.google.com/o/oauth2/auth";
credJson["token_uri"] = "https://oauth2.googleapis.com/token";
credJson["auth_provider_x509_cert_url"] = "https://www.googleapis.com/oauth2/v1/certs";

std::string authStr = credJson.dump();
// Convert authStr to char* secret.
auto creds = oauth2::CreateServiceAccountCredentialsFromJsonContents(secret);

Let me know if I am missing something here in the implementation or if I have understood it incorrect but the overall concern is, with the way I am constructing object which carries the details to establish authentication, I am able to read objects (possibly other operation would be successful too) by having a dummy private_key_id.

Thanks,
Supen

[coryan]

Hello,

Hi!

We are currently using google-cloud-cpp version v0.20.x for our integration with Google storage.

Thanks for including this information.

It was noticed that when trying to establish a service account credentials through JSON contents, private_id field is made mandatory to establish credentials, but we can pass dummy contents to it and it is able to connect to the service.

I see.

The JSON is having all other fields correct except the private_id field.

The field is used in the JWT assertion, it seems the server is not using that field right now to validate the JWT, my guess is this might change in the future.

Let me know if I am missing something here in the implementation or if I have understood it incorrect but the overall concern is, with the way I am constructing object which carries the details to establish authentication, I am able to read objects (possibly other operation would be successful too) by having a dummy private_key_id.

I am no security expert, but no, I do not think you will be able to gain access to something you should not. You still need to be able to sign the JWT with a valid private key (for the given value of client_email), and that is validated on the server-side. The server is maybe being too generous, in that you might be signing with one key but saying that it was signed with another, but the key must still be valid and the service account identified by client_email must have the right access.

[spnshah]

Thank you Carlos for the quick reply.

The field is used in the JWT assertion, it seems the server is not using that field right now to validate the JWT, my guess is this might change in the future.

So not with the current version, but we can possibly expect that field to be in use for JWT validation in future? Reason to double check this is we were thinking to not request this field from the user to avoid an extra input parameter.

I am no security expert, but no, I do not think you will be able to gain access to something you should not.

Per my test, I am actually able to fetch data from cloud bucket and also write to it. Just to mention again that I have all fields in that JSON valid except this one private_key_id.

You still need to be able to sign the JWT with a valid private key (for the given value of client_email), and that is validated on the server-side.

In the JSON that I am constructing, i have all fields valid except the private_key_id. So what you mentioned here seems to be very correct, that it needs a valid private_key and client_email and that is what the server is using to validate.

But then a question comes that since private_key_id isn't really used for validation of credentials, why is it made a mandatory field in the JSON? Currently I am constructing the whole JSON object with just 3 details (private_key_id, private_key and client_email) from the user and it seems we can have one less requirement on the user who is passing over the credentials details.

The server is maybe being too generous, in that you might be signing with one key but saying that it was signed with another, but the key must still be valid and the service account identified by client_email must have the right access.

In my test I had set private_key_id to "some dummy string" and there was no key-id matching "some dummy string". To me it seems like the field is optional as such but the key (private_key_id) is made mandatory.

The question is, is there a need and a plan in place to make this field use for JWT validation? If not, we, as a user of google-cloud-cpp library, were thinking to remove asking its detail from the user and within our code, substitute a dummy value for private_key_id. That way we keep our interface to the user limited to just 2 fields which is also consistent with other clouds and have us avoid extra input data processing.

Thanks.

I am also not an expert but here are my 2 cents. In RFC 7523 the closest equivalent to private_key_id I can find is JWT ID which is optional and used to prevent replay attacks. In googleapis/oauth2client it's also optional so I think that it wouldn't hurt to make it optional here as well?

[coryan]

Thanks @remyabel, I think the relevant RFC is RFC 7515, specifically section 4.1.4 which defines the kid field as optional.

@spnshah I think we should change the C++ library to accept empty strings for the private_key_id. Meanwhile, I believe you can safely assume that this is not going to become a mandatory field.

@spnshah This should be fixed now. Let me know if there's any issues with the latest change.

[spnshah]

Thanks @remyabel, I think the relevant RFC is RFC 7515, specifically section 4.1.4 which defines the kid field as optional.

Thank you @remyabel for the references.

@spnshah I think we should change the C++ library to accept empty strings for the private_key_id. Meanwhile, I believe you can safely assume that this is not going to become a mandatory field.

Thank you @coryan for all the help.