From 24bb708b21e158712e4c67ceca2816defd44ca88 Mon Sep 17 00:00:00 2001 From: Simon Zolin Date: Tue, 13 Aug 2019 12:32:52 +0300 Subject: [PATCH 1/5] + config: add certificate_path, private_key_path * POST /control/tls/configure: support certificate_path and private_key_path --- AGHTechDoc.md | 63 +++++++++++++++++++++++++++++++++++ dnsforward/dnsforward.go | 10 ++++-- dnsforward/dnsforward_test.go | 6 ++-- home/config.go | 6 ++++ home/control_tls.go | 57 +++++++++++++++++++++++++++++-- home/home.go | 14 ++++---- 6 files changed, 142 insertions(+), 14 deletions(-) diff --git a/AGHTechDoc.md b/AGHTechDoc.md index 54becca5..b4b7bd61 100644 --- a/AGHTechDoc.md +++ b/AGHTechDoc.md @@ -12,6 +12,9 @@ Contents: * Updating * Get version command * Update command +* TLS + * API: Get TLS configuration + * API: Set TLS configuration * Device Names and Per-client Settings * Per-client settings * Get list of clients @@ -515,6 +518,66 @@ Response: 200 OK +## TLS + + +### API: Get TLS configuration + +Request: + + GET /control/tls/status + +Response: + + 200 OK + + { + "enabled":true, + "server_name":"...", + "port_https":443, + "port_dns_over_tls":853, + "certificate_chain":"...", + "private_key":"...", + "certificate_path":"...", + "private_key_path":"..." + + "subject":"CN=...", + "issuer":"CN=...", + "not_before":"2019-03-19T08:23:45Z", + "not_after":"2029-03-16T08:23:45Z", + "dns_names":null, + "key_type":"RSA", + "valid_cert":true, + "valid_key":true, + "valid_chain":false, + "valid_pair":true, + "warning_validation":"Your certificate does not verify: x509: certificate signed by unknown authority" + } + + +### API: Set TLS configuration + +Request: + + POST /control/tls/configure + + { + "enabled":true, + "server_name":"hostname", + "force_https":false, + "port_https":443, + "port_dns_over_tls":853, + "certificate_chain":"...", + "private_key":"...", + "certificate_path":"...", // if set, certificate_chain must be empty + "private_key_path":"..." // if set, private_key must be empty + } + +Response: + + 200 OK + + ## Device Names and Per-client Settings When a client requests information from DNS server, he's identified by IP address. diff --git a/dnsforward/dnsforward.go b/dnsforward/dnsforward.go index 43fda4c4..845c8ca6 100644 --- a/dnsforward/dnsforward.go +++ b/dnsforward/dnsforward.go @@ -108,6 +108,12 @@ type TLSConfig struct { TLSListenAddr *net.TCPAddr `yaml:"-" json:"-"` CertificateChain string `yaml:"certificate_chain" json:"certificate_chain"` // PEM-encoded certificates chain PrivateKey string `yaml:"private_key" json:"private_key"` // PEM-encoded private key + + CertificatePath string `yaml:"certificate_path" json:"certificate_path"` // certificate file name + PrivateKeyPath string `yaml:"private_key_path" json:"private_key_path"` // private key file name + + CertificateChainData []byte `yaml:"-" json:"-"` + PrivateKeyData []byte `yaml:"-" json:"-"` } // ServerConfig represents server configuration. @@ -216,9 +222,9 @@ func (s *Server) startInternal(config *ServerConfig) error { convertArrayToMap(&s.BlockedHosts, s.conf.BlockedHosts) - if s.conf.TLSListenAddr != nil && s.conf.CertificateChain != "" && s.conf.PrivateKey != "" { + if s.conf.TLSListenAddr != nil && len(s.conf.CertificateChainData) != 0 && len(s.conf.PrivateKeyData) != 0 { proxyConfig.TLSListenAddr = s.conf.TLSListenAddr - keypair, err := tls.X509KeyPair([]byte(s.conf.CertificateChain), []byte(s.conf.PrivateKey)) + keypair, err := tls.X509KeyPair(s.conf.CertificateChainData, s.conf.PrivateKeyData) if err != nil { return errorx.Decorate(err, "Failed to parse TLS keypair") } diff --git a/dnsforward/dnsforward_test.go b/dnsforward/dnsforward_test.go index ffcb4d67..cd16f714 100644 --- a/dnsforward/dnsforward_test.go +++ b/dnsforward/dnsforward_test.go @@ -118,9 +118,9 @@ func TestDotServer(t *testing.T) { defer removeDataDir(t) s.conf.TLSConfig = TLSConfig{ - TLSListenAddr: &net.TCPAddr{Port: 0}, - CertificateChain: string(certPem), - PrivateKey: string(keyPem), + TLSListenAddr: &net.TCPAddr{Port: 0}, + CertificateChainData: certPem, + PrivateKeyData: keyPem, } // Starting the server diff --git a/home/config.go b/home/config.go index fd3460ea..1c558e16 100644 --- a/home/config.go +++ b/home/config.go @@ -279,6 +279,12 @@ func parseConfig() error { } config.Clients = nil + status := tlsConfigStatus{} + if !tlsLoadConfig(&config.TLS, &status) { + log.Error("%s", status.WarningValidation) + return err + } + // Deduplicate filters deduplicateFilters() diff --git a/home/control_tls.go b/home/control_tls.go index f1f3d98d..312cf30d 100644 --- a/home/control_tls.go +++ b/home/control_tls.go @@ -14,6 +14,7 @@ import ( "encoding/pem" "errors" "fmt" + "io/ioutil" "net/http" "reflect" "strings" @@ -23,6 +24,41 @@ import ( "github.com/joomcode/errorx" ) +// Set certificate and private key data +func tlsLoadConfig(tls *tlsConfig, status *tlsConfigStatus) bool { + tls.CertificateChainData = []byte(tls.CertificateChain) + tls.PrivateKeyData = []byte(tls.PrivateKey) + + var err error + if tls.CertificatePath != "" { + if tls.CertificateChain != "" { + status.WarningValidation = "certificate data and file can't be set together" + return false + } + tls.CertificateChainData, err = ioutil.ReadFile(tls.CertificatePath) + if err != nil { + status.WarningValidation = err.Error() + return false + } + status.ValidCert = true + } + + if tls.PrivateKeyPath != "" { + if tls.PrivateKey != "" { + status.WarningValidation = "private key data and file can't be set together" + return false + } + tls.PrivateKeyData, err = ioutil.ReadFile(tls.PrivateKeyPath) + if err != nil { + status.WarningValidation = err.Error() + return false + } + status.ValidKey = true + } + + return true +} + // RegisterTLSHandlers registers HTTP handlers for TLS configuration func RegisterTLSHandlers() { httpRegister(http.MethodGet, "/control/tls/status", handleTLSStatus) @@ -55,7 +91,12 @@ func handleTLSValidate(w http.ResponseWriter, r *http.Request) { } } - data.tlsConfigStatus = validateCertificates(data.CertificateChain, data.PrivateKey, data.ServerName) + status := tlsConfigStatus{} + if tlsLoadConfig(&data, &status) { + status = validateCertificates(string(data.CertificateChainData), string(data.PrivateKeyData), data.ServerName) + } + data.tlsConfigStatus = status + marshalTLS(w, data) } @@ -80,8 +121,14 @@ func handleTLSConfigure(w http.ResponseWriter, r *http.Request) { } } + status := tlsConfigStatus{} + if !tlsLoadConfig(&data, &status) { + data.tlsConfigStatus = status + marshalTLS(w, data) + return + } + data.tlsConfigStatus = validateCertificates(string(data.CertificateChainData), string(data.PrivateKeyData), data.ServerName) restartHTTPS := false - data.tlsConfigStatus = validateCertificates(data.CertificateChain, data.PrivateKey, data.ServerName) if !reflect.DeepEqual(config.TLS.tlsConfigSettings, data.tlsConfigSettings) { log.Printf("tls config settings have changed, will restart HTTPS server") restartHTTPS = true @@ -300,6 +347,9 @@ func unmarshalTLS(r *http.Request) (tlsConfig, error) { return data, errorx.Decorate(err, "Failed to base64-decode certificate chain") } data.CertificateChain = string(certPEM) + if data.CertificatePath != "" { + return data, fmt.Errorf("certificate data and file can't be set together") + } } if data.PrivateKey != "" { @@ -309,6 +359,9 @@ func unmarshalTLS(r *http.Request) (tlsConfig, error) { } data.PrivateKey = string(keyPEM) + if data.PrivateKeyPath != "" { + return data, fmt.Errorf("private key data and file can't be set together") + } } return data, nil diff --git a/home/home.go b/home/home.go index 54a3b0a5..0ae64459 100644 --- a/home/home.go +++ b/home/home.go @@ -218,13 +218,13 @@ func httpServerLoop() { // this mechanism doesn't let us through until all conditions are met for config.TLS.Enabled == false || config.TLS.PortHTTPS == 0 || - config.TLS.PrivateKey == "" || - config.TLS.CertificateChain == "" { // sleep until necessary data is supplied + len(config.TLS.PrivateKeyData) == 0 || + len(config.TLS.CertificateChainData) == 0 { // sleep until necessary data is supplied config.httpsServer.cond.Wait() } address := net.JoinHostPort(config.BindHost, strconv.Itoa(config.TLS.PortHTTPS)) // validate current TLS config and update warnings (it could have been loaded from file) - data := validateCertificates(config.TLS.CertificateChain, config.TLS.PrivateKey, config.TLS.ServerName) + data := validateCertificates(string(config.TLS.CertificateChainData), string(config.TLS.PrivateKeyData), config.TLS.ServerName) if !data.ValidPair { cleanupAlways() log.Fatal(data.WarningValidation) @@ -235,10 +235,10 @@ func httpServerLoop() { // prepare certs for HTTPS server // important -- they have to be copies, otherwise changing the contents in config.TLS will break encryption for in-flight requests - certchain := make([]byte, len(config.TLS.CertificateChain)) - copy(certchain, []byte(config.TLS.CertificateChain)) - privatekey := make([]byte, len(config.TLS.PrivateKey)) - copy(privatekey, []byte(config.TLS.PrivateKey)) + certchain := make([]byte, len(config.TLS.CertificateChainData)) + copy(certchain, config.TLS.CertificateChainData) + privatekey := make([]byte, len(config.TLS.PrivateKeyData)) + copy(privatekey, config.TLS.PrivateKeyData) cert, err := tls.X509KeyPair(certchain, privatekey) if err != nil { cleanupAlways() From 4445c4b66985900d4c25f22c0b8a99910cb05c51 Mon Sep 17 00:00:00 2001 From: Simon Zolin Date: Tue, 27 Aug 2019 11:59:05 +0300 Subject: [PATCH 2/5] * openapi: update "TlsConfig" --- openapi/openapi.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index ac831231..a730678f 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -1478,6 +1478,12 @@ definitions: private_key: type: "string" description: "Base64 string with PEM-encoded private key" + certificate_path: + type: "string" + description: "Path to certificate file" + private_key_path: + type: "string" + description: "Path to private key file" # Below goes validation fields valid_cert: type: "boolean" From 6d63450f03882260db4827a9926a9d039d8cbbe8 Mon Sep 17 00:00:00 2001 From: Ildar Kamalov Date: Wed, 28 Aug 2019 18:55:53 +0300 Subject: [PATCH 3/5] + client: handle fields for certificate path and private key path --- client/src/__locales/en.json | 12 +- .../Settings/Encryption/CertificateStatus.js | 71 ++++++ .../components/Settings/Encryption/Form.js | 240 +++++++++++------- .../Settings/Encryption/KeyStatus.js | 31 +++ .../components/Settings/Encryption/index.js | 62 ++++- client/src/helpers/constants.js | 5 + client/src/reducers/encryption.js | 2 + 7 files changed, 311 insertions(+), 112 deletions(-) create mode 100644 client/src/components/Settings/Encryption/CertificateStatus.js create mode 100644 client/src/components/Settings/Encryption/KeyStatus.js diff --git a/client/src/__locales/en.json b/client/src/__locales/en.json index b08147eb..8c0c65db 100644 --- a/client/src/__locales/en.json +++ b/client/src/__locales/en.json @@ -353,5 +353,13 @@ "blocked_services_global": "Use global blocked services", "blocked_service": "Blocked service", "block_all": "Block all", - "unblock_all": "Unblock all" -} \ No newline at end of file + "unblock_all": "Unblock all", + "encryption_certificate_path": "Certificate path", + "encryption_certificate_path_notice": "To add a path to the certificate, clear the certificate chain textarea.", + "encryption_private_key_path": "Private key path", + "encryption_private_key_path_notice": "To add a path to the private key, clear the private key textarea.", + "encryption_certificates_source_path": "Set a certificates file path", + "encryption_certificates_source_content":"Paste the certificates contents", + "encryption_key_source_path": "Set a private key file", + "encryption_key_source_content": "Paste the private key contents" +} diff --git a/client/src/components/Settings/Encryption/CertificateStatus.js b/client/src/components/Settings/Encryption/CertificateStatus.js new file mode 100644 index 00000000..1ecc2742 --- /dev/null +++ b/client/src/components/Settings/Encryption/CertificateStatus.js @@ -0,0 +1,71 @@ +import React, { Fragment } from 'react'; +import PropTypes from 'prop-types'; +import { withNamespaces, Trans } from 'react-i18next'; +import format from 'date-fns/format'; + +import { EMPTY_DATE } from '../../../helpers/constants'; + +const CertificateStatus = ({ + validChain, + validCert, + subject, + issuer, + notAfter, + dnsNames, +}) => ( + +
+ encryption_status: +
+
    +
  • + {validChain ? ( + encryption_chain_valid + ) : ( + encryption_chain_invalid + )} +
    • + {validCert && ( + + {subject && ( +
  • + encryption_subject:  + {subject} +
    • + )} + {issuer && ( +
  • + encryption_issuer:  + {issuer} +
    • + )} + {notAfter && notAfter !== EMPTY_DATE && ( +
  • + encryption_expire:  + {format(notAfter, 'YYYY-MM-DD HH:mm:ss')} +
    • + )} + {dnsNames && ( +
  • + encryption_hostnames:  + {dnsNames} +
    • + )} +     + )} +
