Skip to content

Commit

Permalink
tls: hide saved private key
Browse files Browse the repository at this point in the history
If private key saved as a string, then hide it from the answer to UI
  • Loading branch information
105th committed Aug 13, 2021
1 parent 394c2f6 commit dc5ce07
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 40 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]).
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion client/src/__locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -613,5 +613,6 @@
"port_53_faq_link": "Port 53 is often occupied by \"DNSStubListener\" or \"systemd-resolved\" services. Please read <0>this instruction</0> 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"
}
61 changes: 43 additions & 18 deletions client/src/components/Settings/Encryption/Form.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -83,6 +83,7 @@ let Form = (props) => {
setTlsConfig,
certificateSource,
privateKeySource,
useSavedKey,
} = props;

const isSavingDisabled = invalid
Expand Down Expand Up @@ -265,7 +266,7 @@ let Form = (props) => {
</div>
</div>

{certificateSource === 'content' && (
{certificateSource === ENCRYPTION_SOURCE.CONTENT && (
<Field
id="certificate_chain"
name="certificate_chain"
Expand All @@ -277,7 +278,7 @@ let Form = (props) => {
disabled={!isEnabled}
/>
)}
{certificateSource === 'path' && (
{certificateSource === ENCRYPTION_SOURCE.PATH && (
<Field
id="certificate_path"
name="certificate_path"
Expand Down Expand Up @@ -314,39 +315,29 @@ let Form = (props) => {
<div className="form__inline mb-2">
<div className="custom-controls-stacked">
<Field
id="key_source_path"
name="key_source"
component={renderRadioField}
type="radio"
className="form-control mr-2"
value="path"
value={ENCRYPTION_SOURCE.PATH}
placeholder={t('encryption_key_source_path')}
disabled={!isEnabled}
/>
<Field
id="key_source_content"
name="key_source"
component={renderRadioField}
type="radio"
className="form-control mr-2"
value="content"
value={ENCRYPTION_SOURCE.CONTENT}
placeholder={t('encryption_key_source_content')}
disabled={!isEnabled}
/>
</div>
</div>

{privateKeySource === 'content' && (
<Field
id="private_key"
name="private_key"
component="textarea"
type="text"
className="form-control form-control--textarea"
placeholder={t('encryption_key_input')}
onChange={handleChange}
disabled={!isEnabled}
/>
)}
{privateKeySource === 'path' && (
{privateKeySource === ENCRYPTION_SOURCE.PATH && (
<Field
id="private_key_path"
name="private_key_path"
Expand All @@ -358,6 +349,37 @@ let Form = (props) => {
disabled={!isEnabled}
/>
)}
{privateKeySource === ENCRYPTION_SOURCE.CONTENT && [
<div
className="form__group form__group--settings mb-2"
key="use_saved_key"
>
<Field
id="use_saved_key"
name="use_saved_key"
type="checkbox"
component={CheckboxField}
placeholder={t('use_saved_key')}
onChange={(event) => {
if (event.target.checked) {
change("private_key", "")
}
handleChange && handleChange(event)
}}
/>
</div>,
<Field
id="private_key"
key="private_key"
name="private_key"
component="textarea"
type="text"
className="form-control form-control--textarea"
placeholder={t('encryption_key_input')}
onChange={handleChange}
disabled={!isEnabled || useSavedKey}
/>
]}
</div>
<div className="form__status">
{(privateKey || privateKeyPath) && (
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand All @@ -442,6 +466,7 @@ Form = connect((state) => {
privateKeyPath,
certificateSource,
privateKeySource,
useSavedKey
};
})(Form);

Expand Down
12 changes: 9 additions & 3 deletions client/src/components/Settings/Encryption/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -71,6 +75,7 @@ class Encryption extends Component {
private_key,
certificate_path,
private_key_path,
use_saved_key
} = encryption;

const initialValues = this.getInitialValues({
Expand All @@ -84,6 +89,7 @@ class Encryption extends Component {
private_key,
certificate_path,
private_key_path,
use_saved_key
});

return (
Expand Down
1 change: 0 additions & 1 deletion client/src/install/Setup/Submit.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ Submit = connect((state) => {
};
})(Submit);


export default flow([
withTranslation(),
reduxForm({
Expand Down
50 changes: 33 additions & 17 deletions internal/home/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)

Expand All @@ -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()

Expand All @@ -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)
Expand All @@ -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)
}()
}
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit dc5ce07

Please sign in to comment.