From fecf3640ca7a95f0e5bcaba959cfa1e914961933 Mon Sep 17 00:00:00 2001 From: Aleksandr Rykalin Date: Wed, 13 Feb 2019 11:35:49 +0300 Subject: [PATCH 1/4] Cleanup --- plugin/pki/backend_test.go | 2 + plugin/pki/cert_util.go | 149 -------------------------------- plugin/pki/cert_util_test.go | 125 --------------------------- plugin/pki/path_venafi_fetch.go | 22 +++++ 4 files changed, 24 insertions(+), 274 deletions(-) delete mode 100644 plugin/pki/cert_util.go delete mode 100644 plugin/pki/cert_util_test.go diff --git a/plugin/pki/backend_test.go b/plugin/pki/backend_test.go index 9e61e5c5..28043276 100644 --- a/plugin/pki/backend_test.go +++ b/plugin/pki/backend_test.go @@ -243,3 +243,5 @@ func TestPKI_Cloud_BaseEnroll(t *testing.T) { checkStandartCert(t, data) } + +//TODO: add tests of fetching certificates by common name and serial \ No newline at end of file diff --git a/plugin/pki/cert_util.go b/plugin/pki/cert_util.go deleted file mode 100644 index 6fb1e101..00000000 --- a/plugin/pki/cert_util.go +++ /dev/null @@ -1,149 +0,0 @@ -package pki - -import ( - "context" - "encoding/asn1" - "fmt" - "github.com/hashicorp/vault/helper/errutil" - "github.com/hashicorp/vault/logical" - "regexp" - "strconv" - "strings" -) - -var ( - // A note on hostnameRegex: although we set the StrictDomainName option - // when doing the idna conversion, this appears to only affect output, not - // input, so it will allow e.g. host^123.example.com straight through. So - // we still need to use this to check the output. - hostnameRegex = regexp.MustCompile(`^(\*\.)?(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$`) - oidExtensionBasicConstraints = []int{2, 5, 29, 19} -) - -func validateKeyTypeLength(keyType string, keyBits int) *logical.Response { - switch keyType { - case "rsa": - switch keyBits { - case 2048: - case 4096: - case 8192: - default: - return logical.ErrorResponse(fmt.Sprintf( - "unsupported bit length for RSA key: %d", keyBits)) - } - case "ec": - switch keyBits { - case 224: - case 256: - case 384: - case 521: - default: - return logical.ErrorResponse(fmt.Sprintf( - "unsupported bit length for EC key: %d", keyBits)) - } - case "any": - default: - return logical.ErrorResponse(fmt.Sprintf( - "unknown key type %s", keyType)) - } - - return nil -} - -// Allows fetching certificates from the backend; it handles the slightly -// separate pathing for CA, CRL, and revoked certificates. -func fetchCertBySerial(ctx context.Context, req *logical.Request, prefix, serial string) (*logical.StorageEntry, error) { - var path, legacyPath string - var err error - var certEntry *logical.StorageEntry - - hyphenSerial := normalizeSerial(serial) - colonSerial := strings.Replace(strings.ToLower(serial), "-", ":", -1) - - switch { - // Revoked goes first as otherwise ca/crl get hardcoded paths which fail if - // we actually want revocation info - case strings.HasPrefix(prefix, "revoked/"): - legacyPath = "revoked/" + colonSerial - path = "revoked/" + hyphenSerial - case serial == "ca": - path = "ca" - case serial == "crl": - path = "crl" - default: - legacyPath = "certs/" + colonSerial - path = "certs/" + hyphenSerial - } - - certEntry, err = req.Storage.Get(ctx, path) - if err != nil { - return nil, errutil.InternalError{Err: fmt.Sprintf("error fetching certificate %s: %s", serial, err)} - } - if certEntry != nil { - if certEntry.Value == nil || len(certEntry.Value) == 0 { - return nil, errutil.InternalError{Err: fmt.Sprintf("returned certificate bytes for serial %s were empty", serial)} - } - return certEntry, nil - } - - // If legacyPath is unset, it's going to be a CA or CRL; return immediately - if legacyPath == "" { - return nil, nil - } - - // Retrieve the old-style path - certEntry, err = req.Storage.Get(ctx, legacyPath) - if err != nil { - return nil, errutil.InternalError{Err: fmt.Sprintf("error fetching certificate %s: %s", serial, err)} - } - if certEntry == nil { - return nil, nil - } - if certEntry.Value == nil || len(certEntry.Value) == 0 { - return nil, errutil.InternalError{Err: fmt.Sprintf("returned certificate bytes for serial %s were empty", serial)} - } - - // Update old-style paths to new-style paths - certEntry.Key = path - if err = req.Storage.Put(ctx, certEntry); err != nil { - return nil, errutil.InternalError{Err: fmt.Sprintf("error saving certificate with serial %s to new location", serial)} - } - if err = req.Storage.Delete(ctx, legacyPath); err != nil { - return nil, errutil.InternalError{Err: fmt.Sprintf("error deleting certificate with serial %s from old location", serial)} - } - - return certEntry, nil -} - -func parseOtherSANs(others []string) (map[string][]string, error) { - result := map[string][]string{} - for _, other := range others { - splitOther := strings.SplitN(other, ";", 2) - if len(splitOther) != 2 { - return nil, fmt.Errorf("expected a semicolon in other SAN %q", other) - } - splitType := strings.SplitN(splitOther[1], ":", 2) - if len(splitType) != 2 { - return nil, fmt.Errorf("expected a colon in other SAN %q", other) - } - if strings.ToLower(splitType[0]) != "utf8" { - return nil, fmt.Errorf("only utf8 other SANs are supported; found non-supported type in other SAN %q", other) - } - result[splitOther[0]] = append(result[splitOther[0]], splitType[1]) - } - - return result, nil -} - -func stringToOid(in string) (asn1.ObjectIdentifier, error) { - split := strings.Split(in, ".") - ret := make(asn1.ObjectIdentifier, 0, len(split)) - for _, v := range split { - i, err := strconv.Atoi(v) - if err != nil { - return nil, err - } - ret = append(ret, i) - } - return asn1.ObjectIdentifier(ret), nil -} diff --git a/plugin/pki/cert_util_test.go b/plugin/pki/cert_util_test.go deleted file mode 100644 index 0f243df3..00000000 --- a/plugin/pki/cert_util_test.go +++ /dev/null @@ -1,125 +0,0 @@ -package pki - -import ( - "context" - "fmt" - "testing" - - "strings" - - "github.com/hashicorp/vault/logical" -) - -func TestPki_FetchCertBySerial(t *testing.T) { - storage := &logical.InmemStorage{} - - cases := map[string]struct { - Req *logical.Request - Prefix string - Serial string - }{ - "valid cert": { - &logical.Request{ - Storage: storage, - }, - "certs/", - "00:00:00:00:00:00:00:00", - }, - "revoked cert": { - &logical.Request{ - Storage: storage, - }, - "revoked/", - "11:11:11:11:11:11:11:11", - }, - } - - // Test for colon-based paths in storage - for name, tc := range cases { - storageKey := fmt.Sprintf("%s%s", tc.Prefix, tc.Serial) - err := storage.Put(context.Background(), &logical.StorageEntry{ - Key: storageKey, - Value: []byte("some data"), - }) - if err != nil { - t.Fatalf("error writing to storage on %s colon-based storage path: %s", name, err) - } - - certEntry, err := fetchCertBySerial(context.Background(), tc.Req, tc.Prefix, tc.Serial) - if err != nil { - t.Fatalf("error on %s for colon-based storage path: %s", name, err) - } - - // Check for non-nil on valid/revoked certs - if certEntry == nil { - t.Fatalf("nil on %s for colon-based storage path", name) - } - - // Ensure that cert serials are converted/updated after fetch - expectedKey := tc.Prefix + normalizeSerial(tc.Serial) - se, err := storage.Get(context.Background(), expectedKey) - if err != nil { - t.Fatalf("error on %s for colon-based storage path:%s", name, err) - } - if strings.Compare(expectedKey, se.Key) != 0 { - t.Fatalf("expected: %s, got: %s", expectedKey, certEntry.Key) - } - } - - // Reset storage - storage = &logical.InmemStorage{} - - // Test for hyphen-base paths in storage - for name, tc := range cases { - storageKey := tc.Prefix + normalizeSerial(tc.Serial) - err := storage.Put(context.Background(), &logical.StorageEntry{ - Key: storageKey, - Value: []byte("some data"), - }) - if err != nil { - t.Fatalf("error writing to storage on %s hyphen-based storage path: %s", name, err) - } - - certEntry, err := fetchCertBySerial(context.Background(), tc.Req, tc.Prefix, tc.Serial) - if err != nil || certEntry == nil { - t.Fatalf("error on %s for hyphen-based storage path: err: %v, entry: %v", name, err, certEntry) - } - } - - noConvCases := map[string]struct { - Req *logical.Request - Prefix string - Serial string - }{ - "ca": { - &logical.Request{ - Storage: storage, - }, - "", - "ca", - }, - "crl": { - &logical.Request{ - Storage: storage, - }, - "", - "crl", - }, - } - - // Test for ca and crl case - for name, tc := range noConvCases { - err := storage.Put(context.Background(), &logical.StorageEntry{ - Key: tc.Serial, - Value: []byte("some data"), - }) - if err != nil { - t.Fatalf("error writing to storage on %s: %s", name, err) - } - - certEntry, err := fetchCertBySerial(context.Background(), tc.Req, tc.Prefix, tc.Serial) - if err != nil || certEntry == nil { - t.Fatalf("error on %s: err: %v, entry: %v", name, err, certEntry) - } - } -} diff --git a/plugin/pki/path_venafi_fetch.go b/plugin/pki/path_venafi_fetch.go index c36b31b7..2407864b 100644 --- a/plugin/pki/path_venafi_fetch.go +++ b/plugin/pki/path_venafi_fetch.go @@ -3,6 +3,7 @@ package pki import ( "context" "encoding/pem" + "fmt" "strings" "github.com/hashicorp/vault/helper/errutil" @@ -152,6 +153,27 @@ reply: return } +func fetchCertBySerial(ctx context.Context, req *logical.Request, prefix, serial string) (*logical.StorageEntry, error) { + var path string + var err error + var certEntry *logical.StorageEntry + + hyphenSerial := normalizeSerial(serial) + path = "certs/" + hyphenSerial + + certEntry, err = req.Storage.Get(ctx, path) + if err != nil { + return nil, errutil.InternalError{Err: fmt.Sprintf("error fetching certificate %s: %s", serial, err)} + } + if certEntry != nil { + if certEntry.Value == nil || len(certEntry.Value) == 0 { + return nil, errutil.InternalError{Err: fmt.Sprintf("returned certificate bytes for serial %s were empty", serial)} + } + return certEntry, nil + } + return certEntry, nil +} + const pathVenafiFetchHelpSyn = ` This allows certificates to be fetched. ` From 184dd9d79188618114071ba940bd2b029ce5b46e Mon Sep 17 00:00:00 2001 From: Aleksandr Rykalin Date: Wed, 13 Feb 2019 18:43:34 +0300 Subject: [PATCH 2/4] remove policy modification code from original PKI --- plugin/pki/path_roles.go | 47 ---------------------------------------- 1 file changed, 47 deletions(-) diff --git a/plugin/pki/path_roles.go b/plugin/pki/path_roles.go index 2a8720db..0fbb2bc7 100644 --- a/plugin/pki/path_roles.go +++ b/plugin/pki/path_roles.go @@ -2,11 +2,8 @@ package pki import ( "context" - "strings" "time" - "github.com/hashicorp/vault/helper/consts" - "github.com/hashicorp/vault/helper/parseutil" "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/logical/framework" ) @@ -152,50 +149,6 @@ func (b *backend) getRole(ctx context.Context, s logical.Storage, n string) (*ro return nil, err } - // Migrate existing saved entries and save back if changed - modified := false - if len(result.DeprecatedTTL) == 0 && len(result.Lease) != 0 { - result.DeprecatedTTL = result.Lease - result.Lease = "" - modified = true - } - if result.TTL == 0 && len(result.DeprecatedTTL) != 0 { - parsed, err := parseutil.ParseDurationSecond(result.DeprecatedTTL) - if err != nil { - return nil, err - } - result.TTL = parsed - result.DeprecatedTTL = "" - modified = true - } - if len(result.DeprecatedMaxTTL) == 0 && len(result.LeaseMax) != 0 { - result.DeprecatedMaxTTL = result.LeaseMax - result.LeaseMax = "" - modified = true - } - if result.MaxTTL == 0 && len(result.DeprecatedMaxTTL) != 0 { - parsed, err := parseutil.ParseDurationSecond(result.DeprecatedMaxTTL) - if err != nil { - return nil, err - } - result.MaxTTL = parsed - result.DeprecatedMaxTTL = "" - modified = true - } - - if modified && (b.System().LocalMount() || !b.System().ReplicationState().HasState(consts.ReplicationPerformanceSecondary)) { - jsonEntry, err := logical.StorageEntryJSON("role/"+n, &result) - if err != nil { - return nil, err - } - if err := s.Put(ctx, jsonEntry); err != nil { - // Only perform upgrades on replication primary - if !strings.Contains(err.Error(), logical.ErrReadOnly.Error()) { - return nil, err - } - } - } - return &result, nil } From f745dfab5401f0ebc41df8c0f2867a92b8fc9c15 Mon Sep 17 00:00:00 2001 From: Denis Subbotin Date: Thu, 14 Feb 2019 16:04:17 +0000 Subject: [PATCH 3/4] add artifacts collections --- Makefile | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Makefile b/Makefile index 002c2072..043d2f28 100644 --- a/Makefile +++ b/Makefile @@ -287,3 +287,14 @@ cert_list: show_config: fake_config_read cloud_config_read tpp_config_read config: fake_config_write cloud_config_write tpp_config_write + +collect_artifacts: + mkdir -p artifcats + VERSION=`git describe --abbrev=0 --tags` + mv (PLUGIN_DIR)/linux/$(PLUGIN_NAME) artifcats/$(PLUGIN_NAME)-$(VERSION)_linux + mv (PLUGIN_DIR)/linux86/$(PLUGIN_NAME) artifcats/$(PLUGIN_NAME)-$(VERSION)_linux86 + mv (PLUGIN_DIR)/darwin/$(PLUGIN_NAME) artifcats/$(PLUGIN_NAME)-$(VERSION)_darwin + mv (PLUGIN_DIR)/darwin86/$(PLUGIN_NAME) artifcats/$(PLUGIN_NAME)-$(VERSION)_darwin86 + mv (PLUGIN_DIR)/windows/$(PLUGIN_NAME).exe artifcats/$(PLUGIN_NAME)-$(VERSION)_windows.exe + mv (PLUGIN_DIR)/windows86/$(PLUGIN_NAME).ext artifcats/$(PLUGIN_NAME)-$(VERSION)_windows86.exe + cd artifcats; sha1sum * > hashsums.sha1 \ No newline at end of file From fbbb85f527f51ab7feb6ac270c5474f04d105faa Mon Sep 17 00:00:00 2001 From: Denis Subbotin Date: Thu, 14 Feb 2019 17:52:45 +0000 Subject: [PATCH 4/4] remove redundant VERSION --- Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/Makefile b/Makefile index 043d2f28..51cc2231 100644 --- a/Makefile +++ b/Makefile @@ -290,7 +290,6 @@ config: fake_config_write cloud_config_write tpp_config_write collect_artifacts: mkdir -p artifcats - VERSION=`git describe --abbrev=0 --tags` mv (PLUGIN_DIR)/linux/$(PLUGIN_NAME) artifcats/$(PLUGIN_NAME)-$(VERSION)_linux mv (PLUGIN_DIR)/linux86/$(PLUGIN_NAME) artifcats/$(PLUGIN_NAME)-$(VERSION)_linux86 mv (PLUGIN_DIR)/darwin/$(PLUGIN_NAME) artifcats/$(PLUGIN_NAME)-$(VERSION)_darwin