Return a Status enum from GetPublicKeyAndAlgorithm.

This CL is a refactor with no intended behavior change. The goal is to
reduce redundant strings across different ash consumers of the
platform_keys api.

Bug: 1164523
Change-Id: I4b34cf98ac7303b7dab4e7a799b045972b61ba6b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2630430
Commit-Queue: Erik Chen <erikchen@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843838}

chrome/browser/chromeos/crosapi/keystore_service_ash.cc

@@ -32,8 +32,6 @@ const char kEnterprisePlatformErrorInvalidX509Cert[] =
     "Certificate is not a valid X.509 certificate.";
 const char kUnsupportedKeystoreType[] = "The token is not valid.";
 const char kUnsupportedAlgorithmType[] = "Algorithm type is not supported.";
-const char kErrorAlgorithmNotPermittedByCertificate[] =
-    "The requested Algorithm is not permitted by the certificate.";
 
 // Converts a binary blob to a certificate.
 scoped_refptr<net::X509Certificate> ParseCertificate(
@@ -183,7 +181,8 @@ void KeystoreServiceAsh::GenerateKey(
     }
     default: {
       std::move(callback).Run(mojom::KeystoreBinaryResult::NewErrorMessage(
-          kUnsupportedAlgorithmType));
+          chromeos::platform_keys::StatusToString(
+              chromeos::platform_keys::Status::kErrorAlgorithmNotSupported)));
       break;
     }
   }
@@ -243,7 +242,9 @@ void KeystoreServiceAsh::GetPublicKey(
       StringFromSigningAlgorithmName(algorithm_name);
   if (!name) {
     std::move(callback).Run(mojom::GetPublicKeyResult::NewErrorMessage(
-        kErrorAlgorithmNotPermittedByCertificate));
+        chromeos::platform_keys::StatusToString(
+            chromeos::platform_keys::Status::
+                kErrorAlgorithmNotPermittedByCertificate)));
     return;
   }
 
@@ -252,7 +253,7 @@ void KeystoreServiceAsh::GetPublicKey(
                                                         name.value());
 
   mojom::GetPublicKeyResultPtr result_ptr = mojom::GetPublicKeyResult::New();
