Merge pull request #2633 from dlarson04/Issue_2596

Issue 2596 - Agent fails to verify signature due to internal error
open-horizon · Jul 19, 2021 · 69db567 · 69db567
2 parents 0ba1731 + b1aecf6
commit 69db567
Show file tree

Hide file tree

Showing 2 changed files with 95 additions and 17 deletions.
diff --git a/governance/microservice.go b/governance/microservice.go
@@ -41,6 +41,23 @@ func (w *GovernanceWorker) governMicroservices() int {
 	return 0
 }
 
+// Since we changed to saving the signing key with the agreement id, we need to make sure we delete the key when done with it
+// to avoid filling up the filesystem
+func (w *GovernanceWorker) cleanupSigningKeys(keys []string) {
+
+        errHandler := func(keyname string) api.ErrorHandler {
+                return func(err error) bool {
+                        glog.Errorf(logString(fmt.Sprintf("received error when deleting the signing key file %v to anax. %v", keyname, err)))
+                        return true
+                }
+        }
+
+        for _, key := range keys {
+                glog.V(3).Info(fmt.Sprintf("About to delete signing key %s", key))
+                api.DeletePublicKey(key, w.Config, errHandler(key))
+        }
+}
+
 // This function is called when there is a change to a service in the exchange. That might signal a service upgrade.
 func (w *GovernanceWorker) governMicroserviceVersions() {
 
@@ -104,6 +121,10 @@ func (w *GovernanceWorker) StartMicroservice(ms_key string, agreementId string,
 			// now handle the case where there are containers
 			deployment, deploymentSig := msdef.GetDeployment()
 
+			// keep track of keys used for validating signatures so we can clean them up when signatures verified
+			signingKeys := make([]string, 0, 1)
+			num_signing_keys := 0
+
 			// convert workload to policy workload structure
 			var ms_workload policy.Workload
 			ms_workload.Deployment = deployment
@@ -126,25 +147,42 @@ func (w *GovernanceWorker) StartMicroservice(ms_key string, agreementId string,
 						}
 					}
 
+					var prepend_key_string string
+					if len(agreementId) > 0 {
+						prepend_key_string = agreementId
+					} else {
+						prepend_key_string = ms_key
+					}
 					for key, content := range key_map {
 						//add .pem the end of the keyname if it does not have none.
 						fn := key
 						if !strings.HasSuffix(key, ".pem") {
 							fn = fmt.Sprintf("%v.pem", key)
 						}
 
-						api.UploadPublicKey(fn, []byte(content), w.Config, errHandler(fn))
+						// Keys for different services might have the same key name like service.public.pem so prepend something unique like the agreement id 
+						// but then we have to make sure we delete the key when done with it
+						prepend_string := prepend_key_string + "_" + strconv.Itoa(num_signing_keys) + "_"
+						num_signing_keys += 1
+						key_name := prepend_string + fn
+
+						api.UploadPublicKey(key_name, []byte(content), w.Config, errHandler(fn))
+						signingKeys = append(signingKeys, key_name)
 					}
 				}
 			}
 
 			// Verify the deployment signature
 			if pemFiles, err := w.Config.Collaborators.KeyFileNamesFetcher.GetKeyFileNames(w.Config.Edge.PublicKeyPath, w.Config.UserPublicKeyPath()); err != nil {
+				w.cleanupSigningKeys(signingKeys)
 				return nil, fmt.Errorf(logString(fmt.Sprintf("received error getting pem key files: %v", err)))
 			} else if err := ms_workload.HasValidSignature(pemFiles); err != nil {
+				w.cleanupSigningKeys(signingKeys)
 				return nil, fmt.Errorf(logString(fmt.Sprintf("service container has invalid deployment signature %v for %v", ms_workload.DeploymentSignature, ms_workload.Deployment)))
 			}
 
