[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #6337: updated database and added script to pull expiration info from cert data

[GitBox <gi...@apache.org>]

ocket8888 commented on a change in pull request #6337:
URL: https://github.com/apache/trafficcontrol/pull/6337#discussion_r743995062



##########
File path: traffic_ops/app/db/trafficvault/scripts/fill_expiration_and_provider.go
##########
@@ -0,0 +1,217 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership.  The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+import (
+	"bytes"
+	"crypto/x509"
+	"database/sql"
+	"encoding/base64"
+	"encoding/json"
+	"encoding/pem"
+	"errors"
+	"flag"
+	"fmt"
+	"github.com/apache/trafficcontrol/lib/go-tc"
+	"github.com/apache/trafficcontrol/lib/go-util"
+	"github.com/jmoiron/sqlx"
+	"io/ioutil"
+	"os"
+	"strings"
+	"time"
+
+	_ "github.com/lib/pq"
+)
+
+const PROPERTIES_FILE = "./fill_expiration_and_provider.conf"
+
+func main() {
+	aesKeyLocation := flag.String("aes-key", "/opt/traffic_ops/app/conf/aes.key", "(Optional) The file path for the previous base64 encoded AES key. Default is /opt/traffic_ops/app/conf/aes.key.")
+	cfg := flag.String("cfg", PROPERTIES_FILE, "(Optional) The path for the configuration file. Default is "+PROPERTIES_FILE+".")
+	help := flag.Bool("help", false, "(Optional) Print usage information and exit.")
+	flag.Parse()
+
+	if *help {
+		flag.Usage()
+		os.Exit(0)
+	}
+
+	aesKey, err := readKey(*aesKeyLocation)
+	if err != nil {
+		die("reading previous-key: " + err.Error())
+	}
+
+	dbConfBytes, err := ioutil.ReadFile(*cfg)
+	if err != nil {
+		die("reading db conf '" + *cfg + "': " + err.Error())
+	}
+
+	pgCfg := Config{}
+	err = json.Unmarshal(dbConfBytes, &pgCfg)
+	if err != nil {
+		die("unmarshalling '" + *cfg + "': " + err.Error())
+	}
+
+	sslStr := "require"
+	if !pgCfg.SSL {
+		sslStr = "disable"
+	}
+
+	db, err := sqlx.Open("postgres", fmt.Sprintf("postgres://%s:%s@%s:%d/%s?sslmode=%s&fallback_application_name=trafficvault", pgCfg.User, pgCfg.Password, pgCfg.Hostname, pgCfg.Port, pgCfg.DBName, sslStr))
+	if err != nil {
+		die("opening database: " + err.Error())
+	}
+
+	tx, err := db.Begin()
+	if err != nil {
+		die(fmt.Sprintf("transaction begin failed %v %v ", err, tx))
+	}
+	defer tx.Commit()
+
+	rows, err := tx.Query("SELECT deliveryservice, cdn, version, data, provider, expiration FROM sslkey")
+	if err != nil {
+		die("querying: " + err.Error())
+	}
+	defer rows.Close()
+
+	type expiryAndProvider struct {
+		Provider   string
+		Expiration time.Time
+	}
+	sslKeyMap := map[string]expiryAndProvider{}
+
+	for rows.Next() {
+		var ds string
+		var cdn string
+		var version string
+		var encryptedSslKeys []byte
+		provider := sql.NullString{}
+		var expiration time.Time
+		if err = rows.Scan(&ds, &cdn, &version, &encryptedSslKeys, &provider, &expiration); err != nil {
+			die("getting SSL Keys: " + err.Error())
+		}
+		id := strings.Join([]string{ds, cdn, version}, ", ")
+		jsonKeys, err := util.AESDecrypt(encryptedSslKeys, aesKey)
+		if err != nil {
+			die("reading SSL Keys: " + err.Error())
+		}
+
+		if !bytes.HasPrefix(jsonKeys, []byte("{")) {
+			die("decrypted SSL Key did not have prefix '{' for id " + id)
+		}

Review comment:
       Do you need to do this check? Wouldn't the `json.Unmarshal` call below catch it?

##########
File path: traffic_ops/app/db/trafficvault/scripts/fill_expiration_and_provider.conf
##########
@@ -0,0 +1,8 @@
+{
+	"dbname": "",
+	"hostname": "",
+	"user": "",
+	"password": "",
+	"port": 5432,
+	"ssl": false
+}

Review comment:
       Can you name this file with `.json`? Some editors aren't smart enough to figure out the grammar from the contents and rely on "file extensions".

##########
File path: traffic_ops/app/db/trafficvault/scripts/fill_expiration_and_provider.go
##########
@@ -0,0 +1,217 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership.  The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+import (
+	"bytes"
+	"crypto/x509"
+	"database/sql"
+	"encoding/base64"
+	"encoding/json"
+	"encoding/pem"
+	"errors"
+	"flag"
+	"fmt"
+	"github.com/apache/trafficcontrol/lib/go-tc"
+	"github.com/apache/trafficcontrol/lib/go-util"
+	"github.com/jmoiron/sqlx"
+	"io/ioutil"
+	"os"
+	"strings"
+	"time"
+
+	_ "github.com/lib/pq"
+)
+
+const PROPERTIES_FILE = "./fill_expiration_and_provider.conf"
+
+func main() {
+	aesKeyLocation := flag.String("aes-key", "/opt/traffic_ops/app/conf/aes.key", "(Optional) The file path for the previous base64 encoded AES key. Default is /opt/traffic_ops/app/conf/aes.key.")
+	cfg := flag.String("cfg", PROPERTIES_FILE, "(Optional) The path for the configuration file. Default is "+PROPERTIES_FILE+".")
+	help := flag.Bool("help", false, "(Optional) Print usage information and exit.")
+	flag.Parse()
+
+	if *help {
+		flag.Usage()
+		os.Exit(0)
+	}
+
+	aesKey, err := readKey(*aesKeyLocation)
+	if err != nil {
+		die("reading previous-key: " + err.Error())
+	}
+
+	dbConfBytes, err := ioutil.ReadFile(*cfg)
+	if err != nil {
+		die("reading db conf '" + *cfg + "': " + err.Error())
+	}
+
+	pgCfg := Config{}
+	err = json.Unmarshal(dbConfBytes, &pgCfg)
+	if err != nil {
+		die("unmarshalling '" + *cfg + "': " + err.Error())
+	}
+
+	sslStr := "require"
+	if !pgCfg.SSL {
+		sslStr = "disable"
+	}
+
+	db, err := sqlx.Open("postgres", fmt.Sprintf("postgres://%s:%s@%s:%d/%s?sslmode=%s&fallback_application_name=trafficvault", pgCfg.User, pgCfg.Password, pgCfg.Hostname, pgCfg.Port, pgCfg.DBName, sslStr))
+	if err != nil {
+		die("opening database: " + err.Error())
+	}
+
+	tx, err := db.Begin()
+	if err != nil {
+		die(fmt.Sprintf("transaction begin failed %v %v ", err, tx))
+	}
+	defer tx.Commit()
+
+	rows, err := tx.Query("SELECT deliveryservice, cdn, version, data, provider, expiration FROM sslkey")
+	if err != nil {
+		die("querying: " + err.Error())
+	}
+	defer rows.Close()
+
+	type expiryAndProvider struct {
+		Provider   string
+		Expiration time.Time
+	}
+	sslKeyMap := map[string]expiryAndProvider{}
+
+	for rows.Next() {
+		var ds string
+		var cdn string
+		var version string
+		var encryptedSslKeys []byte
+		provider := sql.NullString{}
+		var expiration time.Time
+		if err = rows.Scan(&ds, &cdn, &version, &encryptedSslKeys, &provider, &expiration); err != nil {
+			die("getting SSL Keys: " + err.Error())
+		}
+		id := strings.Join([]string{ds, cdn, version}, ", ")
+		jsonKeys, err := util.AESDecrypt(encryptedSslKeys, aesKey)
+		if err != nil {
+			die("reading SSL Keys: " + err.Error())
+		}
+
+		if !bytes.HasPrefix(jsonKeys, []byte("{")) {
+			die("decrypted SSL Key did not have prefix '{' for id " + id)
+		}
+
+		sslKey := tc.DeliveryServiceSSLKeysV15{}
+		err = json.Unmarshal([]byte(jsonKeys), &sslKey)
+		if err != nil {
+			die("unmarshalling ssl keys: " + err.Error())
+		}
+
+		parsedCert := sslKey.Certificate
+		err = Base64DecodeCertificate(&parsedCert)
+		if err != nil {
+			die("getting SSL keys for ID '" + id + "': " + err.Error())
+		}
+
+		block, _ := pem.Decode([]byte(parsedCert.Crt))
+		if block == nil {
+			die("Error decoding cert to parse expiration")
+		}
+
+		x509cert, err := x509.ParseCertificate(block.Bytes)
+		if err != nil {
+			die("Error parsing cert to get expiration - " + err.Error())
+		}
+
+		sslKeyMap[id] = expiryAndProvider{
+			Provider:   sslKey.AuthType,
+			Expiration: x509cert.NotAfter,
+		}
+	}
+
+	for id, info := range sslKeyMap {
+		idParts := strings.Split(id, ", ")
+		ds := idParts[0]
+		cdn := idParts[1]
+		version := idParts[2]
+		res, err := tx.Exec(`UPDATE sslkey SET provider = $1, expiration = $2 WHERE deliveryservice = $3 AND cdn = $4 AND version = $5`, info.Provider, info.Expiration, ds, cdn, version)
+		if err != nil {
+			die(fmt.Sprintf("updating SSL Keys for %s, %s", id, err.Error()))

Review comment:
       nit but calling `.Error()` is already the default string coercion for errors, so if you know it isn't `nil` `%s` formatting suffices: https://play.golang.org/p/FtjDb-2ze0Y (if it might be `nil` you'd need to use `%v` instead to avoid segfaulting in that case - but still don't need to explicitly call `.Error()`).

##########
File path: traffic_ops/app/db/trafficvault/scripts/fill_expiration_and_provider.go
##########
@@ -0,0 +1,217 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership.  The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+import (
+	"bytes"
+	"crypto/x509"
+	"database/sql"
+	"encoding/base64"
+	"encoding/json"
+	"encoding/pem"
+	"errors"
+	"flag"
+	"fmt"
+	"github.com/apache/trafficcontrol/lib/go-tc"
+	"github.com/apache/trafficcontrol/lib/go-util"
+	"github.com/jmoiron/sqlx"
+	"io/ioutil"
+	"os"
+	"strings"
+	"time"
+
+	_ "github.com/lib/pq"
+)
+
+const PROPERTIES_FILE = "./fill_expiration_and_provider.conf"
+
+func main() {
+	aesKeyLocation := flag.String("aes-key", "/opt/traffic_ops/app/conf/aes.key", "(Optional) The file path for the previous base64 encoded AES key. Default is /opt/traffic_ops/app/conf/aes.key.")
+	cfg := flag.String("cfg", PROPERTIES_FILE, "(Optional) The path for the configuration file. Default is "+PROPERTIES_FILE+".")
+	help := flag.Bool("help", false, "(Optional) Print usage information and exit.")
+	flag.Parse()
+
+	if *help {
+		flag.Usage()
+		os.Exit(0)
+	}
+
+	aesKey, err := readKey(*aesKeyLocation)
+	if err != nil {
+		die("reading previous-key: " + err.Error())
+	}
+
+	dbConfBytes, err := ioutil.ReadFile(*cfg)
+	if err != nil {
+		die("reading db conf '" + *cfg + "': " + err.Error())
+	}
+
+	pgCfg := Config{}
+	err = json.Unmarshal(dbConfBytes, &pgCfg)
+	if err != nil {
+		die("unmarshalling '" + *cfg + "': " + err.Error())
+	}
+
+	sslStr := "require"
+	if !pgCfg.SSL {
+		sslStr = "disable"
+	}
+
+	db, err := sqlx.Open("postgres", fmt.Sprintf("postgres://%s:%s@%s:%d/%s?sslmode=%s&fallback_application_name=trafficvault", pgCfg.User, pgCfg.Password, pgCfg.Hostname, pgCfg.Port, pgCfg.DBName, sslStr))
+	if err != nil {
+		die("opening database: " + err.Error())
+	}
+
+	tx, err := db.Begin()
+	if err != nil {
+		die(fmt.Sprintf("transaction begin failed %v %v ", err, tx))
+	}
+	defer tx.Commit()
+
+	rows, err := tx.Query("SELECT deliveryservice, cdn, version, data, provider, expiration FROM sslkey")
+	if err != nil {
+		die("querying: " + err.Error())
+	}
+	defer rows.Close()
+
+	type expiryAndProvider struct {
+		Provider   string
+		Expiration time.Time
+	}
+	sslKeyMap := map[string]expiryAndProvider{}
+
+	for rows.Next() {
+		var ds string
+		var cdn string
+		var version string
+		var encryptedSslKeys []byte
+		provider := sql.NullString{}
+		var expiration time.Time
+		if err = rows.Scan(&ds, &cdn, &version, &encryptedSslKeys, &provider, &expiration); err != nil {
+			die("getting SSL Keys: " + err.Error())
+		}
+		id := strings.Join([]string{ds, cdn, version}, ", ")
+		jsonKeys, err := util.AESDecrypt(encryptedSslKeys, aesKey)
+		if err != nil {
+			die("reading SSL Keys: " + err.Error())
+		}
+
+		if !bytes.HasPrefix(jsonKeys, []byte("{")) {
+			die("decrypted SSL Key did not have prefix '{' for id " + id)
+		}
+
+		sslKey := tc.DeliveryServiceSSLKeysV15{}
+		err = json.Unmarshal([]byte(jsonKeys), &sslKey)
+		if err != nil {
+			die("unmarshalling ssl keys: " + err.Error())
+		}
+
+		parsedCert := sslKey.Certificate
+		err = Base64DecodeCertificate(&parsedCert)
+		if err != nil {
+			die("getting SSL keys for ID '" + id + "': " + err.Error())
+		}
+
+		block, _ := pem.Decode([]byte(parsedCert.Crt))
+		if block == nil {
+			die("Error decoding cert to parse expiration")
+		}
+
+		x509cert, err := x509.ParseCertificate(block.Bytes)
+		if err != nil {
+			die("Error parsing cert to get expiration - " + err.Error())
+		}
+
+		sslKeyMap[id] = expiryAndProvider{
+			Provider:   sslKey.AuthType,
+			Expiration: x509cert.NotAfter,
+		}
+	}
+
+	for id, info := range sslKeyMap {
+		idParts := strings.Split(id, ", ")

Review comment:
       If the map key doesn't contain exactly two commas, should this exit here? Currently, more than two commas will be ignored and any text after the third comma is stripped and discarded - is that proper? Or should it consider that part of the `version`? If it's less than two commas, then this will segfault and exit here. IMO it would be better to print a single line error and exit with an error code, but alternatively, would you want it to continue in that case and try to read the other key ids? If so, you'd need to handle that here.

##########
File path: traffic_ops/app/db/trafficvault/scripts/fill_expiration_and_provider.go
##########
@@ -0,0 +1,217 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership.  The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+import (
+	"bytes"
+	"crypto/x509"
+	"database/sql"
+	"encoding/base64"
+	"encoding/json"
+	"encoding/pem"
+	"errors"
+	"flag"
+	"fmt"
+	"github.com/apache/trafficcontrol/lib/go-tc"
+	"github.com/apache/trafficcontrol/lib/go-util"
+	"github.com/jmoiron/sqlx"
+	"io/ioutil"
+	"os"
+	"strings"
+	"time"
+
+	_ "github.com/lib/pq"
+)
+
+const PROPERTIES_FILE = "./fill_expiration_and_provider.conf"
+
+func main() {
+	aesKeyLocation := flag.String("aes-key", "/opt/traffic_ops/app/conf/aes.key", "(Optional) The file path for the previous base64 encoded AES key. Default is /opt/traffic_ops/app/conf/aes.key.")
+	cfg := flag.String("cfg", PROPERTIES_FILE, "(Optional) The path for the configuration file. Default is "+PROPERTIES_FILE+".")
+	help := flag.Bool("help", false, "(Optional) Print usage information and exit.")
+	flag.Parse()
+
+	if *help {
+		flag.Usage()
+		os.Exit(0)
+	}
+
+	aesKey, err := readKey(*aesKeyLocation)
+	if err != nil {
+		die("reading previous-key: " + err.Error())
+	}
+
+	dbConfBytes, err := ioutil.ReadFile(*cfg)
+	if err != nil {
+		die("reading db conf '" + *cfg + "': " + err.Error())
+	}
+
+	pgCfg := Config{}
+	err = json.Unmarshal(dbConfBytes, &pgCfg)
+	if err != nil {
+		die("unmarshalling '" + *cfg + "': " + err.Error())
+	}
+
+	sslStr := "require"
+	if !pgCfg.SSL {
+		sslStr = "disable"
+	}
+
+	db, err := sqlx.Open("postgres", fmt.Sprintf("postgres://%s:%s@%s:%d/%s?sslmode=%s&fallback_application_name=trafficvault", pgCfg.User, pgCfg.Password, pgCfg.Hostname, pgCfg.Port, pgCfg.DBName, sslStr))
+	if err != nil {
+		die("opening database: " + err.Error())
+	}
+
+	tx, err := db.Begin()
+	if err != nil {
+		die(fmt.Sprintf("transaction begin failed %v %v ", err, tx))
+	}
+	defer tx.Commit()
+
+	rows, err := tx.Query("SELECT deliveryservice, cdn, version, data, provider, expiration FROM sslkey")
+	if err != nil {
+		die("querying: " + err.Error())
+	}
+	defer rows.Close()
+
+	type expiryAndProvider struct {
+		Provider   string
+		Expiration time.Time
+	}
+	sslKeyMap := map[string]expiryAndProvider{}
+
+	for rows.Next() {
+		var ds string
+		var cdn string
+		var version string
+		var encryptedSslKeys []byte
+		provider := sql.NullString{}
+		var expiration time.Time
+		if err = rows.Scan(&ds, &cdn, &version, &encryptedSslKeys, &provider, &expiration); err != nil {
+			die("getting SSL Keys: " + err.Error())
+		}
+		id := strings.Join([]string{ds, cdn, version}, ", ")
+		jsonKeys, err := util.AESDecrypt(encryptedSslKeys, aesKey)
+		if err != nil {
+			die("reading SSL Keys: " + err.Error())
+		}
+
+		if !bytes.HasPrefix(jsonKeys, []byte("{")) {
+			die("decrypted SSL Key did not have prefix '{' for id " + id)
+		}
+
+		sslKey := tc.DeliveryServiceSSLKeysV15{}
+		err = json.Unmarshal([]byte(jsonKeys), &sslKey)
+		if err != nil {
+			die("unmarshalling ssl keys: " + err.Error())
+		}
+
+		parsedCert := sslKey.Certificate
+		err = Base64DecodeCertificate(&parsedCert)
+		if err != nil {
+			die("getting SSL keys for ID '" + id + "': " + err.Error())
+		}
+
+		block, _ := pem.Decode([]byte(parsedCert.Crt))
+		if block == nil {
+			die("Error decoding cert to parse expiration")
+		}
+
+		x509cert, err := x509.ParseCertificate(block.Bytes)
+		if err != nil {
+			die("Error parsing cert to get expiration - " + err.Error())
+		}
+
+		sslKeyMap[id] = expiryAndProvider{
+			Provider:   sslKey.AuthType,
+			Expiration: x509cert.NotAfter,
+		}
+	}
+
+	for id, info := range sslKeyMap {
+		idParts := strings.Split(id, ", ")
+		ds := idParts[0]
+		cdn := idParts[1]
+		version := idParts[2]
+		res, err := tx.Exec(`UPDATE sslkey SET provider = $1, expiration = $2 WHERE deliveryservice = $3 AND cdn = $4 AND version = $5`, info.Provider, info.Expiration, ds, cdn, version)
+		if err != nil {
+			die(fmt.Sprintf("updating SSL Keys for %s, %s", id, err.Error()))
+		}
+		rowsAffected, err := res.RowsAffected()
+		if err != nil {
+			die(fmt.Sprintf("determining rows affected for expiration and provider in SSL Keys: %s: %s", id, err.Error()))
+		}
+		if rowsAffected == 0 {
+			die(fmt.Sprintf("no rows updated for expiration and provider in SSL Keys for %s", id))
+		}
+	}
+}
+
+type Config struct {
+	DBName   string `json:"dbname"`
+	Hostname string `json:"hostname"`
+	User     string `json:"user"`
+	Password string `json:"password"`
+	Port     int    `json:"port"`
+	SSL      bool   `json:"ssl"`
+}
+
+func readKey(keyLocation string) ([]byte, error) {
+	var keyBase64 string
+	keyBase64Bytes, err := ioutil.ReadFile(keyLocation)
+	if err != nil {
+		return []byte{}, errors.New("reading file '" + keyLocation + "':" + err.Error())

Review comment:
       constructing errors from errors should use wrapping




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org