From dc5ce0721b5c095ed79f2a302ad90d9616785f93 Mon Sep 17 00:00:00 2001 From: Dmitriy Seregin Date: Fri, 13 Aug 2021 20:12:18 +0300 Subject: [PATCH] tls: hide saved private key If private key saved as a string, then hide it from the answer to UI --- CHANGELOG.md | 3 + client/src/__locales/en.json | 3 +- .../components/Settings/Encryption/Form.js | 61 +++++++++++++------ .../components/Settings/Encryption/index.js | 12 +++- client/src/install/Setup/Submit.js | 1 - internal/home/tls.go | 50 +++++++++------ 6 files changed, 90 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d20ef582b69..86b76e2ed98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to ### Added +- Added use saved option when setting private key as string ([#1898]) - Static IP address detection on FreeBSD ([#3289]). - Optimistic cache ([#2145]). - New possible value of `6h` for `querylog_interval` setting ([#2504]). @@ -65,6 +66,7 @@ and this project adheres to ### Fixed +- If private key was saved as a string, then hide it from the UI ([#1898]) - Client ID checking ([#3437]). - Discovering other DHCP servers on `darwin` and `freebsd` ([#3417]). - Switching listening address to unspecified one when bound to a single @@ -92,6 +94,7 @@ and this project adheres to [#1381]: https://github.com/AdguardTeam/AdGuardHome/issues/1381 [#1691]: https://github.com/AdguardTeam/AdGuardHome/issues/1691 +[#1898]: https://github.com/AdguardTeam/AdGuardHome/issues/1898 [#2141]: https://github.com/AdguardTeam/AdGuardHome/issues/2141 [#2145]: https://github.com/AdguardTeam/AdGuardHome/issues/2145 [#2280]: https://github.com/AdguardTeam/AdGuardHome/issues/2280 diff --git a/client/src/__locales/en.json b/client/src/__locales/en.json index e8d4a9b2efb..a6bec141840 100644 --- a/client/src/__locales/en.json +++ b/client/src/__locales/en.json @@ -613,5 +613,6 @@ "port_53_faq_link": "Port 53 is often occupied by \"DNSStubListener\" or \"systemd-resolved\" services. Please read <0>this instruction on how to resolve this.", "adg_will_drop_dns_queries": "AdGuard Home will be dropping all DNS queries from this client.", "client_not_in_allowed_clients": "The client is not allowed because it is not in the \"Allowed clients\" list.", - "experimental": "Experimental" + "experimental": "Experimental", + "use_saved_key": "Use saved key" } diff --git a/client/src/components/Settings/Encryption/Form.js b/client/src/components/Settings/Encryption/Form.js index 58cd181e757..77c2caef859 100644 --- a/client/src/components/Settings/Encryption/Form.js +++ b/client/src/components/Settings/Encryption/Form.js @@ -18,7 +18,7 @@ import i18n from '../../../i18n'; import KeyStatus from './KeyStatus'; import CertificateStatus from './CertificateStatus'; import { - DNS_OVER_QUIC_PORT, DNS_OVER_TLS_PORT, FORM_NAME, STANDARD_HTTPS_PORT, + DNS_OVER_QUIC_PORT, DNS_OVER_TLS_PORT, FORM_NAME, STANDARD_HTTPS_PORT, ENCRYPTION_SOURCE } from '../../../helpers/constants'; const validate = (values) => { @@ -83,6 +83,7 @@ let Form = (props) => { setTlsConfig, certificateSource, privateKeySource, + useSavedKey, } = props; const isSavingDisabled = invalid @@ -265,7 +266,7 @@ let Form = (props) => { - {certificateSource === 'content' && ( + {certificateSource === ENCRYPTION_SOURCE.CONTENT && ( { disabled={!isEnabled} /> )} - {certificateSource === 'path' && ( + {certificateSource === ENCRYPTION_SOURCE.PATH && ( {
- {privateKeySource === 'content' && ( - - )} - {privateKeySource === 'path' && ( + {privateKeySource === ENCRYPTION_SOURCE.PATH && ( { disabled={!isEnabled} /> )} + {privateKeySource === ENCRYPTION_SOURCE.CONTENT && [ +
+ { + if (event.target.checked) { + change("private_key", "") + } + handleChange && handleChange(event) + }} + /> +
, + + ]}
{(privateKey || privateKeyPath) && ( @@ -422,6 +444,7 @@ Form.propTypes = { setTlsConfig: PropTypes.func.isRequired, certificateSource: PropTypes.string, privateKeySource: PropTypes.string, + useSavedKey: PropTypes.bool, }; const selector = formValueSelector(FORM_NAME.ENCRYPTION); @@ -434,6 +457,7 @@ Form = connect((state) => { const privateKeyPath = selector(state, 'private_key_path'); const certificateSource = selector(state, 'certificate_source'); const privateKeySource = selector(state, 'key_source'); + const useSavedKey = selector(state, 'use_saved_key'); return { isEnabled, certificateChain, @@ -442,6 +466,7 @@ Form = connect((state) => { privateKeyPath, certificateSource, privateKeySource, + useSavedKey }; })(Form); diff --git a/client/src/components/Settings/Encryption/index.js b/client/src/components/Settings/Encryption/index.js index f7ca52e0853..37f66ad4ada 100644 --- a/client/src/components/Settings/Encryption/index.js +++ b/client/src/components/Settings/Encryption/index.js @@ -29,9 +29,13 @@ class Encryption extends Component { }, DEBOUNCE_TIMEOUT); getInitialValues = (data) => { - const { certificate_chain, private_key } = data; - const certificate_source = certificate_chain ? 'content' : 'path'; - const key_source = private_key ? 'content' : 'path'; + const { certificate_chain, private_key, use_saved_key } = data; + const certificate_source = certificate_chain + ? ENCRYPTION_SOURCE.CONTENT + : ENCRYPTION_SOURCE.PATH; + const key_source = private_key || use_saved_key + ? ENCRYPTION_SOURCE.CONTENT + : ENCRYPTION_SOURCE.PATH; return { ...data, @@ -71,6 +75,7 @@ class Encryption extends Component { private_key, certificate_path, private_key_path, + use_saved_key } = encryption; const initialValues = this.getInitialValues({ @@ -84,6 +89,7 @@ class Encryption extends Component { private_key, certificate_path, private_key_path, + use_saved_key }); return ( diff --git a/client/src/install/Setup/Submit.js b/client/src/install/Setup/Submit.js index d3c9f555627..a4328d1e25a 100644 --- a/client/src/install/Setup/Submit.js +++ b/client/src/install/Setup/Submit.js @@ -47,7 +47,6 @@ Submit = connect((state) => { }; })(Submit); - export default flow([ withTranslation(), reduxForm({ diff --git a/internal/home/tls.go b/internal/home/tls.go index 709ada191fc..091b0168cfa 100644 --- a/internal/home/tls.go +++ b/internal/home/tls.go @@ -210,15 +210,23 @@ type tlsConfigStatus struct { // field ordering is important -- yaml fields will mirror ordering from here type tlsConfig struct { - tlsConfigStatus `json:",inline"` + tlsConfigStatus `json:",inline"` + tlsConfigSettingsWithKey `json:",inline"` +} + +type tlsConfigSettingsWithKey struct { tlsConfigSettings `json:",inline"` + // If private key saved as string, we set this flag to true and omit it from answer. + UseSavedKey bool `json:"use_saved_key,inline"` } func (t *TLSMod) handleTLSStatus(w http.ResponseWriter, _ *http.Request) { t.confLock.Lock() data := tlsConfig{ - tlsConfigSettings: t.conf, - tlsConfigStatus: t.status, + tlsConfigSettingsWithKey: tlsConfigSettingsWithKey{ + tlsConfigSettings: t.conf, + }, + tlsConfigStatus: t.status, } t.confLock.Unlock() marshalTLS(w, data) @@ -231,19 +239,23 @@ func (t *TLSMod) handleTLSValidate(w http.ResponseWriter, r *http.Request) { return } + if setts.UseSavedKey { + setts.PrivateKey = t.conf.PrivateKey + } + if !WebCheckPortAvailable(setts.PortHTTPS) { httpError(w, http.StatusBadRequest, "port %d is not available, cannot enable HTTPS on it", setts.PortHTTPS) return } status := tlsConfigStatus{} - if tlsLoadConfig(&setts, &status) { + if tlsLoadConfig(&setts.tlsConfigSettings, &status) { status = validateCertificates(string(setts.CertificateChainData), string(setts.PrivateKeyData), setts.ServerName) } data := tlsConfig{ - tlsConfigSettings: setts, - tlsConfigStatus: status, + tlsConfigSettingsWithKey: setts, + tlsConfigStatus: status, } marshalTLS(w, data) } @@ -290,16 +302,20 @@ func (t *TLSMod) handleTLSConfigure(w http.ResponseWriter, r *http.Request) { return } + if data.UseSavedKey { + data.PrivateKey = t.conf.PrivateKey + } + if !WebCheckPortAvailable(data.PortHTTPS) { httpError(w, http.StatusBadRequest, "port %d is not available, cannot enable HTTPS on it", data.PortHTTPS) return } status := tlsConfigStatus{} - if !tlsLoadConfig(&data, &status) { + if !tlsLoadConfig(&data.tlsConfigSettings, &status) { data2 := tlsConfig{ - tlsConfigSettings: data, - tlsConfigStatus: t.status, + tlsConfigSettingsWithKey: data, + tlsConfigStatus: t.status, } marshalTLS(w, data2) @@ -308,7 +324,7 @@ func (t *TLSMod) handleTLSConfigure(w http.ResponseWriter, r *http.Request) { status = validateCertificates(string(data.CertificateChainData), string(data.PrivateKeyData), data.ServerName) - restartHTTPS := t.setConfig(data, status) + restartHTTPS := t.setConfig(data.tlsConfigSettings, status) t.setCertFileTime() onConfigModified() @@ -320,8 +336,8 @@ func (t *TLSMod) handleTLSConfigure(w http.ResponseWriter, r *http.Request) { } data2 := tlsConfig{ - tlsConfigSettings: data, - tlsConfigStatus: t.status, + tlsConfigSettingsWithKey: data, + tlsConfigStatus: t.status, } marshalTLS(w, data2) @@ -335,7 +351,7 @@ func (t *TLSMod) handleTLSConfigure(w http.ResponseWriter, r *http.Request) { // goroutine due to the same reason. if restartHTTPS { go func() { - Context.web.TLSConfigChanged(context.Background(), data) + Context.web.TLSConfigChanged(context.Background(), data.tlsConfigSettings) }() } } @@ -514,8 +530,8 @@ func parsePrivateKey(der []byte) (crypto.PrivateKey, string, error) { } // unmarshalTLS handles base64-encoded certificates transparently -func unmarshalTLS(r *http.Request) (tlsConfigSettings, error) { - data := tlsConfigSettings{} +func unmarshalTLS(r *http.Request) (tlsConfigSettingsWithKey, error) { + data := tlsConfigSettingsWithKey{} err := json.NewDecoder(r.Body).Decode(&data) if err != nil { return data, fmt.Errorf("failed to parse new TLS config json: %w", err) @@ -559,8 +575,8 @@ func marshalTLS(w http.ResponseWriter, data tlsConfig) { } if data.PrivateKey != "" { - encoded := base64.StdEncoding.EncodeToString([]byte(data.PrivateKey)) - data.PrivateKey = encoded + data.UseSavedKey = true + data.PrivateKey = "" } err := json.NewEncoder(w).Encode(data)