+ +); + +CertificateStatus.propTypes = { + validChain: PropTypes.bool.isRequired, + validCert: PropTypes.bool.isRequired, + subject: PropTypes.string, + issuer: PropTypes.string, + notAfter: PropTypes.string, + dnsNames: PropTypes.string, +}; + +export default withNamespaces()(CertificateStatus); diff --git a/client/src/components/Settings/Encryption/Form.js b/client/src/components/Settings/Encryption/Form.js index 94e9923c..058cf918 100644 --- a/client/src/components/Settings/Encryption/Form.js +++ b/client/src/components/Settings/Encryption/Form.js @@ -1,14 +1,22 @@ -import React, { Fragment } from 'react'; +import React from 'react'; import { connect } from 'react-redux'; import PropTypes from 'prop-types'; import { Field, reduxForm, formValueSelector } from 'redux-form'; import { Trans, withNamespaces } from 'react-i18next'; import flow from 'lodash/flow'; -import format from 'date-fns/format'; -import { renderField, renderSelectField, toNumber, port, portTLS, isSafePort } from '../../../helpers/form'; -import { EMPTY_DATE } from '../../../helpers/constants'; +import { + renderField, + renderSelectField, + renderRadioField, + toNumber, + port, + portTLS, + isSafePort, +} from '../../../helpers/form'; import i18n from '../../../i18n'; +import KeyStatus from './KeyStatus'; +import CertificateStatus from './CertificateStatus'; const validate = (values) => { const errors = {}; @@ -27,6 +35,8 @@ const clearFields = (change, setTlsConfig, t) => { const fields = { private_key: '', certificate_chain: '', + private_key_path: '', + certificate_path: '', port_https: 443, port_dns_over_tls: 853, server_name: '', @@ -48,6 +58,8 @@ let Form = (props) => { isEnabled, certificateChain, privateKey, + certificatePath, + privateKeyPath, change, invalid, submitting, @@ -64,6 +76,8 @@ let Form = (props) => { subject, warning_validation, setTlsConfig, + certificateSource, + privateKeySource, } = props; const isSavingDisabled = @@ -71,10 +85,9 @@ let Form = (props) => { submitting || processingConfig || processingValidate || - (isEnabled && (!privateKey || !certificateChain)) || - (privateKey && !valid_key) || - (certificateChain && !valid_cert) || - (privateKey && certificateChain && !valid_pair); + !valid_key || + !valid_cert || + !valid_pair; return (
@@ -182,7 +195,7 @@ let Form = (props) => {
- -
- {certificateChain && ( - -
- encryption_status: -
-
    -
  • - {valid_chain ? ( - encryption_chain_valid - ) : ( - encryption_chain_invalid - )} -
    • - {valid_cert && ( - - {subject && ( -
  • - encryption_subject:  - {subject} -
    • - )} - {issuer && ( -
  • - encryption_issuer:  - {issuer} -
    • - )} - {not_after && not_after !== EMPTY_DATE && ( -
  • - encryption_expire:  - {format(not_after, 'YYYY-MM-DD HH:mm:ss')} -
    • - )} - {dns_names && ( -
  • - encryption_hostnames:  - {dns_names} -
    • - )} -     - )} -
- - )} + +
+
+ + +
+ + {certificateSource === 'content' && ( + + )} + {certificateSource === 'path' && ( + + )} +
+
+ {(certificateChain || certificatePath) && ( + + )}
-
+
- -
- {privateKey && ( - -
- encryption_status: -
-
    -
  • - {valid_key ? ( - - encryption_key_valid - - ) : ( - - encryption_key_invalid - - )} -
    • -
- - )} + +
+
+ + +
+ + {privateKeySource === 'content' && ( + + )} + {privateKeySource === 'path' && ( + + )} +
+
+ {(privateKey || privateKeyPath) && ( + + )}
{warning_validation && ( @@ -334,6 +366,8 @@ Form.propTypes = { isEnabled: PropTypes.bool.isRequired, certificateChain: PropTypes.string.isRequired, privateKey: PropTypes.string.isRequired, + certificatePath: PropTypes.string.isRequired, + privateKeyPath: PropTypes.string.isRequired, change: PropTypes.func.isRequired, submitting: PropTypes.bool.isRequired, invalid: PropTypes.bool.isRequired, @@ -353,6 +387,8 @@ Form.propTypes = { subject: PropTypes.string, t: PropTypes.func.isRequired, setTlsConfig: PropTypes.func.isRequired, + certificateSource: PropTypes.string, + privateKeySource: PropTypes.string, }; const selector = formValueSelector('encryptionForm'); @@ -361,10 +397,18 @@ Form = connect((state) => { const isEnabled = selector(state, 'enabled'); const certificateChain = selector(state, 'certificate_chain'); const privateKey = selector(state, 'private_key'); + const certificatePath = selector(state, 'certificate_path'); + const privateKeyPath = selector(state, 'private_key_path'); + const certificateSource = selector(state, 'certificate_source'); + const privateKeySource = selector(state, 'key_source'); return { isEnabled, certificateChain, privateKey, + certificatePath, + privateKeyPath, + certificateSource, + privateKeySource, }; })(Form); diff --git a/client/src/components/Settings/Encryption/KeyStatus.js b/client/src/components/Settings/Encryption/KeyStatus.js new file mode 100644 index 00000000..08ae1df0 --- /dev/null +++ b/client/src/components/Settings/Encryption/KeyStatus.js @@ -0,0 +1,31 @@ +import React, { Fragment } from 'react'; +import PropTypes from 'prop-types'; +import { withNamespaces, Trans } from 'react-i18next'; + +const KeyStatus = ({ validKey, keyType }) => ( + +
+ encryption_status: +
+
    +
  • + {validKey ? ( + + encryption_key_valid + + ) : ( + + encryption_key_invalid + + )} +
    • +
+ +); + +KeyStatus.propTypes = { + validKey: PropTypes.bool.isRequired, + keyType: PropTypes.string.isRequired, +}; + +export default withNamespaces()(KeyStatus); diff --git a/client/src/components/Settings/Encryption/index.js b/client/src/components/Settings/Encryption/index.js index a8075dc1..06a3b73a 100644 --- a/client/src/components/Settings/Encryption/index.js +++ b/client/src/components/Settings/Encryption/index.js @@ -3,7 +3,7 @@ import PropTypes from 'prop-types'; import { withNamespaces } from 'react-i18next'; import debounce from 'lodash/debounce'; -import { DEBOUNCE_TIMEOUT } from '../../../helpers/constants'; +import { DEBOUNCE_TIMEOUT, ENCRYPTION_SOURCE } from '../../../helpers/constants'; import Form from './Form'; import Card from '../../ui/Card'; import PageTitle from '../../ui/PageTitle'; @@ -19,13 +19,45 @@ class Encryption extends Component { } handleFormSubmit = (values) => { - this.props.setTlsConfig(values); + const submitValues = this.getSubmitValues(values); + this.props.setTlsConfig(submitValues); }; handleFormChange = debounce((values) => { - this.props.validateTlsConfig(values); + const submitValues = this.getSubmitValues(values); + this.props.validateTlsConfig(submitValues); }, DEBOUNCE_TIMEOUT); + getInitialValues = (data) => { + const { certificate_chain, private_key } = data; + const certificate_source = certificate_chain ? 'content' : 'path'; + const key_source = private_key ? 'content' : 'path'; + + return { + ...data, + certificate_source, + key_source, + }; + }; + + getSubmitValues = (values) => { + const { certificate_source, key_source, ...config } = values; + + if (certificate_source === ENCRYPTION_SOURCE.PATH) { + config.certificate_chain = ''; + } else { + config.certificate_path = ''; + } + + if (values.key_source === ENCRYPTION_SOURCE.PATH) { + config.private_key = ''; + } else { + config.private_key_path = ''; + } + + return config; + }; + render() { const { encryption, t } = this.props; const { @@ -36,8 +68,22 @@ class Encryption extends Component { port_dns_over_tls, certificate_chain, private_key, + certificate_path, + private_key_path, } = encryption; + const initialValues = this.getInitialValues({ + enabled, + server_name, + force_https, + port_https, + port_dns_over_tls, + certificate_chain, + private_key, + certificate_path, + private_key_path, + }); + return (
@@ -49,15 +95,7 @@ class Encryption extends Component { bodyType="card-body box-body--settings" > Date: Thu, 29 Aug 2019 11:12:34 +0300 Subject: [PATCH 4/5] * README: update link to the crowdin --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 3f75216c..82b8cf98 100644 --- a/README.md +++ b/README.md @@ -191,7 +191,7 @@ If you run into any problem or have a suggestion, head to [this page](https://gi If you want to help with AdGuard Home translations, please learn more about translating AdGuard products here: https://kb.adguard.com/en/general/adguard-translations -Here is a link to AdGuard Home project: https://crowdin.com/project/adguard-applications +Here is a link to AdGuard Home project: https://crowdin.com/project/adguard-applications/en#/adguard-home ## Acknowledgments From c05917bce0a4c823a4a3f58973e09f7dd8dae877 Mon Sep 17 00:00:00 2001 From: Ildar Kamalov Date: Thu, 29 Aug 2019 11:23:53 +0300 Subject: [PATCH 5/5] - client: remove unused strings --- client/src/__locales/en.json | 2 -- 1 file changed, 2 deletions(-) diff --git a/client/src/__locales/en.json b/client/src/__locales/en.json index 8c0c65db..b6401284 100644 --- a/client/src/__locales/en.json +++ b/client/src/__locales/en.json @@ -355,9 +355,7 @@ "block_all": "Block all", "unblock_all": "Unblock all", "encryption_certificate_path": "Certificate path", - "encryption_certificate_path_notice": "To add a path to the certificate, clear the certificate chain textarea.", "encryption_private_key_path": "Private key path", - "encryption_private_key_path_notice": "To add a path to the private key, clear the private key textarea.", "encryption_certificates_source_path": "Set a certificates file path", "encryption_certificates_source_content":"Paste the certificates contents", "encryption_key_source_path": "Set a private key file",