+			w.cleanupSigningKeys(signingKeys)
+
 			// Gather up the service dependencies, if there are any. Microservices in the workload/microservice model never have dependencies,
 			// but services can. It is important to use the correct version for the service dependency, which is the version we have
 			// in the local database, not necessarily the version in the dependency. The version we have in the local database should always

diff --git a/producer/producer_protocol_handler.go b/producer/producer_protocol_handler.go
@@ -17,6 +17,7 @@ import (
 	"github.com/open-horizon/anax/policy"
 	"github.com/open-horizon/anax/worker"
 	"strings"
+	"strconv"
 	"time"
 )
 
@@ -83,6 +84,23 @@ type BaseProducerProtocolHandler struct {
 	ec     exchange.ExchangeContext
 }
 
+// Since we changed to saving the signing key with the agreement id, we need to make sure we delete the key when done with it 
+// to avoid filling up the filesystem
+func (w *BaseProducerProtocolHandler) cleanupSigningKeys(keys []string) {
+
+	errHandler := func(keyname string) api.ErrorHandler {
+                return func(err error) bool {
+                        glog.Errorf(BPPHlogString(w.Name(), fmt.Sprintf("received error when deleting the signing key file %v to anax. %v", keyname, err)))
+                        return true
+                }
+        }
+
+	for _, key := range keys {
+		glog.V(3).Info(fmt.Sprintf("About to delete signing key %s", key))
+		api.DeletePublicKey(key, w.config, errHandler(key))
+	}
+}
+
 func (w *BaseProducerProtocolHandler) GetSendMessage() func(mt interface{}, pay []byte) error {
 	return w.sendMessage
 }
@@ -222,19 +240,10 @@ func (w *BaseProducerProtocolHandler) HandleProposal(ph abstractprotocol.Protoco
 
 		err_log_event := ""
 
-		if err := w.saveSigningKeys(tcPolicy); err != nil {
-			glog.Errorf(BPPHlogString(w.Name(), fmt.Sprintf("received error handling signing keys from the exchange: %v", err)))
-			err_log_event = fmt.Sprintf("Received error handling signing keys from the exchange: %v", err)
-			handled = true
-		} else if pemFiles, err := w.config.Collaborators.KeyFileNamesFetcher.GetKeyFileNames(w.config.Edge.PublicKeyPath, w.config.UserPublicKeyPath()); err != nil {
-			glog.Errorf(BPPHlogString(w.Name(), fmt.Sprintf("received error getting pem key files: %v", err)))
-			err_log_event = fmt.Sprintf("Received error getting pem key files: %v", err)
-			handled = true
-		} else if err := tcPolicy.Is_Self_Consistent(pemFiles, w.GetServiceResolver()); err != nil {
-			glog.Errorf(BPPHlogString(w.Name(), fmt.Sprintf("received error checking self consistency of TsAndCs, %v", err)))
-			err_log_event = fmt.Sprintf("Received error checking self consistency of TsAndCs: %v", err)
-			handled = true
-		} else if dev, err := persistence.FindExchangeDevice(w.db); err != nil {
+		// Keep track of any signing keys we download so we can delete them when done
+		signingKeys := make([]string, 0)
+
+		if dev, err := persistence.FindExchangeDevice(w.db); err != nil {
 			glog.Errorf(BPPHlogString(w.Name(), fmt.Sprintf("device is not configured to accept agreement yet.")))
 			err_log_event = fmt.Sprintf("Device is not configured to accept agreement yet.")
 			handled = true
@@ -275,6 +284,18 @@ func (w *BaseProducerProtocolHandler) HandleProposal(ph abstractprotocol.Protoco
 				proposal.ConsumerId(),
 				proposal.Protocol())
 			handled = true
+		} else if err := w.saveSigningKeys(tcPolicy, proposal.AgreementId(), &signingKeys); err != nil {
+			glog.Errorf(BPPHlogString(w.Name(), fmt.Sprintf("received error handling signing keys from the exchange: %v", err)))
+			err_log_event = fmt.Sprintf("Received error handling signing keys from the exchange: %v", err)
+			handled = true
+		} else if pemFiles, err := w.config.Collaborators.KeyFileNamesFetcher.GetKeyFileNames(w.config.Edge.PublicKeyPath, w.config.UserPublicKeyPath()); err != nil {
+			glog.Errorf(BPPHlogString(w.Name(), fmt.Sprintf("received error getting pem key files: %v", err)))
+			err_log_event = fmt.Sprintf("Received error getting pem key files: %v", err)
+			handled = true
+		} else if err := tcPolicy.Is_Self_Consistent(pemFiles, w.GetServiceResolver()); err != nil {
+			glog.Errorf(BPPHlogString(w.Name(), fmt.Sprintf("received error checking self consistency of TsAndCs, %v", err)))
+			err_log_event = fmt.Sprintf("Received error checking self consistency of TsAndCs: %v", err)
+			handled = true
 		} else if messageTarget, err := exchange.CreateMessageTarget(exchangeMsg.AgbotId, nil, exchangeMsg.AgbotPubKey, ""); err != nil {
 			glog.Errorf(BPPHlogString(w.Name(), fmt.Sprintf("error creating message target: %v", err)))
 			err_log_event = fmt.Sprintf("Error creating message target: %v", err)
@@ -304,9 +325,11 @@ func (w *BaseProducerProtocolHandler) HandleProposal(ph abstractprotocol.Protoco
 						proposal.ConsumerId(),
 						proposal.Protocol())
 				}
+				w.cleanupSigningKeys(signingKeys)
 				return handled, r, tcPolicy
 			}
 		}