-  if (output.error.empty()) {
+  if (output.status == chromeos::platform_keys::Status::kSuccess) {
     base::Optional<crosapi::mojom::KeystoreSigningAlgorithmPtr>
         signing_algorithm =
             crosapi::keystore_service_util::SigningAlgorithmFromDictionary(
@@ -268,7 +269,8 @@ void KeystoreServiceAsh::GetPublicKey(
       result_ptr->set_error_message(kUnsupportedAlgorithmType);
     }
   } else {
-    result_ptr->set_error_message(output.error);
+    result_ptr->set_error_message(
+        chromeos::platform_keys::StatusToString(output.status));
   }
   std::move(callback).Run(std::move(result_ptr));
 }

chrome/browser/chromeos/platform_keys/platform_keys.cc

@@ -19,11 +19,6 @@
 
 namespace {
 
-const char kErrorAlgorithmNotPermittedByCertificate[] =
-    "The requested Algorithm is not permitted by the certificate.";
-const char kErrorInvalidX509Cert[] =
-    "Certificate is not a valid X.509 certificate.";
-
 using crosapi::keystore_service_util::kWebCryptoEcdsa;
 using crosapi::keystore_service_util::kWebCryptoNamedCurveP256;
 using crosapi::keystore_service_util::kWebCryptoRsassaPkcs1v15;
@@ -74,8 +69,12 @@ std::string StatusToString(Status status) {
       return "The operation was successfully executed.";
     case Status::kErrorAlgorithmNotSupported:
       return "Algorithm not supported.";
+    case Status::kErrorAlgorithmNotPermittedByCertificate:
+      return "The requested Algorithm is not permitted by the certificate.";
     case Status::kErrorCertificateNotFound:
       return "Certificate could not be found.";
+    case Status::kErrorCertificateInvalid:
+      return "Certificate is not a valid X.509 certificate.";
     case Status::kErrorInputTooLong:
       return "Input too long.";
     case Status::kErrorGrantKeyPermissionForExtension:
@@ -134,7 +133,7 @@ GetPublicKeyAndAlgorithmOutput GetPublicKeyAndAlgorithm(
   GetPublicKeyAndAlgorithmOutput output;
 
   if (possibly_invalid_cert_der.empty()) {
-    output.error = kErrorInvalidX509Cert;
+    output.status = Status::kErrorCertificateInvalid;
     return output;
   }
 
@@ -147,7 +146,7 @@ GetPublicKeyAndAlgorithmOutput GetPublicKeyAndAlgorithm(
           reinterpret_cast<const char*>(possibly_invalid_cert_der.data()),
           possibly_invalid_cert_der.size(), options);
   if (!cert_x509) {
-    output.error = kErrorInvalidX509Cert;
+    output.status = Status::kErrorCertificateInvalid;
     return output;
   }
 
@@ -158,7 +157,7 @@ GetPublicKeyAndAlgorithmOutput GetPublicKeyAndAlgorithm(
                                              &key_info.key_size_bits) ||
       (key_info.key_type != net::X509Certificate::kPublicKeyTypeRSA &&
        key_info.key_type != net::X509Certificate::kPublicKeyTypeECDSA)) {
-    output.error = StatusToString(Status::kErrorAlgorithmNotSupported);
+    output.status = Status::kErrorAlgorithmNotSupported;
     return output;
   }
 
@@ -169,7 +168,7 @@ GetPublicKeyAndAlgorithmOutput GetPublicKeyAndAlgorithm(
   // with the ECDSA algorithm.
   if (algorithm_name == kWebCryptoRsassaPkcs1v15) {
     if (key_info.key_type != net::X509Certificate::kPublicKeyTypeRSA) {
-      output.error = kErrorAlgorithmNotPermittedByCertificate;
+      output.status = Status::kErrorAlgorithmNotPermittedByCertificate;
       return output;
     }
 
@@ -177,12 +176,13 @@ GetPublicKeyAndAlgorithmOutput GetPublicKeyAndAlgorithm(
     output.public_key =
         std::vector<uint8_t>(key_info.public_key_spki_der.begin(),
                              key_info.public_key_spki_der.end());
+    output.status = Status::kSuccess;
     return output;
   }
 
   if (algorithm_name == kWebCryptoEcdsa) {
     if (key_info.key_type != net::X509Certificate::kPublicKeyTypeECDSA) {
-      output.error = kErrorAlgorithmNotPermittedByCertificate;
+      output.status = Status::kErrorAlgorithmNotPermittedByCertificate;
       return output;
     }
 
@@ -190,10 +190,11 @@ GetPublicKeyAndAlgorithmOutput GetPublicKeyAndAlgorithm(
     output.public_key =
         std::vector<uint8_t>(key_info.public_key_spki_der.begin(),
                              key_info.public_key_spki_der.end());
+    output.status = Status::kSuccess;
     return output;
   }
 
-  output.error = kErrorAlgorithmNotPermittedByCertificate;
+  output.status = Status::kErrorAlgorithmNotPermittedByCertificate;
   return output;
 }
 

chrome/browser/chromeos/platform_keys/platform_keys.h

@@ -45,7 +45,9 @@ enum class TokenId { kUser, kSystem };
 enum class Status {
   kSuccess,
   kErrorAlgorithmNotSupported,
+  kErrorAlgorithmNotPermittedByCertificate,
   kErrorCertificateNotFound,
+  kErrorCertificateInvalid,
   kErrorInputTooLong,
   kErrorGrantKeyPermissionForExtension,
   kErrorInternal,
@@ -85,9 +87,9 @@ struct GetPublicKeyAndAlgorithmOutput {
   GetPublicKeyAndAlgorithmOutput(GetPublicKeyAndAlgorithmOutput&&);
   ~GetPublicKeyAndAlgorithmOutput();
 
-  std::string error;                // Only set on error.
-  std::vector<uint8_t> public_key;  // Only set on success.
-  base::DictionaryValue algorithm;  // Only set on success.
+  Status status = Status::kSuccess;
+  std::vector<uint8_t> public_key;  // Only set if status == kSuccess
+  base::DictionaryValue algorithm;  // Only set if status == kSuccess
 };
 
 // This is a convenient wrapper around GetPublicKey which also builds a

chrome/browser/extensions/api/platform_keys/platform_keys_api_ash.cc

@@ -98,8 +98,9 @@ PlatformKeysInternalGetPublicKeyFunction::Run() {
   chromeos::platform_keys::GetPublicKeyAndAlgorithmOutput output =
       chromeos::platform_keys::GetPublicKeyAndAlgorithm(params->certificate,
                                                         params->algorithm_name);
-  if (!output.error.empty()) {
-    return RespondNow(Error(output.error));
+  if (output.status != chromeos::platform_keys::Status::kSuccess) {
+    return RespondNow(
+        Error(chromeos::platform_keys::StatusToString(output.status)));
   }
 
   api_pki::GetPublicKey::Results::Algorithm algorithm;