+		w.cleanupSigningKeys(signingKeys)
 
 		if err_log_event != "" {
 			eventlog.LogAgreementEvent2(
@@ -325,7 +348,10 @@ func (w *BaseProducerProtocolHandler) HandleProposal(ph abstractprotocol.Protoco
 }
 
 // This function gets the pattern and workload's signing keys and save them to anax
-func (w *BaseProducerProtocolHandler) saveSigningKeys(pol *policy.Policy) error {
+func (w *BaseProducerProtocolHandler) saveSigningKeys(pol *policy.Policy, agreementId string, signingKeys *[]string) error {
+
+	num_signing_keys := 0
+
 	// do nothing if the config does not allow using the certs from the org on the exchange
 	if !w.config.Edge.TrustCertUpdatesFromOrg {
 		return nil
@@ -351,7 +377,14 @@ func (w *BaseProducerProtocolHandler) saveSigningKeys(pol *policy.Policy) error
 					fn = fmt.Sprintf("%v.pem", key)
 				}
 
-				api.UploadPublicKey(fn, []byte(content), w.config, errHandler(fn))
+				// Keys for different services might have the same key name like service.public.pem so prepend something unique like the agreement id
+				// but then we have to make sure we delete the key when done with it
+				prepend_string := agreementId + "_" + strconv.Itoa(num_signing_keys) + "_"
+				num_signing_keys += 1
+				key_name := prepend_string + fn
+
+				api.UploadPublicKey(key_name, []byte(content), w.config, errHandler(fn))
+				(*signingKeys) = append((*signingKeys), key_name)
 			}
 		}
 	}
@@ -369,7 +402,14 @@ func (w *BaseProducerProtocolHandler) saveSigningKeys(pol *policy.Policy) error
 						fn = fmt.Sprintf("%v.pem", key)
 					}
 
-					api.UploadPublicKey(fn, []byte(content), w.config, errHandler(fn))
+				        // Keys for different services might have the same key name like service.public.pem so prepend something unique like the agreement id
+				        // but then we have to make sure we delete the key when done with it
+					prepend_string := agreementId + "_" + strconv.Itoa(num_signing_keys) + "_"
+					num_signing_keys += 1
+					key_name := prepend_string + fn
+
+					api.UploadPublicKey(key_name, []byte(content), w.config, errHandler(fn))
+					(*signingKeys) = append((*signingKeys), key_name)
 				}
 			}
 		}